diff --git a/cmd/gc/agent_build_params.go b/cmd/gc/agent_build_params.go index d0e843e28..91b177796 100644 --- a/cmd/gc/agent_build_params.go +++ b/cmd/gc/agent_build_params.go @@ -32,7 +32,7 @@ type agentBuildParams struct { packOverlayDirs []string rigOverlayDirs map[string][]string globalFragments []string - appendFragments []string // V2: from [agents].append_fragments / [agent_defaults].append_fragments + appendFragments []string // V2: city-level [agents].append_fragments / [agent_defaults].append_fragments stderr io.Writer // beadStore is the city-level bead store for session bead lookups. diff --git a/cmd/gc/apiroute.go b/cmd/gc/apiroute.go index 1f07c02af..805b9b42d 100644 --- a/cmd/gc/apiroute.go +++ b/cmd/gc/apiroute.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "io" "net" "path/filepath" "strconv" @@ -62,7 +63,7 @@ func standaloneControllerCityName(cfg *config.City, cityPath string) string { // the API server can find the agent. If already qualified or resolution // fails, the original name is returned. func resolveAgentForAPI(cityPath, name string) string { - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return name } diff --git a/cmd/gc/beads_provider_lifecycle.go b/cmd/gc/beads_provider_lifecycle.go index 506fc352f..7eafb72f9 100644 --- a/cmd/gc/beads_provider_lifecycle.go +++ b/cmd/gc/beads_provider_lifecycle.go @@ -184,7 +184,7 @@ func desiredScopeDoltConfigStateForInit(cityPath, dir, prefix string) (contract. return contract.ConfigState{}, false, nil } cityDolt := config.DoltConfig{} - if cfg, err := loadCityConfig(cityPath); err == nil { + if cfg, err := loadCityConfig(cityPath, io.Discard); err == nil { resolveRigPaths(cityPath, cfg.Rigs) cityPrefix := config.EffectiveHQPrefix(cfg) cityDolt = cfg.Dolt @@ -499,7 +499,7 @@ func forcedScopeDoltConfigStateForInit(cityPath, dir, prefix string) (contract.C return contract.ConfigState{}, false, nil } cityDolt := config.DoltConfig{} - if cfg, err := loadCityConfig(cityPath); err == nil { + if cfg, err := loadCityConfig(cityPath, io.Discard); err == nil { resolveRigPaths(cityPath, cfg.Rigs) cityState := desiredCityDoltConfigState(cityPath, cfg.Dolt, config.EffectiveHQPrefix(cfg)) if samePath(cityPath, dir) { @@ -581,7 +581,7 @@ func waitForAllBeadsScopesReadyAfterRecovery(cityPath string, timeout time.Durat // migrated rigs (rig.path only in .gc/site.toml) are still waited // for. A raw config.Load here would silently skip every migrated // rig — the site binding wouldn't populate rig.Path. - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return nil } diff --git a/cmd/gc/build_desired_state_test.go b/cmd/gc/build_desired_state_test.go index 47add5e5c..ebae53ffc 100644 --- a/cmd/gc/build_desired_state_test.go +++ b/cmd/gc/build_desired_state_test.go @@ -16,6 +16,7 @@ import ( "github.com/gastownhall/gascity/internal/beads" "github.com/gastownhall/gascity/internal/beads/contract" "github.com/gastownhall/gascity/internal/config" + "github.com/gastownhall/gascity/internal/fsys" "github.com/gastownhall/gascity/internal/runtime" ) @@ -243,6 +244,153 @@ func TestBuildDesiredState_InstallsGeminiHooksBeforeFingerprinting(t *testing.T) } } +func TestBuildDesiredState_IncludesImportedAlwaysNamedSessions(t *testing.T) { + cityPath := t.TempDir() + rigPath := filepath.Join(cityPath, "repo") + for path, contents := range map[string]string{ + filepath.Join(cityPath, "pack.toml"): ` +[pack] +name = "import-regression" +schema = 2 + +[imports.gs] +source = "./assets/sidecar" +`, + filepath.Join(cityPath, "city.toml"): ` +[workspace] +name = "import-regression" +provider = "claude" + +[[rigs]] +name = "repo" +path = "./repo" + +[rigs.imports.gs] +source = "./assets/sidecar" +`, + filepath.Join(cityPath, "assets", "sidecar", "pack.toml"): ` +[pack] +name = "sidecar" +schema = 2 + +[[named_session]] +template = "captain" +scope = "city" +mode = "always" + +[[named_session]] +template = "watcher" +scope = "rig" +mode = "always" +`, + filepath.Join(cityPath, "assets", "sidecar", "agents", "captain", "agent.toml"): "scope = \"city\"\n", + filepath.Join(cityPath, "assets", "sidecar", "agents", "captain", "prompt.md"): "You are the imported captain.\n", + filepath.Join(cityPath, "assets", "sidecar", "agents", "watcher", "agent.toml"): "scope = \"rig\"\n", + filepath.Join(cityPath, "assets", "sidecar", "agents", "watcher", "prompt.md"): "You are the imported watcher.\n", + } { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%q): %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + t.Fatalf("WriteFile(%q): %v", path, err) + } + } + if err := os.MkdirAll(rigPath, 0o755); err != nil { + t.Fatalf("MkdirAll(%q): %v", rigPath, err) + } + + cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + dsResult := buildDesiredState(cfg.EffectiveCityName(), cityPath, time.Now().UTC(), cfg, runtime.NewFake(), beads.NewMemStore(), io.Discard) + + captain, ok := dsResult.State["gs__captain"] + if !ok { + t.Fatalf("desired state missing gs__captain; keys=%v", mapKeys(dsResult.State)) + } + if captain.TemplateName != "gs.captain" { + t.Fatalf("gs__captain TemplateName = %q, want %q", captain.TemplateName, "gs.captain") + } + if captain.ConfiguredNamedIdentity != "gs.captain" { + t.Fatalf("gs__captain ConfiguredNamedIdentity = %q, want %q", captain.ConfiguredNamedIdentity, "gs.captain") + } + + watcher, ok := dsResult.State["repo--gs__watcher"] + if !ok { + t.Fatalf("desired state missing repo--gs__watcher; keys=%v", mapKeys(dsResult.State)) + } + if watcher.TemplateName != "repo/gs.watcher" { + t.Fatalf("repo--gs__watcher TemplateName = %q, want %q", watcher.TemplateName, "repo/gs.watcher") + } + if watcher.ConfiguredNamedIdentity != "repo/gs.watcher" { + t.Fatalf("repo--gs__watcher ConfiguredNamedIdentity = %q, want %q", watcher.ConfiguredNamedIdentity, "repo/gs.watcher") + } +} + +func TestBuildDesiredState_TransitiveFalseSkipsNestedImportedNamedSessions(t *testing.T) { + cityPath := t.TempDir() + for path, contents := range map[string]string{ + filepath.Join(cityPath, "city.toml"): ` +[workspace] +name = "import-regression" +provider = "claude" + +[imports.outer] +source = "./assets/outer" +transitive = false +`, + filepath.Join(cityPath, "assets", "outer", "pack.toml"): ` +[pack] +name = "outer" +schema = 2 + +[imports.inner] +source = "../inner" + +[[named_session]] +template = "captain" +scope = "city" +mode = "always" +`, + filepath.Join(cityPath, "assets", "outer", "agents", "captain", "agent.toml"): "scope = \"city\"\n", + filepath.Join(cityPath, "assets", "outer", "agents", "captain", "prompt.md"): "You are the outer captain.\n", + filepath.Join(cityPath, "assets", "inner", "pack.toml"): ` +[pack] +name = "inner" +schema = 2 + +[[named_session]] +template = "watcher" +scope = "city" +mode = "always" +`, + filepath.Join(cityPath, "assets", "inner", "agents", "watcher", "agent.toml"): "scope = \"city\"\n", + filepath.Join(cityPath, "assets", "inner", "agents", "watcher", "prompt.md"): "You are the inner watcher.\n", + } { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%q): %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + t.Fatalf("WriteFile(%q): %v", path, err) + } + } + + cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + dsResult := buildDesiredState(cfg.EffectiveCityName(), cityPath, time.Now().UTC(), cfg, runtime.NewFake(), beads.NewMemStore(), io.Discard) + if _, ok := dsResult.State["outer__captain"]; !ok { + t.Fatalf("desired state missing outer__captain; keys=%v", mapKeys(dsResult.State)) + } + if _, ok := dsResult.State["outer__watcher"]; ok { + t.Fatalf("desired state should not include nested named session when transitive=false; keys=%v", mapKeys(dsResult.State)) + } +} + func TestBuildDesiredState_RoutedQueueDoesNotCreateOneSessionPerBead(t *testing.T) { cityPath := t.TempDir() if err := os.MkdirAll(filepath.Join(cityPath, ".beads"), 0o700); err != nil { diff --git a/cmd/gc/chat_autosuspend.go b/cmd/gc/chat_autosuspend.go index 5eccba107..6dfdfed96 100644 --- a/cmd/gc/chat_autosuspend.go +++ b/cmd/gc/chat_autosuspend.go @@ -23,7 +23,7 @@ func autoSuspendChatSessions(store beads.Store, sp runtime.Provider, idleTimeout cityPath, _ := resolveCity() var cfg *config.City if cityPath != "" { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } catalog, err := workerSessionCatalogWithConfig(cityPath, store, sp, cfg) if err != nil { diff --git a/cmd/gc/cmd_agent.go b/cmd/gc/cmd_agent.go index f4fff2a18..0bc7d7a2e 100644 --- a/cmd/gc/cmd_agent.go +++ b/cmd/gc/cmd_agent.go @@ -27,21 +27,22 @@ Describe what this agent should do here. // via packs are visible. The only exceptions are quick pre-fetch checks // in cmd_config.go and cmd_start.go that intentionally use config.Load to // discover remote packs before fetching them. -func loadCityConfig(cityPath string) (*config.City, error) { +func loadCityConfig(cityPath string, warningWriter ...io.Writer) (*config.City, error) { extras := builtinPackIncludes(cityPath) - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml"), extras...) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml"), extras...) if err != nil { return nil, err } + emitLoadCityConfigWarnings(resolveLoadCityConfigWarningWriter(warningWriter...), prov) applyFeatureFlags(cfg) return cfg, nil } // loadCityConfigSuppressDeprecatedOrderWarnings performs a full config load // while suppressing only legacy order-path migration warnings. -func loadCityConfigSuppressDeprecatedOrderWarnings(cityPath string) (*config.City, error) { +func loadCityConfigSuppressDeprecatedOrderWarnings(cityPath string, warningWriter ...io.Writer) (*config.City, error) { extras := builtinPackIncludes(cityPath) - cfg, _, err := config.LoadWithIncludesOptions( + cfg, prov, err := config.LoadWithIncludesOptions( fsys.OSFS{}, filepath.Join(cityPath, "city.toml"), config.LoadOptions{SuppressDeprecatedOrderWarnings: true}, @@ -50,6 +51,9 @@ func loadCityConfigSuppressDeprecatedOrderWarnings(cityPath string) (*config.Cit if err != nil { return nil, err } + if len(warningWriter) > 0 { + emitLoadCityConfigWarnings(resolveLoadCityConfigWarningWriter(warningWriter...), prov) + } applyFeatureFlags(cfg) return cfg, nil } @@ -57,15 +61,97 @@ func loadCityConfigSuppressDeprecatedOrderWarnings(cityPath string) (*config.Cit // loadCityConfigFS is the testable variant of loadCityConfig that accepts a // filesystem implementation. Used by functions that take an fsys.FS parameter // for unit testing. -func loadCityConfigFS(fs fsys.FS, tomlPath string) (*config.City, error) { - cfg, _, err := config.LoadWithIncludes(fs, tomlPath) +func loadCityConfigFS(fs fsys.FS, tomlPath string, warningWriter ...io.Writer) (*config.City, error) { + cfg, prov, err := config.LoadWithIncludes(fs, tomlPath) if err != nil { return nil, err } + emitLoadCityConfigWarnings(resolveLoadCityConfigWarningWriter(warningWriter...), prov) applyFeatureFlags(cfg) return cfg, nil } +func resolveLoadCityConfigWarningWriter(warningWriter ...io.Writer) io.Writer { + for _, w := range warningWriter { + if w != nil { + return w + } + } + return os.Stderr +} + +func emitLoadCityConfigWarnings(w io.Writer, prov *config.Provenance) { + if w == nil || prov == nil || len(prov.Warnings) == 0 { + return + } + seen := make(map[string]struct{}, len(prov.Warnings)) + for _, warning := range prov.Warnings { + if !shouldEmitLoadCityConfigWarning(warning) { + continue + } + if _, dup := seen[warning]; dup { + continue + } + seen[warning] = struct{}{} + fmt.Fprintln(w, warning) //nolint:errcheck // best-effort warning emission + } +} + +// Alias-only warnings, deferred future-surface keys, and tombstone attachment +// deprecations stay soft so legacy configs keep booting. A mixed +// [agent_defaults]/[agents] config remains strict-fatal because overlapping +// default tables are ambiguous even after normalization. +func isNonFatalLoadConfigWarning(warning string) bool { + if strings.Contains(warning, "[agents] is a deprecated compatibility alias for [agent_defaults]") { + return true + } + if strings.Contains(warning, "attachment-list fields") { + return true + } + if !strings.Contains(warning, `" is not supported`) { + return false + } + return strings.Contains(warning, `"agent_defaults.`) || strings.Contains(warning, `"agents.`) +} + +func shouldEmitLoadCityConfigWarning(warning string) bool { + if strings.Contains(warning, "both [agent_defaults] and [agents] are present") { + return true + } + return isNonFatalLoadConfigWarning(warning) +} + +func strictFatalLoadConfigWarnings(warnings []string) []string { + if len(warnings) == 0 { + return nil + } + var fatal []string + for _, warning := range warnings { + if isNonFatalLoadConfigWarning(warning) { + continue + } + fatal = append(fatal, warning) + } + return fatal +} + +func emitNonFatalLoadConfigWarnings(w io.Writer, prov *config.Provenance) { + if w == nil || prov == nil || len(prov.Warnings) == 0 { + return + } + seen := make(map[string]struct{}, len(prov.Warnings)) + for _, warning := range prov.Warnings { + if !isNonFatalLoadConfigWarning(warning) { + continue + } + if _, dup := seen[warning]; dup { + continue + } + seen[warning] = struct{}{} + fmt.Fprintln(w, warning) //nolint:errcheck // best-effort warning emission + } +} + // loadCityConfigForEditFS loads the raw city config WITHOUT pack/include // expansion. Use for commands that modify city.toml and write it back — // preserves include directives, pack references, and patches. @@ -385,7 +471,7 @@ func doAgentAdd(fs fsys.FS, cityPath, name, promptTemplate, dir string, suspende return 1 } - cfg, err := loadCityConfigFS(fs, tomlPath) + cfg, err := loadCityConfigFS(fs, tomlPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc agent add: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -606,7 +692,7 @@ func doAgentSuspendOrResume(fs fsys.FS, cityPath, name string, suspended bool, s } // Phase 2: not in raw config — check expanded config for provenance. - expanded, err := loadCityConfigFS(fs, tomlPath) + expanded, err := loadCityConfigFS(fs, tomlPath, stderr) if err != nil { fmt.Fprintln(stderr, agentNotFoundMsg("gc agent "+verb, name, cfg)) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_agent_test.go b/cmd/gc/cmd_agent_test.go index d259dc6ee..d9c2dbdf5 100644 --- a/cmd/gc/cmd_agent_test.go +++ b/cmd/gc/cmd_agent_test.go @@ -3,10 +3,13 @@ package main import ( "bytes" "fmt" + "os" "path/filepath" + "regexp" "strings" "testing" + "github.com/gastownhall/gascity/internal/config" "github.com/gastownhall/gascity/internal/formula" "github.com/gastownhall/gascity/internal/fsys" "github.com/gastownhall/gascity/internal/molecule" @@ -135,6 +138,92 @@ func TestDoAgentResumePackDerivedError(t *testing.T) { } } +func TestLoadCityConfigFSEmitsProvenanceWarnings(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(`[workspace] +name = "test-city" +`) + fs.Files["/city/pack.toml"] = []byte(`[pack] +name = "test-city" +schema = 2 + +[agents] +append_fragments = ["footer"] +`) + + var stderr bytes.Buffer + cfg, err := loadCityConfigFS(fs, "/city/city.toml", &stderr) + if err != nil { + t.Fatalf("loadCityConfigFS: %v", err) + } + if cfg == nil { + t.Fatal("loadCityConfigFS returned nil config") + } + if !strings.Contains(stderr.String(), "[agents] is a deprecated compatibility alias for [agent_defaults]") { + t.Fatalf("expected [agents] alias warning, got %q", stderr.String()) + } +} + +func TestLoadCityConfigFSEmitsMigrationWarningsAcrossCalls(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(`[workspace] +name = "test-city" +`) + fs.Files["/city/pack.toml"] = []byte(`[pack] +name = "test-city" +schema = 2 + +[agents] +append_fragments = ["footer"] +`) + + var stderr bytes.Buffer + for i := 0; i < 2; i++ { + cfg, err := loadCityConfigFS(fs, "/city/city.toml", &stderr) + if err != nil { + t.Fatalf("loadCityConfigFS call %d: %v", i+1, err) + } + if cfg == nil { + t.Fatalf("loadCityConfigFS call %d returned nil config", i+1) + } + } + + const want = "[agents] is a deprecated compatibility alias for [agent_defaults]" + if got := strings.Count(stderr.String(), want); got != 2 { + t.Fatalf("warning count = %d, want 2; stderr=%q", got, stderr.String()) + } +} + +func TestEmitLoadCityConfigWarningsFiltersNonMigrationWarnings(t *testing.T) { + var stderr bytes.Buffer + emitLoadCityConfigWarnings(&stderr, &config.Provenance{ + Warnings: []string{ + `workspace.name redefined by "/city/defaults.toml"`, + `/city/pack.toml: [agents] is a deprecated compatibility alias for [agent_defaults]; rewrite the table name to [agent_defaults]`, + `/city/pack.toml: both [agent_defaults] and [agents] are present; [agent_defaults] wins on overlapping keys and [agents] only fills gaps`, + `/city/pack.toml: "agent_defaults.provider" is not supported in [agent_defaults]; keep using workspace.provider or set provider per agent in agents//agent.toml`, + `gc: warning: attachment-list fields (` + "`skills`, `mcp`, `skills_append`, `mcp_append`, `shared_skills`" + `) are deprecated as of v0.15.1 and ignored.`, + }, + }) + + output := stderr.String() + if strings.Contains(output, `workspace.name redefined by "/city/defaults.toml"`) { + t.Fatalf("non-migration warning should be filtered, got %q", output) + } + if !strings.Contains(output, `[agents] is a deprecated compatibility alias for [agent_defaults]`) { + t.Fatalf("expected alias warning, got %q", output) + } + if !strings.Contains(output, `both [agent_defaults] and [agents] are present`) { + t.Fatalf("expected mixed-table warning, got %q", output) + } + if !strings.Contains(output, `"agent_defaults.provider" is not supported`) { + t.Fatalf("expected unsupported-key warning, got %q", output) + } + if !strings.Contains(output, "attachment-list fields") { + t.Fatalf("expected attachment deprecation warning, got %q", output) + } +} + func TestDoAgentSuspendRootPackAgent(t *testing.T) { fs := fsys.NewFake() fs.Files["/city/city.toml"] = []byte(`[workspace] @@ -312,6 +401,52 @@ name = "mayor" } } +func TestStrictFatalLoadConfigWarningsKeepsMixedTableWarningsFatal(t *testing.T) { + warnings := []string{ + `/city/pack.toml: [agents] is a deprecated compatibility alias for [agent_defaults]; rewrite the table name to [agent_defaults]`, + `/city/pack.toml: both [agent_defaults] and [agents] are present; [agent_defaults] wins on overlapping keys and [agents] only fills gaps`, + `/city/pack.toml: "agent_defaults.provider" is not supported in [agent_defaults]; keep using workspace.provider or set provider per agent in agents//agent.toml`, + `workspace.name redefined by "/city/defaults.toml"`, + } + + got := strictFatalLoadConfigWarnings(warnings) + if len(got) != 2 { + t.Fatalf("strictFatalLoadConfigWarnings len = %d, want 2; got=%q", len(got), got) + } + if got[0] != `/city/pack.toml: both [agent_defaults] and [agents] are present; [agent_defaults] wins on overlapping keys and [agents] only fills gaps` { + t.Fatalf("strictFatalLoadConfigWarnings[0] = %q, want mixed-table warning", got[0]) + } + if got[1] != `workspace.name redefined by "/city/defaults.toml"` { + t.Fatalf("strictFatalLoadConfigWarnings[1] = %q, want non-migration warning", got[1]) + } +} + +func TestNonTestLoadCityConfigCallersPassWarningWriter(t *testing.T) { + files, err := filepath.Glob("*.go") + if err != nil { + t.Fatalf("Glob(*.go): %v", err) + } + bareCall := regexp.MustCompile(`\bloadCityConfig(FS)?\([^,\n)]*\)`) + var offenders []string + for _, file := range files { + if strings.HasSuffix(file, "_test.go") || file == "cmd_agent.go" { + continue + } + data, err := os.ReadFile(file) + if err != nil { + t.Fatalf("ReadFile(%q): %v", file, err) + } + for i, line := range strings.Split(string(data), "\n") { + if bareCall.MatchString(line) { + offenders = append(offenders, fmt.Sprintf("%s:%d: %s", file, i+1, strings.TrimSpace(line))) + } + } + } + if len(offenders) > 0 { + t.Fatalf("bare loadCityConfig callers found:\n%s", strings.Join(offenders, "\n")) + } +} + // --------------------------------------------------------------------------- // doAgentAdd — v2 scaffold behavior // --------------------------------------------------------------------------- diff --git a/cmd/gc/cmd_bd.go b/cmd/gc/cmd_bd.go index 0d25bef42..702f0111d 100644 --- a/cmd/gc/cmd_bd.go +++ b/cmd/gc/cmd_bd.go @@ -97,7 +97,7 @@ func doBd(args []string, stdout, stderr io.Writer) int { // resolve to their bound path. A raw config.Load here would make // every already-migrated rig look unbound and fail the new guard // in resolveBdScopeTarget / bdRigScopeTarget. - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc bd: loading config: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_beads_city.go b/cmd/gc/cmd_beads_city.go index 8582752e2..d5e7654e5 100644 --- a/cmd/gc/cmd_beads_city.go +++ b/cmd/gc/cmd_beads_city.go @@ -130,7 +130,7 @@ func doBeadsCityEndpoint(fs fsys.FS, cityPath string, opts cityEndpointOptions, fmt.Fprintf(stderr, "%s: loading config: %v\n", name, err) //nolint:errcheck return 1 } - cfg, err := loadCityConfigFS(fs, filepath.Join(cityPath, "city.toml")) + cfg, err := loadCityConfigFS(fs, filepath.Join(cityPath, "city.toml"), stderr) if err != nil { fmt.Fprintf(stderr, "%s: loading expanded config: %v\n", name, err) //nolint:errcheck return 1 diff --git a/cmd/gc/cmd_citystatus.go b/cmd/gc/cmd_citystatus.go index bd17b7f54..374b37e3d 100644 --- a/cmd/gc/cmd_citystatus.go +++ b/cmd/gc/cmd_citystatus.go @@ -92,7 +92,7 @@ func cmdCityStatus(args []string, jsonOutput bool, stdout, stderr io.Writer) int return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc status: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_convoy.go b/cmd/gc/cmd_convoy.go index 978418e9d..a4c6d30fc 100644 --- a/cmd/gc/cmd_convoy.go +++ b/cmd/gc/cmd_convoy.go @@ -102,11 +102,12 @@ func cmdConvoyCreateWithOptions(args []string, opts convoyCreateOptions, stdout, fmt.Fprintf(stderr, "gc convoy create: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) if err != nil { fmt.Fprintf(stderr, "gc convoy create: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } + emitLoadCityConfigWarnings(stderr, prov) issueIDs := []string(nil) if len(args) > 1 { @@ -374,11 +375,12 @@ func openAllConvoyStores(stderr io.Writer, cmdName string) ([]convoyStoreView, i fmt.Fprintf(stderr, "%s: %v\n", cmdName, err) //nolint:errcheck // best-effort stderr return nil, 1 } - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) if err != nil { fmt.Fprintf(stderr, "%s: %v\n", cmdName, err) //nolint:errcheck // best-effort stderr return nil, 1 } + emitLoadCityConfigWarnings(stderr, prov) stores, err := openConvoyStores(cfg, cityPath, "", func(storeDir string) (beads.Store, error) { return openStoreAtForCity(storeDir, cityPath) }) @@ -421,11 +423,12 @@ func openConvoyStoreByID(convoyID string, stderr io.Writer, cmdName string) (bea fmt.Fprintf(stderr, "%s: %v\n", cmdName, err) //nolint:errcheck // best-effort stderr return nil, 1 } - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) if err != nil { fmt.Fprintf(stderr, "%s: %v\n", cmdName, err) //nolint:errcheck // best-effort stderr return nil, 1 } + emitLoadCityConfigWarnings(stderr, prov) store, err := resolveConvoyStore(convoyID, cfg, cityPath, func(storeDir string) (beads.Store, error) { return openStoreAtForCity(storeDir, cityPath) }) diff --git a/cmd/gc/cmd_convoy_dispatch.go b/cmd/gc/cmd_convoy_dispatch.go index 33636de67..c00e6e34c 100644 --- a/cmd/gc/cmd_convoy_dispatch.go +++ b/cmd/gc/cmd_convoy_dispatch.go @@ -111,14 +111,14 @@ func pokeControlDispatch(cityPath string) error { return pokeController(cityPath) } -func runControlDispatcher(beadID string, stdout, _ io.Writer) error { +func runControlDispatcher(beadID string, stdout, stderr io.Writer) error { cityPath, err := resolveCity() if err != nil { return err } // Try all stores (city + rigs) to find the bead. - store, bead, err := findBeadAcrossStores(cityPath, beadID) + store, bead, err := findBeadAcrossStores(cityPath, beadID, stderr) if err != nil { return fmt.Errorf("loading bead %s: %w", beadID, err) } @@ -131,7 +131,7 @@ func runControlDispatcher(beadID string, stdout, _ io.Writer) error { loadCfg = true } if loadCfg { - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { return err } @@ -179,8 +179,10 @@ func runControlDispatcher(beadID string, stdout, _ io.Writer) error { return nil } -// findBeadAcrossStores preserves the historical city-first lookup semantics. -func findBeadAcrossStores(cityPath, beadID string) (beads.Store, beads.Bead, error) { +// findBeadAcrossStores tries the city store first, then all rig stores, +// returning the store and bead on first match. +func findBeadAcrossStores(cityPath, beadID string, warningWriter io.Writer) (beads.Store, beads.Bead, error) { + // Try city store first. cityStore, err := openStoreAtForCity(cityPath, cityPath) if err != nil { return nil, beads.Bead{}, fmt.Errorf("opening city store: %w", err) @@ -190,7 +192,9 @@ func findBeadAcrossStores(cityPath, beadID string) (beads.Store, beads.Bead, err } else if !errors.Is(err, beads.ErrNotFound) { return nil, beads.Bead{}, fmt.Errorf("getting bead %q from %s: %w", beadID, cityPath, err) } - cfg, err := loadCityConfig(cityPath) + + // Try rig stores. + cfg, err := loadCityConfig(cityPath, warningWriter) if err != nil { return nil, beads.Bead{}, err } @@ -215,7 +219,7 @@ func findBeadAcrossStores(cityPath, beadID string) (beads.Store, beads.Bead, err } func findUniqueBeadAcrossStoresView(cityPath, beadID string) (convoyStoreView, beads.Bead, error) { - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, os.Stderr) if err != nil { return convoyStoreView{}, beads.Bead{}, fmt.Errorf("loading city config for bead %q: %w", beadID, err) } @@ -484,7 +488,7 @@ func cmdWorkflowDelete(workflowID string, force, deleteBeads bool, stdout, stder _, _ = fmt.Fprintf(stderr, "gc workflow delete: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc workflow delete: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -719,7 +723,7 @@ func cmdWorkflowDeleteSource(sourceBeadID string, selector sourceWorkflowStoreSe _, _ = fmt.Fprintf(stderr, "gc workflow delete-source: %v\n", err) return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { _, _ = fmt.Fprintf(stderr, "gc workflow delete-source: %v\n", err) return 1 @@ -863,7 +867,7 @@ func cmdWorkflowReopenSource(sourceBeadID string, selector sourceWorkflowStoreSe _, _ = fmt.Fprintf(stderr, "gc workflow reopen-source: %v\n", err) return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { _, _ = fmt.Fprintf(stderr, "gc workflow reopen-source: %v\n", err) return 1 diff --git a/cmd/gc/cmd_convoy_dispatch_test.go b/cmd/gc/cmd_convoy_dispatch_test.go index 10e39487d..2a5047f80 100644 --- a/cmd/gc/cmd_convoy_dispatch_test.go +++ b/cmd/gc/cmd_convoy_dispatch_test.go @@ -1702,7 +1702,7 @@ name = "test-city" } t.Setenv("GC_BEADS", "exec:/definitely/missing/provider") - _, _, err := findBeadAcrossStores(cityPath, "gc-missing") + _, _, err := findBeadAcrossStores(cityPath, "gc-missing", io.Discard) if err == nil { t.Fatal("findBeadAcrossStores() error = nil, want provider failure") } diff --git a/cmd/gc/cmd_dashboard.go b/cmd/gc/cmd_dashboard.go index 04d569a67..a704cf27e 100644 --- a/cmd/gc/cmd_dashboard.go +++ b/cmd/gc/cmd_dashboard.go @@ -69,7 +69,7 @@ func bindDashboardServeFlags(cmd *cobra.Command, port *int, apiURL *string) { } func runDashboardServe(commandName string, port int, apiURLOverride string, stderr io.Writer) error { - cityPath, cfg, err := resolveDashboardContext() + cityPath, cfg, err := resolveDashboardContext(stderr) if err != nil { fmt.Fprintf(stderr, "%s: %v\n", commandName, err) //nolint:errcheck // best-effort stderr return err @@ -88,7 +88,7 @@ func runDashboardServe(commandName string, port int, apiURLOverride string, stde return nil } -func resolveDashboardContext() (cityPath string, cfg *config.City, err error) { +func resolveDashboardContext(warningWriter ...io.Writer) (cityPath string, cfg *config.City, err error) { cityPath, err = resolveCity() if err != nil { if strings.TrimSpace(cityFlag) == "" && strings.Contains(err.Error(), "not in a city directory") { @@ -96,7 +96,7 @@ func resolveDashboardContext() (cityPath string, cfg *config.City, err error) { } return "", nil, err } - cfg, err = loadCityConfig(cityPath) + cfg, err = loadCityConfig(cityPath, warningWriter...) if err != nil { return "", nil, err } diff --git a/cmd/gc/cmd_doctor.go b/cmd/gc/cmd_doctor.go index 6d7f57eca..908662ff2 100644 --- a/cmd/gc/cmd_doctor.go +++ b/cmd/gc/cmd_doctor.go @@ -54,7 +54,7 @@ func doctorSkipsDoltChecks(cityPath string) bool { if os.Getenv("GC_DOLT") == "skip" { return true } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return !cityUsesBdStoreContract(cityPath) } @@ -130,7 +130,7 @@ func doDoctor(fix, verbose bool, stdout, stderr io.Writer) int { // Load config for deeper checks. If it fails, we still run the core // checks above (which will report the parse error). - cfg, cfgErr := loadCityConfig(cityPath) + cfg, cfgErr := loadCityConfig(cityPath, stderr) if cfgErr == nil { resolveRigPaths(cityPath, cfg.Rigs) if workspaceUsesManagedBdStoreContract(cityPath, cfg.Rigs) { @@ -273,7 +273,7 @@ func collectPackDirs(cfg *config.City) []string { // backfillRigIndex registers all rigs from the given city in the global // rig index and writes GT_ROOT to each rig's .beads/.env. func backfillRigIndex(cityPath string) error { - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return err } diff --git a/cmd/gc/cmd_formula.go b/cmd/gc/cmd_formula.go index b80fc4e62..6bd5364c9 100644 --- a/cmd/gc/cmd_formula.go +++ b/cmd/gc/cmd_formula.go @@ -18,13 +18,13 @@ func newFormulaCmd(stdout, stderr io.Writer) *cobra.Command { Short: "Manage and inspect formulas", } - cmd.AddCommand(newFormulaListCmd(stdout)) + cmd.AddCommand(newFormulaListCmd(stdout, stderr)) cmd.AddCommand(newFormulaShowCmd(stdout, stderr)) cmd.AddCommand(newFormulaCookCmd(stdout, stderr)) return cmd } -func newFormulaListCmd(stdout io.Writer) *cobra.Command { +func newFormulaListCmd(stdout, stderr io.Writer) *cobra.Command { return &cobra.Command{ Use: "list", Short: "List available formulas", @@ -33,7 +33,7 @@ func newFormulaListCmd(stdout io.Writer) *cobra.Command { Formulas are discovered from city-level and rig-level formula directories configured via packs and formulas_dir settings.`, RunE: func(_ *cobra.Command, _ []string) error { - paths := allFormulaSearchPaths() + paths := allFormulaSearchPaths(stderr) if len(paths) == 0 { _, _ = fmt.Fprintln(stdout, "No formula search paths configured.") return nil @@ -79,7 +79,7 @@ configured via packs and formulas_dir settings.`, } } -func newFormulaShowCmd(stdout, _ io.Writer) *cobra.Command { +func newFormulaShowCmd(stdout, stderr io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "show ", Short: "Show a compiled formula recipe", @@ -106,7 +106,7 @@ Examples: compileVars := vars - recipe, err := formula.Compile(cmd.Context(), name, cityFormulaSearchPaths(), compileVars) + recipe, err := formula.Compile(cmd.Context(), name, cityFormulaSearchPaths(stderr), compileVars) if err != nil { return err } @@ -201,7 +201,7 @@ Examples: return cmd } -func newFormulaCookCmd(stdout, _ io.Writer) *cobra.Command { +func newFormulaCookCmd(stdout, stderr io.Writer) *cobra.Command { var title string var vars []string var metadata []string @@ -225,7 +225,7 @@ bead into a sub-workflow at runtime.`, if err != nil { return err } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { return err } @@ -331,12 +331,12 @@ func parseMetadataArgs(items []string) (map[string]string, error) { // cityFormulaSearchPaths returns the city-level formula search paths. // Best-effort: returns nil if no city is loaded. -func cityFormulaSearchPaths() []string { +func cityFormulaSearchPaths(warningWriter ...io.Writer) []string { cityPath, err := resolveCity() if err != nil { return nil } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, warningWriter...) if err != nil { return nil } @@ -346,12 +346,12 @@ func cityFormulaSearchPaths() []string { // allFormulaSearchPaths returns the deduplicated union of formula search // paths across city and all rigs. Used by gc formula list to discover // every available formula regardless of scope. -func allFormulaSearchPaths() []string { +func allFormulaSearchPaths(warningWriter ...io.Writer) []string { cityPath, err := resolveCity() if err != nil { return nil } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, warningWriter...) if err != nil { return nil } diff --git a/cmd/gc/cmd_graph.go b/cmd/gc/cmd_graph.go index 7451fe1ac..a67e20041 100644 --- a/cmd/gc/cmd_graph.go +++ b/cmd/gc/cmd_graph.go @@ -70,7 +70,7 @@ func openRigAwareStore(args []string, stderr io.Writer) (beads.Store, int) { // Try to resolve rig from the first bead arg's prefix. if len(args) > 0 { - cfg, cfgErr := loadCityConfig(cityPath) + cfg, cfgErr := loadCityConfig(cityPath, stderr) if cfgErr == nil { if storeDir := slingDirForBead(cfg, cityPath, args[0]); storeDir != cityPath { store, err := openStoreAtForCity(storeDir, cityPath) diff --git a/cmd/gc/cmd_handoff.go b/cmd/gc/cmd_handoff.go index 9b9f0a840..0f9d6a132 100644 --- a/cmd/gc/cmd_handoff.go +++ b/cmd/gc/cmd_handoff.go @@ -77,7 +77,7 @@ func cmdHandoff(args []string, target string, stdout, stderr io.Writer) int { sp := newSessionProvider() dops := newDrainOps(sp) rec := openCityRecorderAt(current.cityPath, stderr) - cfg, _ := loadCityConfig(current.cityPath) + cfg, _ := loadCityConfig(current.cityPath, stderr) persistRestart := sessionRestartPersister(current.cityPath, store, sp, cfg, current.sessionName) outcome := doHandoffWithOutcome(store, rec, dops, persistRestart, current.display, current.sessionName, args, stdout, stderr) @@ -95,7 +95,7 @@ func cmdHandoff(args []string, target string, stdout, stderr io.Writer) int { // cmdHandoffRemote sends handoff mail to a remote session and stops the target // only when the controller can restart it. Returns immediately. func cmdHandoffRemote(args []string, target string, stdout, stderr io.Writer) int { - targetInfo, err := resolveSessionRuntimeTarget(target) + targetInfo, err := resolveSessionRuntimeTarget(target, stderr) if err != nil { fmt.Fprintf(stderr, "gc handoff: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_hook.go b/cmd/gc/cmd_hook.go index bb96d79f3..d19834174 100644 --- a/cmd/gc/cmd_hook.go +++ b/cmd/gc/cmd_hook.go @@ -85,7 +85,7 @@ func cmdHookWithFormat(args []string, inject bool, hookFormat string, stdout, st fmt.Fprintf(stderr, "gc hook: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { if inject { return 0 diff --git a/cmd/gc/cmd_import.go b/cmd/gc/cmd_import.go index e0cbc54fb..68fbdffc6 100644 --- a/cmd/gc/cmd_import.go +++ b/cmd/gc/cmd_import.go @@ -31,20 +31,125 @@ var ( const cityPackSchema = 1 +type cityPackAgentDefaults struct { + Model string `toml:"model,omitempty"` + WakeMode string `toml:"wake_mode,omitempty"` + DefaultSlingFormula string `toml:"default_sling_formula,omitempty"` + AllowOverlay []string `toml:"allow_overlay,omitempty"` + AllowEnvOverride []string `toml:"allow_env_override,omitempty"` + AppendFragments []string `toml:"append_fragments,omitempty"` + Skills []string `toml:"skills,omitempty"` + MCP []string `toml:"mcp,omitempty"` + Provider *string `toml:"provider,omitempty"` + Scope *string `toml:"scope,omitempty"` + InstallAgentHooks *[]string `toml:"install_agent_hooks,omitempty"` +} + type cityPackManifest struct { - Pack config.PackMeta `toml:"pack"` - Imports map[string]config.Import `toml:"imports,omitempty"` - AgentDefaults config.AgentDefaults `toml:"agent_defaults,omitempty"` - Defaults packDefaults `toml:"defaults,omitempty"` - Agents []config.Agent `toml:"agent,omitempty"` - NamedSessions []config.NamedSession `toml:"named_session,omitempty"` - Services []config.Service `toml:"service,omitempty"` - Providers map[string]config.ProviderSpec `toml:"providers,omitempty"` - Formulas config.FormulasConfig `toml:"formulas,omitempty"` - Patches config.Patches `toml:"patches,omitempty"` - Doctor []config.PackDoctorEntry `toml:"doctor,omitempty"` - Commands []config.PackCommandEntry `toml:"commands,omitempty"` - Global config.PackGlobal `toml:"global,omitempty"` + Pack config.PackMeta `toml:"pack"` + Imports map[string]config.Import `toml:"imports,omitempty"` + AgentDefaults cityPackAgentDefaults `toml:"agent_defaults,omitempty"` + AgentsDefaults cityPackAgentDefaults `toml:"agents,omitempty"` + Defaults packDefaults `toml:"defaults,omitempty"` + Agents []config.Agent `toml:"agent,omitempty"` + NamedSessions []config.NamedSession `toml:"named_session,omitempty"` + Services []config.Service `toml:"service,omitempty"` + Providers map[string]config.ProviderSpec `toml:"providers,omitempty"` + Formulas config.FormulasConfig `toml:"formulas,omitempty"` + Patches config.Patches `toml:"patches,omitempty"` + Doctor []config.PackDoctorEntry `toml:"doctor,omitempty"` + Commands []config.PackCommandEntry `toml:"commands,omitempty"` + Global config.PackGlobal `toml:"global,omitempty"` + HadAgentsDefaultsAlias bool `toml:"-"` + HadBothAgentDefaultsTables bool `toml:"-"` +} + +func (d cityPackAgentDefaults) unsupportedKeys() []string { + var keys []string + if d.Provider != nil { + keys = append(keys, "provider") + } + if d.Scope != nil { + keys = append(keys, "scope") + } + if d.InstallAgentHooks != nil { + keys = append(keys, "install_agent_hooks") + } + return keys +} + +func warnPackAgentDefaultsCompatibility(stderr io.Writer, manifest *cityPackManifest, rewrite bool) { + if stderr == nil || manifest == nil { + return + } + if manifest.HadAgentsDefaultsAlias { + if rewrite { + fmt.Fprintln(stderr, "gc import: [agents] is a deprecated compatibility alias for [agent_defaults]; rewriting pack.toml to canonical [agent_defaults]") //nolint:errcheck + } else { + fmt.Fprintln(stderr, "gc import: [agents] is a deprecated compatibility alias for [agent_defaults]; rewrite pack.toml to canonical [agent_defaults]") //nolint:errcheck + } + } + if manifest.HadBothAgentDefaultsTables { + fmt.Fprintln(stderr, "gc import: both [agent_defaults] and [agents] are present in pack.toml; canonical [agent_defaults] wins for overlapping keys") //nolint:errcheck + } + keys := manifest.AgentDefaults.unsupportedKeys() + if len(keys) == 0 { + return + } + fmt.Fprintf(stderr, "gc import: preserved unsupported [agent_defaults] keys in pack.toml: %s; runtime will continue warning until they are moved to per-agent config\n", strings.Join(keys, ", ")) //nolint:errcheck +} + +func mergeCityPackAgentDefaultsPreferCanonical(dst *cityPackAgentDefaults, src cityPackAgentDefaults, meta toml.MetaData) { + if !meta.IsDefined("agent_defaults", "model") { + dst.Model = src.Model + } + if !meta.IsDefined("agent_defaults", "wake_mode") { + dst.WakeMode = src.WakeMode + } + if !meta.IsDefined("agent_defaults", "default_sling_formula") { + dst.DefaultSlingFormula = src.DefaultSlingFormula + } + if !meta.IsDefined("agent_defaults", "allow_overlay") { + dst.AllowOverlay = append([]string(nil), src.AllowOverlay...) + } + if !meta.IsDefined("agent_defaults", "allow_env_override") { + dst.AllowEnvOverride = append([]string(nil), src.AllowEnvOverride...) + } + if !meta.IsDefined("agent_defaults", "append_fragments") { + dst.AppendFragments = append([]string(nil), src.AppendFragments...) + } + if !meta.IsDefined("agent_defaults", "skills") { + dst.Skills = append([]string(nil), src.Skills...) + } + if !meta.IsDefined("agent_defaults", "mcp") { + dst.MCP = append([]string(nil), src.MCP...) + } + if !meta.IsDefined("agent_defaults", "provider") { + dst.Provider = copyStringPointer(src.Provider) + } + if !meta.IsDefined("agent_defaults", "scope") { + dst.Scope = copyStringPointer(src.Scope) + } + if !meta.IsDefined("agent_defaults", "install_agent_hooks") { + dst.InstallAgentHooks = copyStringSlicePointer(src.InstallAgentHooks) + } +} + +func copyStringPointer(in *string) *string { + if in == nil { + return nil + } + out := *in + return &out +} + +func copyStringSlicePointer(in *[]string) *[]string { + if in == nil { + return nil + } + out := make([]string, len(*in)) + copy(out, *in) + return &out } func newImportCmd(stdout, stderr io.Writer) *cobra.Command { @@ -292,6 +397,7 @@ func doImportAdd(fs fsys.FS, cityPath, source, nameOverride, versionFlag string, fmt.Fprintf(stderr, "gc import add %q: %v\n", source, err) //nolint:errcheck return 1 } + warnPackAgentDefaultsCompatibility(stderr, manifest, true) if err := writeImportLockfile(fs, cityPath, lock); err != nil { fmt.Fprintf(stderr, "gc import add %q: %v\n", source, err) //nolint:errcheck return 1 @@ -321,6 +427,7 @@ func doImportRemove(fs fsys.FS, cityPath, name string, stdout, stderr io.Writer) fmt.Fprintf(stderr, "gc import remove %q: %v\n", name, err) //nolint:errcheck return 1 } + warnPackAgentDefaultsCompatibility(stderr, manifest, true) if err := writeImportLockfile(fs, cityPath, lock); err != nil { fmt.Fprintf(stderr, "gc import remove %q: %v\n", name, err) //nolint:errcheck return 1 @@ -335,6 +442,7 @@ func doImportInstall(cityPath string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc import install: %v\n", err) //nolint:errcheck return 1 } + warnPackAgentDefaultsCompatibility(stderr, manifest, false) lock, err := syncImports(cityPath, manifest.Imports, packman.InstallFromLock) if err != nil { fmt.Fprintf(stderr, "gc import install: %v\n", err) //nolint:errcheck @@ -360,6 +468,7 @@ func doImportUpgrade(cityPath, target string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc import upgrade: %v\n", err) //nolint:errcheck return 1 } + warnPackAgentDefaultsCompatibility(stderr, manifest, false) var lock *packman.Lockfile if target == "" { @@ -421,6 +530,7 @@ func doImportList(cityPath string, tree bool, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc import list: %v\n", err) //nolint:errcheck return 1 } + warnPackAgentDefaultsCompatibility(stderr, manifest, false) lock, err := readImportLockfile(fsys.OSFS{}, cityPath) if err != nil { fmt.Fprintf(stderr, "gc import list: %v\n", err) //nolint:errcheck @@ -552,9 +662,11 @@ func loadCityPackManifestFS(fs fsys.FS, cityPath string) (*cityPackManifest, err } var manifest cityPackManifest - if _, err := toml.Decode(string(data), &manifest); err != nil { + md, err := toml.Decode(string(data), &manifest) + if err != nil { return nil, fmt.Errorf("parsing pack.toml: %w", err) } + normalizeCityPackManifestAgentDefaultsAlias(&manifest, md) if manifest.Pack.Name == "" { manifest.Pack.Name = defaultCityPackName(fs, cityPath) } @@ -564,6 +676,7 @@ func loadCityPackManifestFS(fs fsys.FS, cityPath string) (*cityPackManifest, err if manifest.Imports == nil { manifest.Imports = make(map[string]config.Import) } + manifest.AgentsDefaults = cityPackAgentDefaults{} return &manifest, nil } @@ -580,6 +693,7 @@ func writeCityPackManifest(fs fsys.FS, cityPath string, manifest *cityPackManife if manifest.Imports == nil { manifest.Imports = make(map[string]config.Import) } + manifest.AgentsDefaults = cityPackAgentDefaults{} var buf bytes.Buffer if err := toml.NewEncoder(&buf).Encode(manifest); err != nil { @@ -588,6 +702,22 @@ func writeCityPackManifest(fs fsys.FS, cityPath string, manifest *cityPackManife return fsys.WriteFileAtomic(fs, filepath.Join(cityPath, "pack.toml"), buf.Bytes(), 0o644) } +func normalizeCityPackManifestAgentDefaultsAlias(manifest *cityPackManifest, meta toml.MetaData) { + manifest.HadAgentsDefaultsAlias = meta.IsDefined("agents") + if meta.IsDefined("agent_defaults") { + if meta.IsDefined("agents") { + manifest.HadBothAgentDefaultsTables = true + mergeCityPackAgentDefaultsPreferCanonical(&manifest.AgentDefaults, manifest.AgentsDefaults, meta) + } + manifest.AgentsDefaults = cityPackAgentDefaults{} + return + } + if meta.IsDefined("agents") { + manifest.AgentDefaults = manifest.AgentsDefaults + manifest.AgentsDefaults = cityPackAgentDefaults{} + } +} + func defaultCityPackName(fs fsys.FS, cityPath string) string { cfg, err := config.Load(fs, filepath.Join(cityPath, "city.toml")) if err == nil { diff --git a/cmd/gc/cmd_import_test.go b/cmd/gc/cmd_import_test.go index 81ffc9ee1..35aef9d18 100644 --- a/cmd/gc/cmd_import_test.go +++ b/cmd/gc/cmd_import_test.go @@ -6,6 +6,8 @@ import ( "os" "os/exec" "path/filepath" + "reflect" + "sort" "strings" "testing" @@ -45,6 +47,9 @@ func TestDoImportAddRemoteWritesConfigAndLock(t *testing.T) { if code != 0 { t.Fatalf("code = %d, stderr = %s", code, stderr.String()) } + if stderr.Len() != 0 { + t.Fatalf("expected no warnings for synthetic empty manifest, got stderr %q", stderr.String()) + } cfg, err := config.Load(fsys.OSFS{}, filepath.Join(dir, "pack.toml")) if err != nil { @@ -73,6 +78,35 @@ func TestDoImportAddRemoteWritesConfigAndLock(t *testing.T) { } } +func TestCityPackAgentDefaultsCoversConfigAgentDefaultsFields(t *testing.T) { + configFields := make(map[string]bool) + for _, field := range reflect.VisibleFields(reflect.TypeOf(config.AgentDefaults{})) { + if field.PkgPath != "" { + continue + } + configFields[field.Name] = true + } + + manifestFields := make(map[string]bool) + for _, field := range reflect.VisibleFields(reflect.TypeOf(cityPackAgentDefaults{})) { + if field.PkgPath != "" { + continue + } + manifestFields[field.Name] = true + } + + var missing []string + for name := range configFields { + if !manifestFields[name] { + missing = append(missing, name) + } + } + sort.Strings(missing) + if len(missing) > 0 { + t.Fatalf("cityPackAgentDefaults missing config.AgentDefaults fields: %v", missing) + } +} + func TestDoImportAddPreservesExistingPackTomlContent(t *testing.T) { clearGCEnv(t) dir := t.TempDir() @@ -159,6 +193,224 @@ session_live = ["echo hi"] } } +func TestDoImportAddCanonicalizesLegacyAgentsDefaultsAlias(t *testing.T) { + clearGCEnv(t) + dir := t.TempDir() + writeCityToml(t, dir, "[workspace]\nname = \"demo\"\n") + writePackToml(t, dir, `[pack] +name = "demo" +schema = 1 + +[agents] +default_sling_formula = "mol-legacy" +append_fragments = ["legacy-fragment"] +provider = "claude" +scope = "city" +install_agent_hooks = ["claude"] +`) + + prevResolve := resolveImportVersion + prevConstraint := defaultImportConstraint + prevSync := syncImports + t.Cleanup(func() { + resolveImportVersion = prevResolve + defaultImportConstraint = prevConstraint + syncImports = prevSync + }) + resolveImportVersion = func(_, _ string) (packman.ResolvedVersion, error) { + return packman.ResolvedVersion{Version: "1.4.2", Commit: "abc123"}, nil + } + defaultImportConstraint = func(_ string) (string, error) { return "^1.4", nil } + syncImports = func(_ string, _ map[string]config.Import, _ packman.InstallMode) (*packman.Lockfile, error) { + return &packman.Lockfile{ + Schema: packman.LockfileSchema, + Packs: map[string]packman.LockedPack{ + "https://github.com/example/tools.git": {Version: "1.4.2", Commit: "abc123"}, + }, + }, nil + } + + var stdout, stderr bytes.Buffer + code := doImportAdd(fsys.OSFS{}, dir, "https://github.com/example/tools.git", "", "", &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, stderr = %s", code, stderr.String()) + } + + data, err := os.ReadFile(filepath.Join(dir, "pack.toml")) + if err != nil { + t.Fatalf("ReadFile(pack.toml): %v", err) + } + text := string(data) + if strings.Contains(text, "[agents]") { + t.Fatalf("pack.toml should be rewritten to canonical [agent_defaults]:\n%s", text) + } + for _, want := range []string{ + `[agent_defaults]`, + `default_sling_formula = "mol-legacy"`, + `append_fragments = ["legacy-fragment"]`, + `provider = "claude"`, + `scope = "city"`, + `install_agent_hooks = ["claude"]`, + `[imports.tools]`, + } { + if !strings.Contains(text, want) { + t.Fatalf("pack.toml missing %q:\n%s", want, text) + } + } + if !strings.Contains(stderr.String(), "[agents] is a deprecated compatibility alias for [agent_defaults]") { + t.Fatalf("expected alias warning, got stderr %q", stderr.String()) + } +} + +func TestDoImportAddMergesLegacyAgentsDefaultsWhenCanonicalAlsoPresent(t *testing.T) { + clearGCEnv(t) + dir := t.TempDir() + writeCityToml(t, dir, "[workspace]\nname = \"demo\"\n") + writePackToml(t, dir, `[pack] +name = "demo" +schema = 1 + +[agent_defaults] +default_sling_formula = "mol-canonical" +append_fragments = ["canonical-fragment"] +scope = "city" + +[agents] +default_sling_formula = "mol-legacy" +append_fragments = ["legacy-fragment"] +provider = "claude" +install_agent_hooks = ["claude"] +`) + + prevResolve := resolveImportVersion + prevConstraint := defaultImportConstraint + prevSync := syncImports + t.Cleanup(func() { + resolveImportVersion = prevResolve + defaultImportConstraint = prevConstraint + syncImports = prevSync + }) + resolveImportVersion = func(_, _ string) (packman.ResolvedVersion, error) { + return packman.ResolvedVersion{Version: "1.4.2", Commit: "abc123"}, nil + } + defaultImportConstraint = func(_ string) (string, error) { return "^1.4", nil } + syncImports = func(_ string, _ map[string]config.Import, _ packman.InstallMode) (*packman.Lockfile, error) { + return &packman.Lockfile{ + Schema: packman.LockfileSchema, + Packs: map[string]packman.LockedPack{ + "https://github.com/example/tools.git": {Version: "1.4.2", Commit: "abc123"}, + }, + }, nil + } + + var stdout, stderr bytes.Buffer + code := doImportAdd(fsys.OSFS{}, dir, "https://github.com/example/tools.git", "", "", &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, stderr = %s", code, stderr.String()) + } + + data, err := os.ReadFile(filepath.Join(dir, "pack.toml")) + if err != nil { + t.Fatalf("ReadFile(pack.toml): %v", err) + } + text := string(data) + if strings.Contains(text, "[agents]") { + t.Fatalf("pack.toml should be rewritten to canonical [agent_defaults]:\n%s", text) + } + for _, want := range []string{ + `[agent_defaults]`, + `default_sling_formula = "mol-canonical"`, + `append_fragments = ["canonical-fragment"]`, + `provider = "claude"`, + `scope = "city"`, + `install_agent_hooks = ["claude"]`, + `[imports.tools]`, + } { + if !strings.Contains(text, want) { + t.Fatalf("pack.toml missing %q:\n%s", want, text) + } + } + for _, unwanted := range []string{ + `default_sling_formula = "mol-legacy"`, + `append_fragments = ["legacy-fragment"]`, + } { + if strings.Contains(text, unwanted) { + t.Fatalf("pack.toml should preserve canonical overlap instead of %q:\n%s", unwanted, text) + } + } + if !strings.Contains(stderr.String(), "both [agent_defaults] and [agents] are present in pack.toml") { + t.Fatalf("expected mixed-table warning, got stderr %q", stderr.String()) + } + if !strings.Contains(stderr.String(), "preserved unsupported [agent_defaults] keys in pack.toml: provider, scope, install_agent_hooks") { + t.Fatalf("expected unsupported-keys warning, got stderr %q", stderr.String()) + } +} + +func TestDoImportAddPreservesUnsupportedZeroValueAgentDefaultsKeys(t *testing.T) { + clearGCEnv(t) + dir := t.TempDir() + writeCityToml(t, dir, "[workspace]\nname = \"demo\"\n") + writePackToml(t, dir, `[pack] +name = "demo" +schema = 1 + +[agents] +provider = "" +scope = "" +install_agent_hooks = [] +`) + + prevResolve := resolveImportVersion + prevConstraint := defaultImportConstraint + prevSync := syncImports + t.Cleanup(func() { + resolveImportVersion = prevResolve + defaultImportConstraint = prevConstraint + syncImports = prevSync + }) + resolveImportVersion = func(_, _ string) (packman.ResolvedVersion, error) { + return packman.ResolvedVersion{Version: "1.4.2", Commit: "abc123"}, nil + } + defaultImportConstraint = func(_ string) (string, error) { return "^1.4", nil } + syncImports = func(_ string, _ map[string]config.Import, _ packman.InstallMode) (*packman.Lockfile, error) { + return &packman.Lockfile{ + Schema: packman.LockfileSchema, + Packs: map[string]packman.LockedPack{ + "https://github.com/example/tools.git": {Version: "1.4.2", Commit: "abc123"}, + }, + }, nil + } + + var stdout, stderr bytes.Buffer + code := doImportAdd(fsys.OSFS{}, dir, "https://github.com/example/tools.git", "", "", &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, stderr = %s", code, stderr.String()) + } + + data, err := os.ReadFile(filepath.Join(dir, "pack.toml")) + if err != nil { + t.Fatalf("ReadFile(pack.toml): %v", err) + } + text := string(data) + for _, want := range []string{ + `[agent_defaults]`, + `provider = ""`, + `scope = ""`, + `install_agent_hooks = []`, + `[imports.tools]`, + } { + if !strings.Contains(text, want) { + t.Fatalf("pack.toml missing %q:\n%s", want, text) + } + } + if !strings.Contains(stderr.String(), "[agents] is a deprecated compatibility alias for [agent_defaults]") { + t.Fatalf("expected alias warning, got stderr %q", stderr.String()) + } + if !strings.Contains(stderr.String(), "preserved unsupported [agent_defaults] keys in pack.toml: provider, scope, install_agent_hooks") { + t.Fatalf("expected unsupported-keys warning, got stderr %q", stderr.String()) + } +} + func TestDoImportAddPathRejectsVersionFlag(t *testing.T) { clearGCEnv(t) dir := t.TempDir() @@ -418,6 +670,44 @@ func TestDoImportInstallWithNoImportsSucceeds(t *testing.T) { } } +func TestDoImportInstallWarnsOnLegacyAgentDefaultsAlias(t *testing.T) { + clearGCEnv(t) + dir := t.TempDir() + writeCityToml(t, dir, "[workspace]\nname = \"demo\"\n") + writePackToml(t, dir, `[pack] +name = "demo" +schema = 1 + +[agents] +provider = "claude" +`) + if err := writeImportLockfile(fsys.OSFS{}, dir, &packman.Lockfile{Schema: packman.LockfileSchema, Packs: map[string]packman.LockedPack{}}); err != nil { + t.Fatalf("writeImportLockfile: %v", err) + } + + prevSync := syncImports + prevInstall := installLockedImports + t.Cleanup(func() { + syncImports = prevSync + installLockedImports = prevInstall + }) + syncImports = func(_ string, _ map[string]config.Import, _ packman.InstallMode) (*packman.Lockfile, error) { + return &packman.Lockfile{Schema: packman.LockfileSchema, Packs: map[string]packman.LockedPack{}}, nil + } + installLockedImports = func(_ string) (*packman.Lockfile, error) { + return &packman.Lockfile{Schema: packman.LockfileSchema, Packs: map[string]packman.LockedPack{}}, nil + } + + var stdout, stderr bytes.Buffer + code := doImportInstall(dir, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, stderr = %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "[agents] is a deprecated compatibility alias for [agent_defaults]; rewrite pack.toml to canonical [agent_defaults]") { + t.Fatalf("expected alias guidance, got stderr %q", stderr.String()) + } +} + func TestDoImportUpgradeTargetedMergesPreservedImports(t *testing.T) { clearGCEnv(t) dir := t.TempDir() @@ -490,6 +780,43 @@ source = "../packs/local" } } +func TestDoImportUpgradeWarnsOnLegacyAgentDefaultsAlias(t *testing.T) { + clearGCEnv(t) + dir := t.TempDir() + writeCityToml(t, dir, "[workspace]\nname = \"demo\"\n") + writePackToml(t, dir, `[pack] +name = "demo" +schema = 1 + +[agents] +provider = "claude" + +[imports.a] +source = "https://example.com/a.git" +version = "^1.0" +`) + + prevSync := syncImports + t.Cleanup(func() { syncImports = prevSync }) + syncImports = func(_ string, imports map[string]config.Import, _ packman.InstallMode) (*packman.Lockfile, error) { + return &packman.Lockfile{ + Schema: packman.LockfileSchema, + Packs: map[string]packman.LockedPack{ + imports["a"].Source: {Version: "1.1.0", Commit: "abc123"}, + }, + }, nil + } + + var stdout, stderr bytes.Buffer + code := doImportUpgrade(dir, "a", &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, stderr = %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "[agents] is a deprecated compatibility alias for [agent_defaults]; rewrite pack.toml to canonical [agent_defaults]") { + t.Fatalf("expected alias guidance, got stderr %q", stderr.String()) + } +} + func TestDoImportListShowsDirectAndTransitive(t *testing.T) { clearGCEnv(t) dir := t.TempDir() @@ -547,6 +874,31 @@ source = "../packs/local" } } +func TestDoImportListWarnsOnLegacyAgentDefaultsAlias(t *testing.T) { + clearGCEnv(t) + dir := t.TempDir() + writeCityToml(t, dir, "[workspace]\nname = \"demo\"\n") + writePackToml(t, dir, `[pack] +name = "demo" +schema = 1 + +[agents] +provider = "claude" +`) + if err := writeImportLockfile(fsys.OSFS{}, dir, &packman.Lockfile{Schema: packman.LockfileSchema, Packs: map[string]packman.LockedPack{}}); err != nil { + t.Fatalf("writeImportLockfile: %v", err) + } + + var stdout, stderr bytes.Buffer + code := doImportList(dir, false, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, stderr = %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "[agents] is a deprecated compatibility alias for [agent_defaults]; rewrite pack.toml to canonical [agent_defaults]") { + t.Fatalf("expected alias guidance, got stderr %q", stderr.String()) + } +} + func TestDoImportListTreeShowsDependencyGraph(t *testing.T) { clearGCEnv(t) dir := t.TempDir() diff --git a/cmd/gc/cmd_init.go b/cmd/gc/cmd_init.go index 0b38d10b9..fd5ecc25a 100644 --- a/cmd/gc/cmd_init.go +++ b/cmd/gc/cmd_init.go @@ -29,12 +29,13 @@ const initPackSchemaVersion = 2 const initExitAlreadyInitialized = 2 type initPackMeta struct { - Name string `toml:"name"` - Version string `toml:"version,omitempty"` - Schema int `toml:"schema"` - RequiresGC string `toml:"requires_gc,omitempty"` - Includes []string `toml:"includes,omitempty"` - Requires []config.PackRequirement `toml:"requires,omitempty"` + Name string `toml:"name" jsonschema:"required"` + Version string `toml:"version"` + Schema int `toml:"schema" jsonschema:"required"` + Description string `toml:"description,omitempty"` + RequiresGC string `toml:"requires_gc,omitempty"` + Includes []string `toml:"includes,omitempty"` + Requires []config.PackRequirement `toml:"requires,omitempty"` } type packDefaults struct { @@ -49,19 +50,20 @@ type initPackConfig struct { // Keep this layout in lockstep with internal/config.packConfig so // pack.toml write paths in cmd/gc can round-trip the canonical root // pack shape without dropping supported fields. - Pack initPackMeta `toml:"pack"` - Imports map[string]config.Import `toml:"imports,omitempty"` - AgentDefaults config.AgentDefaults `toml:"agent_defaults,omitempty"` - Defaults packDefaults `toml:"defaults,omitempty"` - Agents []config.Agent `toml:"agent"` - NamedSessions []config.NamedSession `toml:"named_session,omitempty"` - Services []config.Service `toml:"service,omitempty"` - Providers map[string]config.ProviderSpec `toml:"providers,omitempty"` - Formulas config.FormulasConfig `toml:"formulas,omitempty"` - Patches config.Patches `toml:"patches,omitempty"` - Doctor []config.PackDoctorEntry `toml:"doctor,omitempty"` - Commands []config.PackCommandEntry `toml:"commands,omitempty"` - Global config.PackGlobal `toml:"global,omitempty"` + Pack initPackMeta `toml:"pack"` + Imports map[string]config.Import `toml:"imports,omitempty"` + AgentDefaults config.AgentDefaults `toml:"agent_defaults,omitempty"` + AgentsDefaults config.AgentDefaults `toml:"agents,omitempty" jsonschema:"-"` + Defaults packDefaults `toml:"defaults,omitempty"` + Agents []config.Agent `toml:"agent"` + NamedSessions []config.NamedSession `toml:"named_session,omitempty"` + Services []config.Service `toml:"service,omitempty"` + Providers map[string]config.ProviderSpec `toml:"providers,omitempty"` + Formulas config.FormulasConfig `toml:"formulas,omitempty"` + Patches config.Patches `toml:"patches,omitempty"` + Doctor []config.PackDoctorEntry `toml:"doctor,omitempty"` + Commands []config.PackCommandEntry `toml:"commands,omitempty"` + Global config.PackGlobal `toml:"global,omitempty"` } var initConventionDirs = []string{ @@ -456,8 +458,17 @@ func writeInitPackToml(fs fsys.FS, cityPath string, packCfg initPackConfig) erro } func marshalInitPackConfig(cfg initPackConfig) ([]byte, error) { + type encodedInitPackMeta struct { + Name string `toml:"name" jsonschema:"required"` + Version string `toml:"version,omitempty"` + Schema int `toml:"schema" jsonschema:"required"` + Description string `toml:"description,omitempty"` + RequiresGC string `toml:"requires_gc,omitempty"` + Includes []string `toml:"includes,omitempty"` + Requires []config.PackRequirement `toml:"requires,omitempty"` + } type encodedInitPackConfig struct { - Pack initPackMeta `toml:"pack"` + Pack encodedInitPackMeta `toml:"pack"` Imports map[string]config.Import `toml:"imports,omitempty"` AgentDefaults *config.AgentDefaults `toml:"agent_defaults,omitempty"` Defaults packDefaults `toml:"defaults,omitempty"` @@ -473,7 +484,15 @@ func marshalInitPackConfig(cfg initPackConfig) ([]byte, error) { } encCfg := encodedInitPackConfig{ - Pack: cfg.Pack, + Pack: encodedInitPackMeta{ + Name: cfg.Pack.Name, + Version: cfg.Pack.Version, + Schema: cfg.Pack.Schema, + Description: cfg.Pack.Description, + RequiresGC: cfg.Pack.RequiresGC, + Includes: cfg.Pack.Includes, + Requires: cfg.Pack.Requires, + }, Imports: cfg.Imports, Defaults: cfg.Defaults, Agents: cfg.Agents, @@ -1134,7 +1153,10 @@ func doInitFromDirWithOptions(srcDir, cityPath, nameOverride string, stdout, std } // Resolve formulas and scripts from pack layers. - expandedCfg, _, loadErr := config.LoadWithIncludes(fsys.OSFS{}, copiedToml) + expandedCfg, prov, loadErr := config.LoadWithIncludes(fsys.OSFS{}, copiedToml) + if loadErr == nil { + emitLoadCityConfigWarnings(stderr, prov) + } if loadErr == nil && len(expandedCfg.FormulaLayers.City) > 0 { if rfErr := ResolveFormulas(cityPath, expandedCfg.FormulaLayers.City); rfErr != nil { fmt.Fprintf(stderr, "gc init: resolving formulas: %v\n", rfErr) //nolint:errcheck // best-effort stderr diff --git a/cmd/gc/cmd_internal_materialize_skills.go b/cmd/gc/cmd_internal_materialize_skills.go index 85c254515..3e6323c6e 100644 --- a/cmd/gc/cmd_internal_materialize_skills.go +++ b/cmd/gc/cmd_internal_materialize_skills.go @@ -55,7 +55,7 @@ func newInternalMaterializeSkillsCmd(stdout, stderr io.Writer) *cobra.Command { fmt.Fprintf(stderr, "gc internal materialize-skills: %v\n", err) //nolint:errcheck // best-effort stderr return errExit } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc internal materialize-skills: %v\n", err) //nolint:errcheck // best-effort stderr return errExit diff --git a/cmd/gc/cmd_internal_project_mcp.go b/cmd/gc/cmd_internal_project_mcp.go index 6c82380ab..c0d7408ac 100644 --- a/cmd/gc/cmd_internal_project_mcp.go +++ b/cmd/gc/cmd_internal_project_mcp.go @@ -34,7 +34,7 @@ func newInternalProjectMCPCmd(stdout, stderr io.Writer) *cobra.Command { fmt.Fprintf(stderr, "gc internal project-mcp: %v\n", err) //nolint:errcheck // best-effort stderr return errExit } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc internal project-mcp: %v\n", err) //nolint:errcheck // best-effort stderr return errExit diff --git a/cmd/gc/cmd_mail.go b/cmd/gc/cmd_mail.go index d02a6d0a9..ae24928c3 100644 --- a/cmd/gc/cmd_mail.go +++ b/cmd/gc/cmd_mail.go @@ -28,7 +28,7 @@ type nudgeFunc func(recipient string) error func newMailNudgeFunc(sender string) nudgeFunc { return func(recipient string) error { - target, err := resolveNudgeTarget(recipient) + target, err := resolveNudgeTarget(recipient, io.Discard) if err != nil { return err } @@ -160,7 +160,7 @@ $GC_SESSION_ID, or "human".`, func cmdMailCheckWithFormat(args []string, inject bool, hookFormat string, stdout, stderr io.Writer) int { // Check city-level suspension before opening the store. if cityPath, err := resolveCity(); err == nil { - if cfg, err := loadCityConfig(cityPath); err == nil { + if cfg, err := loadCityConfig(cityPath, stderr); err == nil { if citySuspended(cfg) { if inject { return 0 @@ -414,7 +414,7 @@ func configuredMailboxAddress(identifier string) (string, bool) { if err != nil { return "", false } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return "", false } @@ -925,7 +925,7 @@ func cmdMailSend(args []string, notify bool, all bool, from string, to string, s ) cityPath, err := resolveCity() if err == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) store, err = openCityStoreAt(cityPath) } // Narrower than isStorelessMailProvider: exec: providers can legitimately @@ -1234,7 +1234,7 @@ func cmdMailReply(args []string, subject, message string, notify bool, stdout, s fmt.Fprintf(stderr, "gc mail reply: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, _ := loadCityConfig(cityPath) + cfg, _ := loadCityConfig(cityPath, stderr) resolved, ok := resolveDefaultMailSenderForCommand(cityPath, cfg, store, stderr, "gc mail reply") if !ok { return 1 diff --git a/cmd/gc/cmd_mcp.go b/cmd/gc/cmd_mcp.go index 1e849b80d..c4b8c49c3 100644 --- a/cmd/gc/cmd_mcp.go +++ b/cmd/gc/cmd_mcp.go @@ -62,7 +62,7 @@ func newMcpListCmd(stdout, stderr io.Writer) *cobra.Command { fmt.Fprintf(stderr, "gc mcp list: %v\n", err) //nolint:errcheck // best-effort stderr return errExit } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc mcp list: %v\n", err) //nolint:errcheck // best-effort stderr return errExit diff --git a/cmd/gc/cmd_nudge.go b/cmd/gc/cmd_nudge.go index fa7056ed5..69a56d8f8 100644 --- a/cmd/gc/cmd_nudge.go +++ b/cmd/gc/cmd_nudge.go @@ -237,7 +237,7 @@ func cmdNudgeStatus(args []string, stdout, stderr io.Writer) int { return 1 } - target, err := resolveNudgeTarget(targetID) + target, err := resolveNudgeTarget(targetID, stderr) if err != nil { fmt.Fprintf(stderr, "gc nudge status: %v\n", err) //nolint:errcheck return 1 @@ -295,7 +295,7 @@ func cmdNudgeDrainWithFormat(args []string, inject bool, hookFormat string, stdo return 1 } - target, err := resolveNudgeTarget(targetID) + target, err := resolveNudgeTarget(targetID, stderr) if err != nil { if inject { return 0 @@ -402,7 +402,7 @@ func cmdNudgePoll(args []string, sessionName string, interval, quiescence time.D fmt.Fprintln(stderr, "gc nudge poll: session not specified (set $GC_ALIAS/$GC_SESSION_ID or pass an alias/id)") //nolint:errcheck return 1 } - target, err := resolveNudgeTarget(targetID) + target, err := resolveNudgeTarget(targetID, stderr) if err != nil { fmt.Fprintf(stderr, "gc nudge poll: %v\n", err) //nolint:errcheck return 1 @@ -597,12 +597,12 @@ func sendMailNotifyWithWorker(target nudgeTarget, store beads.Store, sp runtime. return nil } -func resolveNudgeTarget(identifier string) (nudgeTarget, error) { +func resolveNudgeTarget(identifier string, warningWriter ...io.Writer) (nudgeTarget, error) { cityPath, err := resolveCity() if err != nil { return nudgeTarget{}, err } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, warningWriter...) if err != nil { return nudgeTarget{}, err } diff --git a/cmd/gc/cmd_order.go b/cmd/gc/cmd_order.go index e1003803c..97f87685a 100644 --- a/cmd/gc/cmd_order.go +++ b/cmd/gc/cmd_order.go @@ -168,7 +168,7 @@ func loadOrdersWithCity(stderr io.Writer, cmdName string) (string, *config.City, fmt.Fprintf(stderr, "%s: %v\n", cmdName, err) //nolint:errcheck // best-effort stderr return "", nil, nil, 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "%s: %v\n", cmdName, err) //nolint:errcheck // best-effort stderr return "", nil, nil, 1 @@ -473,7 +473,7 @@ func doOrderRun(aa []orders.Order, name, rig, cityPath string, store beads.Store // Exec orders: run the script directly. if a.IsExec() { - cfg, cfgErr := loadCityConfig(cityPath) + cfg, cfgErr := loadCityConfig(cityPath, stderr) if cfgErr != nil { fmt.Fprintf(stderr, "gc order run: %v\n", cfgErr) //nolint:errcheck // best-effort stderr return 1 @@ -492,7 +492,7 @@ func doOrderRun(aa []orders.Order, name, rig, cityPath string, store beads.Store var cityName string if citylayout.HasCityConfig(cityPath) || citylayout.HasRuntimeRoot(cityPath) { var err error - cfg, err = loadCityConfig(cityPath) + cfg, err = loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc order run: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_pack.go b/cmd/gc/cmd_pack.go index 421242428..4f39e5ca4 100644 --- a/cmd/gc/cmd_pack.go +++ b/cmd/gc/cmd_pack.go @@ -56,7 +56,7 @@ func doPackFetch(stdout, stderr io.Writer) int { return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc pack fetch: %v\n", err) //nolint:errcheck return 1 @@ -122,7 +122,7 @@ func doPackList(stdout, stderr io.Writer) int { return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc pack list: %v\n", err) //nolint:errcheck return 1 diff --git a/cmd/gc/cmd_pack_commands.go b/cmd/gc/cmd_pack_commands.go index d6dfc95c6..08f757b28 100644 --- a/cmd/gc/cmd_pack_commands.go +++ b/cmd/gc/cmd_pack_commands.go @@ -21,7 +21,7 @@ func quietLoadCityConfig(cityPath string) (*config.City, error) { prev := log.Writer() log.SetOutput(io.Discard) defer log.SetOutput(prev) - return loadCityConfig(cityPath) + return loadCityConfig(cityPath, io.Discard) } // registerPackCommands attempts to discover the city, load config, and diff --git a/cmd/gc/cmd_prime.go b/cmd/gc/cmd_prime.go index f12843019..9efb14619 100644 --- a/cmd/gc/cmd_prime.go +++ b/cmd/gc/cmd_prime.go @@ -123,7 +123,7 @@ func doPrimeWithHookFormat(args []string, stdout, stderr io.Writer, hookMode boo fmt.Fprint(stdout, defaultPrimePrompt) //nolint:errcheck // best-effort stdout return 0 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprint(stdout, defaultPrimePrompt) //nolint:errcheck // best-effort stdout return 0 @@ -178,7 +178,12 @@ func doPrimeWithHookFormat(args []string, stdout, stderr io.Writer, hookMode boo ctx = buildPrimeContext(cityPath, cityName, &a, cfg.Rigs, stderr) } if ok && a.PromptTemplate != "" { - fragments := mergeFragmentLists(cfg.Workspace.GlobalFragments, a.InjectFragments) + fragments := effectivePromptFragments( + cfg.Workspace.GlobalFragments, + a.InjectFragments, + a.InheritedAppendFragments, + cfg.AgentDefaults.AppendFragments, + ) prompt := renderPrompt(fsys.OSFS{}, cityPath, cityName, a.PromptTemplate, ctx, cfg.Workspace.SessionTemplate, stderr, cfg.PackDirs, fragments, nil) if prompt != "" { diff --git a/cmd/gc/cmd_register.go b/cmd/gc/cmd_register.go index 97400d07a..5c01dec79 100644 --- a/cmd/gc/cmd_register.go +++ b/cmd/gc/cmd_register.go @@ -162,10 +162,11 @@ func newCitiesCmd(stdout, stderr io.Writer) *cobra.Command { RunE: runList, } cmd.AddCommand(&cobra.Command{ - Use: "list", - Short: "List registered cities", - Args: cobra.NoArgs, - RunE: runList, + Use: "list", + Short: "List registered cities", + Aliases: []string{"ls"}, + Args: cobra.NoArgs, + RunE: runList, }) return cmd } diff --git a/cmd/gc/cmd_restart.go b/cmd/gc/cmd_restart.go index af65faefd..a7832a5ee 100644 --- a/cmd/gc/cmd_restart.go +++ b/cmd/gc/cmd_restart.go @@ -97,7 +97,7 @@ func cmdRigRestart(args []string, stdout, stderr io.Writer) int { return 1 } cityPath := ctx.CityPath - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc rig restart: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_rig.go b/cmd/gc/cmd_rig.go index da0bf93ac..4db04a175 100644 --- a/cmd/gc/cmd_rig.go +++ b/cmd/gc/cmd_rig.go @@ -414,7 +414,8 @@ func doRigAdd(fs fsys.FS, cityPath, rigPath string, includes []string, nameOverr } } - reloadedCfg, _, _ := config.LoadWithIncludes(fsys.OSFS{}, tomlPath) + reloadedCfg, prov, _ := config.LoadWithIncludes(fsys.OSFS{}, tomlPath) + emitLoadCityConfigWarnings(stderr, prov) if reloadedCfg != nil { layers, ok := reloadedCfg.FormulaLayers.Rigs[name] if !ok || len(layers) == 0 { @@ -610,7 +611,7 @@ type RigListItem struct { // how the rig path is declared in city.toml. The cityPath parameter must be // absolute. func doRigList(fs fsys.FS, cityPath string, jsonOutput bool, stdout, stderr io.Writer) int { - cfg, err := loadCityConfigFS(fs, filepath.Join(cityPath, "city.toml")) + cfg, err := loadCityConfigFS(fs, filepath.Join(cityPath, "city.toml"), stderr) if err != nil { fmt.Fprintf(stderr, "gc rig list: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -935,7 +936,7 @@ func cmdRigRemove(rigName string, stdout, stderr io.Writer) int { // Update .beads/.env and routes for the rig's new default city. if newDefault != "" { _ = writeBeadsEnvGTRoot(fsys.OSFS{}, removedPath, newDefault) - if newCfg, err := loadCityConfigSuppressDeprecatedOrderWarnings(newDefault); err == nil { + if newCfg, err := loadCityConfigSuppressDeprecatedOrderWarnings(newDefault, io.Discard); err == nil { resolveRigPaths(newDefault, newCfg.Rigs) newRigs := collectRigRoutes(newDefault, newCfg) _ = writeAllRoutes(newRigs) @@ -1012,7 +1013,7 @@ func cmdRigDefault(rigNameOrPath, cityNameOrPath string, stdout, stderr io.Write } // Validate rig belongs to this city. - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc rig default: loading city config: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_rig_test.go b/cmd/gc/cmd_rig_test.go index 46601190f..5fc7aeadd 100644 --- a/cmd/gc/cmd_rig_test.go +++ b/cmd/gc/cmd_rig_test.go @@ -2408,3 +2408,40 @@ func TestCmdRigAddStoresMachinePathInSiteBindingWhenOutsideCity(t *testing.T) { t.Fatalf("site binding = %+v, want frontend=%s", binding.Rigs, wantRigPath) } } + +func TestDoRigAddEmitsLoadWarningsFromReloadedConfig(t *testing.T) { + cityPath := t.TempDir() + if err := os.MkdirAll(filepath.Join(cityPath, ".gc"), 0o755); err != nil { + t.Fatal(err) + } + cityToml := `[workspace] +name = "test-city" + +[agent_defaults] +skills = ["demo"] + +[[agent]] +name = "mayor" +` + if err := os.WriteFile(filepath.Join(cityPath, "city.toml"), []byte(cityToml), 0o644); err != nil { + t.Fatal(err) + } + + rigPath := filepath.Join(t.TempDir(), "frontend") + if err := os.MkdirAll(rigPath, 0o755); err != nil { + t.Fatal(err) + } + + t.Setenv("GC_DOLT", "skip") + t.Setenv("GC_BEADS", "file") + isolateRigRegistryEnv(t) + + var stdout, stderr bytes.Buffer + code := doRigAdd(fsys.OSFS{}, cityPath, rigPath, nil, "", "", false, false, &stdout, &stderr) + if code != 0 { + t.Fatalf("doRigAdd returned %d, stderr: %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "attachment-list fields") { + t.Fatalf("stderr = %q, want deprecated attachment-list warning", stderr.String()) + } +} diff --git a/cmd/gc/cmd_runtime_drain.go b/cmd/gc/cmd_runtime_drain.go index 7f1aa5f3d..f3bd0c49b 100644 --- a/cmd/gc/cmd_runtime_drain.go +++ b/cmd/gc/cmd_runtime_drain.go @@ -155,7 +155,7 @@ func cmdRuntimeDrain(args []string, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "gc runtime drain: missing session alias or ID") //nolint:errcheck // best-effort stderr return 1 } - target, err := resolveSessionRuntimeTarget(args[0]) + target, err := resolveSessionRuntimeTarget(args[0], stderr) if err != nil { fmt.Fprintf(stderr, "gc runtime drain: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -219,7 +219,7 @@ func cmdRuntimeUndrain(args []string, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "gc runtime undrain: missing session alias or ID") //nolint:errcheck // best-effort stderr return 1 } - target, err := resolveSessionRuntimeTarget(args[0]) + target, err := resolveSessionRuntimeTarget(args[0], stderr) if err != nil { fmt.Fprintf(stderr, "gc runtime undrain: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -282,7 +282,7 @@ arguments, uses the current session context.`, func cmdRuntimeDrainCheck(args []string, stderr io.Writer) int { if len(args) > 0 { - target, err := resolveSessionRuntimeTarget(args[0]) + target, err := resolveSessionRuntimeTarget(args[0], stderr) if err != nil { fmt.Fprintf(stderr, "gc runtime drain-check: %v\n", err) //nolint:errcheck // best-effort stderr return 1 // silent — same as current "not draining" behavior @@ -336,7 +336,7 @@ finished its current work in response to a drain signal.`, func cmdRuntimeDrainAck(args []string, stdout, stderr io.Writer) int { if len(args) > 0 { - target, err := resolveSessionRuntimeTarget(args[0]) + target, err := resolveSessionRuntimeTarget(args[0], stderr) if err != nil { fmt.Fprintf(stderr, "gc runtime drain-ack: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -417,7 +417,7 @@ func cmdRuntimeRequestRestart(stdout, stderr io.Writer) int { } } rec := openCityRecorderAt(current.cityPath, stderr) - cfg, _ := loadCityConfig(current.cityPath) + cfg, _ := loadCityConfig(current.cityPath, stderr) var persistRestart func() error if store != nil { persistRestart = func() error { diff --git a/cmd/gc/cmd_service.go b/cmd/gc/cmd_service.go index 1f7ef1ed2..ca7b2d20f 100644 --- a/cmd/gc/cmd_service.go +++ b/cmd/gc/cmd_service.go @@ -74,7 +74,7 @@ func cmdServiceList(stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc service list: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc service list: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -88,7 +88,7 @@ func cmdServiceDoctor(name string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc service doctor: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc service doctor: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -120,7 +120,7 @@ func cmdServiceRestart(name string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc service restart: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc service restart: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_session.go b/cmd/gc/cmd_session.go index 35e9b3269..4ad9d6157 100644 --- a/cmd/gc/cmd_session.go +++ b/cmd/gc/cmd_session.go @@ -151,7 +151,7 @@ func cmdSessionNew(args []string, alias, title, titleHint string, noAttach bool, fmt.Fprintf(stderr, "gc session new: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc session new: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -819,7 +819,7 @@ func cmdSessionAttach(args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc session attach: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc session attach: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -985,7 +985,7 @@ func cmdSessionSuspend(args []string, stdout, stderr io.Writer) int { cityPath, cityErr := resolveCity() var cfg *config.City if cityErr == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } sessionID, err := resolveSessionIDWithConfig(cityPath, cfg, store, args[0]) if err != nil { @@ -1061,7 +1061,7 @@ func cmdSessionClose(args []string, stdout, stderr io.Writer) int { cityPath, cityErr := resolveCity() var cfg *config.City if cityErr == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } sessionID, err := resolveSessionIDWithConfig(cityPath, cfg, store, args[0]) if err != nil { @@ -1121,7 +1121,7 @@ func cmdSessionRename(args []string, stdout, stderr io.Writer) int { cityPath, err := resolveCity() var cfg *config.City if err == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } sessionID, err := resolveSessionIDWithConfig(cityPath, cfg, store, args[0]) if err != nil { @@ -1262,7 +1262,7 @@ func cmdSessionPeek(args []string, lines int, stdout, stderr io.Writer) int { cityPath, err := resolveCity() var cfg *config.City if err == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } sessionID, err := resolveSessionIDWithConfig(cityPath, cfg, store, args[0]) if err != nil { @@ -1321,7 +1321,7 @@ func cmdSessionKill(args []string, stdout, stderr io.Writer) int { cityPath, err := resolveCity() var cfg *config.City if err == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } sessionID, err := resolveSessionIDWithConfig(cityPath, cfg, store, args[0]) if err != nil { @@ -1420,7 +1420,7 @@ func cmdSessionSubmit(args []string, intent session.SubmitIntent, stdout, stderr } } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc session submit: %v\n", err) //nolint:errcheck // best-effort stderr return 1 @@ -1472,7 +1472,7 @@ func cmdSessionNudge(args []string, delivery nudgeDeliveryMode, stdout, stderr i target := args[0] message := strings.Join(args[1:], " ") - targetInfo, err := resolveNudgeTarget(target) + targetInfo, err := resolveNudgeTarget(target, stderr) if err != nil { fmt.Fprintf(stderr, "gc session nudge: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_session_logs.go b/cmd/gc/cmd_session_logs.go index 5b4526271..2b667db47 100644 --- a/cmd/gc/cmd_session_logs.go +++ b/cmd/gc/cmd_session_logs.go @@ -56,7 +56,7 @@ func cmdSessionLogs(args []string, follow bool, tail int, stdout, stderr io.Writ fmt.Fprintf(stderr, "gc session logs: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc session logs: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_session_pin.go b/cmd/gc/cmd_session_pin.go index 9d06ab8d3..4d67cc47a 100644 --- a/cmd/gc/cmd_session_pin.go +++ b/cmd/gc/cmd_session_pin.go @@ -67,7 +67,7 @@ func cmdSessionSetPin(args []string, pinned bool, stdout, stderr io.Writer) int cityPath, cityErr := resolveCity() var cfg *config.City if cityErr == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } var ( diff --git a/cmd/gc/cmd_session_reset.go b/cmd/gc/cmd_session_reset.go index c25c25269..18899c15c 100644 --- a/cmd/gc/cmd_session_reset.go +++ b/cmd/gc/cmd_session_reset.go @@ -55,7 +55,7 @@ func cmdSessionReset(args []string, stdout, stderr io.Writer) int { return 1 } - cfg, _ := loadCityConfig(cityPath) + cfg, _ := loadCityConfig(cityPath, stderr) sessionID, err := resolveSessionIDWithConfig(cityPath, cfg, store, args[0]) if err != nil { diff --git a/cmd/gc/cmd_session_wake.go b/cmd/gc/cmd_session_wake.go index b76d93219..da32fda50 100644 --- a/cmd/gc/cmd_session_wake.go +++ b/cmd/gc/cmd_session_wake.go @@ -44,7 +44,7 @@ func cmdSessionWake(args []string, stdout, stderr io.Writer) int { cityPath, cityErr := resolveCity() var cfg *config.City if cityErr == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } id, err := resolveSessionIDMaterializingNamed(cityPath, cfg, store, args[0]) if err != nil { diff --git a/cmd/gc/cmd_skill.go b/cmd/gc/cmd_skill.go index 78962911f..d26fe97df 100644 --- a/cmd/gc/cmd_skill.go +++ b/cmd/gc/cmd_skill.go @@ -65,7 +65,7 @@ func newSkillListCmd(stdout, stderr io.Writer) *cobra.Command { fmt.Fprintf(stderr, "gc skill list: %v\n", err) //nolint:errcheck // best-effort stderr return errExit } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc skill list: %v\n", err) //nolint:errcheck // best-effort stderr return errExit diff --git a/cmd/gc/cmd_sling.go b/cmd/gc/cmd_sling.go index 763cc2eb3..1b8ffa4cc 100644 --- a/cmd/gc/cmd_sling.go +++ b/cmd/gc/cmd_sling.go @@ -189,11 +189,12 @@ func cmdSling(args []string, isFormula, doNudge, force bool, title string, vars fmt.Fprintf(stderr, "gc sling: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) if err != nil { fmt.Fprintf(stderr, "gc sling: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } + emitLoadCityConfigWarnings(stderr, prov) applyFeatureFlags(cfg) var target, beadOrFormula string diff --git a/cmd/gc/cmd_start.go b/cmd/gc/cmd_start.go index 466e5e395..c7ee44edd 100644 --- a/cmd/gc/cmd_start.go +++ b/cmd/gc/cmd_start.go @@ -399,9 +399,12 @@ func doStartStandalone(args []string, controllerMode bool, stdout, stderr io.Wri return 1 } applyFeatureFlags(cfg) - // Strict mode (default) promotes composition warnings to errors. - if strictMode && len(prov.Warnings) > 0 { - for _, w := range prov.Warnings { + // Strict mode keeps composition collisions and mixed canonical/compat + // default tables fatal. Alias-only, unsupported-key, and deprecation + // migration warnings remain soft so legacy compatibility paths still boot. + if fatalWarnings := strictFatalLoadConfigWarnings(prov.Warnings); strictMode && len(fatalWarnings) > 0 { + emitNonFatalLoadConfigWarnings(stderr, prov) + for _, w := range fatalWarnings { fmt.Fprintf(stderr, "gc start: strict: %s\n", w) //nolint:errcheck // best-effort stderr } fmt.Fprintln(stderr, "gc start: use --no-strict to disable strict checking") //nolint:errcheck // best-effort stderr diff --git a/cmd/gc/cmd_status.go b/cmd/gc/cmd_status.go index 7fac1f92a..f63f40328 100644 --- a/cmd/gc/cmd_status.go +++ b/cmd/gc/cmd_status.go @@ -46,7 +46,7 @@ func cmdRigStatus(args []string, stdout, stderr io.Writer) int { return 1 } cityPath := ctx.CityPath - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc rig status: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_stop.go b/cmd/gc/cmd_stop.go index a097e9278..fc5988e26 100644 --- a/cmd/gc/cmd_stop.go +++ b/cmd/gc/cmd_stop.go @@ -49,7 +49,7 @@ func cmdStop(args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gc stop: %v\n", err) //nolint:errcheck // best-effort stderr return 1 } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { fmt.Fprintf(stderr, "gc stop: %v\n", err) //nolint:errcheck // best-effort stderr return 1 diff --git a/cmd/gc/cmd_supervisor.go b/cmd/gc/cmd_supervisor.go index 02ca5c7d6..47f7b6ba9 100644 --- a/cmd/gc/cmd_supervisor.go +++ b/cmd/gc/cmd_supervisor.go @@ -14,6 +14,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "sync/atomic" "syscall" "time" @@ -1104,6 +1105,7 @@ func reconcileCities( recordInitFailure(name, loadErr.Error()) continue } + emitSupervisorLoadCityConfigWarnings(stderr, path, prov) // Use registered name as authoritative identity. city.toml may keep a // different workspace.name because registration aliases are machine-local. @@ -1521,6 +1523,29 @@ func reconcileCities( reconcileRigIndex(reg, stderr) } +var supervisorLoadWarningSeen sync.Map + +func emitSupervisorLoadCityConfigWarnings(w io.Writer, cityPath string, prov *config.Provenance) { + if w == nil || prov == nil || len(prov.Warnings) == 0 { + return + } + seen := make(map[string]struct{}, len(prov.Warnings)) + for _, warning := range prov.Warnings { + if !shouldEmitLoadCityConfigWarning(warning) { + continue + } + if _, dup := seen[warning]; dup { + continue + } + seen[warning] = struct{}{} + key := filepath.Clean(cityPath) + "\x00" + warning + if _, loaded := supervisorLoadWarningSeen.LoadOrStore(key, struct{}{}); loaded { + continue + } + fmt.Fprintln(w, warning) //nolint:errcheck // best-effort warning emission + } +} + // reconcileRigIndex rebuilds the [[rigs]] section of cities.toml from the // rig definitions in each registered city's city.toml. func reconcileRigIndex(reg *supervisor.Registry, stderr io.Writer) { @@ -1532,7 +1557,9 @@ func reconcileRigIndex(reg *supervisor.Registry, stderr io.Writer) { var mappings []supervisor.RigCityMapping var loadFailed bool for _, c := range cities { - cfg, err := loadCityConfigSuppressDeprecatedOrderWarnings(c.Path) + // Rig-index patrol runs continuously under the supervisor. Suppress + // migration warnings here so steady-state reconciles do not flood logs. + cfg, err := loadCityConfigSuppressDeprecatedOrderWarnings(c.Path, io.Discard) if err != nil { // Abort reconciliation if any city can't be loaded — a partial // snapshot would cause ReconcileRigs to drop rigs from the diff --git a/cmd/gc/cmd_supervisor_city.go b/cmd/gc/cmd_supervisor_city.go index 918a74915..3733b8ff4 100644 --- a/cmd/gc/cmd_supervisor_city.go +++ b/cmd/gc/cmd_supervisor_city.go @@ -32,7 +32,7 @@ var ( func supervisorCityStartTimeout(cityPath string) time.Duration { timeout := supervisorCityReadyTimeout - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return timeout } @@ -44,7 +44,7 @@ func supervisorCityStartTimeout(cityPath string) time.Duration { func supervisorCityStopTimeout(cityPath string) time.Duration { timeout := supervisorCityReadyTimeout - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return timeout } diff --git a/cmd/gc/cmd_supervisor_test.go b/cmd/gc/cmd_supervisor_test.go index 8cbb95857..6d62dbc9e 100644 --- a/cmd/gc/cmd_supervisor_test.go +++ b/cmd/gc/cmd_supervisor_test.go @@ -3,6 +3,7 @@ package main import ( "bufio" "bytes" + "fmt" "io" "log" "net" @@ -315,6 +316,80 @@ func TestBuildSupervisorServiceDataExpandsUserManagedPath(t *testing.T) { } } +func TestReconcileRigIndexSuppressesMigrationWarnings(t *testing.T) { + gcHome := t.TempDir() + t.Setenv("GC_HOME", gcHome) + + cityPath := filepath.Join(t.TempDir(), "city") + rigPath := filepath.Join(t.TempDir(), "rig") + if err := os.MkdirAll(cityPath, 0o755); err != nil { + t.Fatalf("MkdirAll(city): %v", err) + } + if err := os.MkdirAll(rigPath, 0o755); err != nil { + t.Fatalf("MkdirAll(rig): %v", err) + } + cityToml := fmt.Sprintf(`[workspace] +name = "alpha" + +[agents] +append_fragments = ["legacy.md"] + +[[rigs]] +name = "alpha-rig" +path = %q +`, rigPath) + if err := os.WriteFile(filepath.Join(cityPath, "city.toml"), []byte(cityToml), 0o644); err != nil { + t.Fatalf("WriteFile(city.toml): %v", err) + } + + reg := supervisor.NewRegistry(supervisor.RegistryPath()) + if err := reg.Register(cityPath, "alpha"); err != nil { + t.Fatalf("Register(city): %v", err) + } + + var stderr bytes.Buffer + reconcileRigIndex(reg, &stderr) + + if strings.Contains(stderr.String(), "[agents] is a deprecated compatibility alias") { + t.Fatalf("stderr = %q, want migration warnings suppressed during rig reconcile", stderr.String()) + } + + rigs, err := reg.ListRigs() + if err != nil { + t.Fatalf("ListRigs(): %v", err) + } + if len(rigs) != 1 { + t.Fatalf("ListRigs() len = %d, want 1", len(rigs)) + } + if rigs[0].Name != "alpha-rig" { + t.Fatalf("rig name = %q, want %q", rigs[0].Name, "alpha-rig") + } + if rigs[0].DefaultCity == "" { + t.Fatal("default city = empty, want reconciled default city") + } +} + +func TestEmitSupervisorLoadCityConfigWarningsOncePerCity(t *testing.T) { + var stderr bytes.Buffer + prov := &config.Provenance{ + Warnings: []string{ + `/city/pack.toml: [agents] is a deprecated compatibility alias for [agent_defaults]; rewrite the table name to [agent_defaults]`, + `/city/pack.toml: [agents] is a deprecated compatibility alias for [agent_defaults]; rewrite the table name to [agent_defaults]`, + }, + } + cityPath := filepath.Join(t.TempDir(), "city") + otherCityPath := filepath.Join(t.TempDir(), "other-city") + + emitSupervisorLoadCityConfigWarnings(&stderr, cityPath, prov) + emitSupervisorLoadCityConfigWarnings(&stderr, cityPath, prov) + emitSupervisorLoadCityConfigWarnings(&stderr, otherCityPath, prov) + + const want = "[agents] is a deprecated compatibility alias for [agent_defaults]" + if got := strings.Count(stderr.String(), want); got != 2 { + t.Fatalf("warning count = %d, want 2 (once per city); stderr=%q", got, stderr.String()) + } +} + func TestBuildSupervisorServiceDataOmitsXDGRuntimeDirForIsolatedGCHome(t *testing.T) { homeDir := t.TempDir() gcHome := filepath.Join(t.TempDir(), "isolated-home") diff --git a/cmd/gc/cmd_wait.go b/cmd/gc/cmd_wait.go index 7eeeead09..e8af94079 100644 --- a/cmd/gc/cmd_wait.go +++ b/cmd/gc/cmd_wait.go @@ -167,7 +167,7 @@ func cmdSessionWait(args, depIDs []string, matchAny bool, note string, sleep boo cityPath, cityErr := resolveCity() var cfg *config.City if cityErr == nil { - cfg, _ = loadCityConfig(cityPath) + cfg, _ = loadCityConfig(cityPath, stderr) } sessionID, err := resolveSessionIDWithConfig(cityPath, cfg, store, target) if err != nil { @@ -485,7 +485,7 @@ func loadWaitDependencyBead(cityPath string, cityStore beads.Store, depID string } return cityStore.Get(depID) } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return beads.Bead{}, err } diff --git a/cmd/gc/controller.go b/cmd/gc/controller.go index 1307de174..a8a7134c1 100644 --- a/cmd/gc/controller.go +++ b/cmd/gc/controller.go @@ -767,11 +767,12 @@ func reloadWarningsFromError(err error) []string { } // tryReloadConfig attempts to reload city.toml with includes and patches. -// Returns the new config, provenance, revision, and non-fatal warnings on -// success, or an error on failure (parse error, validation error, workspace -// name changed). Some failures after composition also return warning metadata -// via the result and error; callers should report those details while keeping -// the old config. Strict mode (default) makes composition warnings fatal. +// Returns the new config, provenance, revision, and load warnings on success, +// or an error on failure. Some failures after composition also return warning +// metadata via the result and error. Alias-only, unsupported-key, and +// deprecation warnings stay soft; composition collisions and mixed +// canonical/compat default tables stay strict-fatal unless --no-strict +// disables the gate. func tryReloadConfig(tomlPath, lockedWorkspaceName, cityRoot string) (*reloadResult, error) { // Auto-fetch remote packs before full config load (mirrors cmd_start). if quickCfg, qErr := config.Load(fsys.OSFS{}, tomlPath); qErr == nil && len(quickCfg.Packs) > 0 { @@ -802,11 +803,11 @@ func tryReloadConfig(tomlPath, lockedWorkspaceName, cityRoot string) (*reloadRes warnings: reloadWarnings, } } - if strictMode && len(prov.Warnings) > 0 { + if fatalWarnings := strictFatalLoadConfigWarnings(reloadWarnings); strictMode && len(fatalWarnings) > 0 { warnings := append(append([]string(nil), reloadWarnings...), reloadStrictWarningHint) result := resultWithWarnings(warnings) return result, reloadWarningError{ - err: fmt.Errorf("strict mode: %d collision warning(s)", len(prov.Warnings)), + err: fmt.Errorf("strict mode: %d collision warning(s)", len(fatalWarnings)), warnings: warnings, } } diff --git a/cmd/gc/dispatch_runtime.go b/cmd/gc/dispatch_runtime.go index d2178b2e6..74e64781f 100644 --- a/cmd/gc/dispatch_runtime.go +++ b/cmd/gc/dispatch_runtime.go @@ -139,7 +139,7 @@ func runWorkflowServe(agentName string, follow bool, _ io.Writer, stderr io.Writ if err != nil { return err } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, stderr) if err != nil { return err } diff --git a/cmd/gc/dolt_process_inspection.go b/cmd/gc/dolt_process_inspection.go index 38dbc4009..9b313d1fb 100644 --- a/cmd/gc/dolt_process_inspection.go +++ b/cmd/gc/dolt_process_inspection.go @@ -2,6 +2,7 @@ package main import ( "bufio" + "context" "fmt" "os" "os/exec" @@ -74,7 +75,9 @@ func findPortHolderPID(port string) int { if _, err := exec.LookPath("lsof"); err != nil { return 0 } - out, err := exec.Command("lsof", "-i", ":"+strings.TrimSpace(port), "-sTCP:LISTEN", "-t").Output() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + out, err := exec.CommandContext(ctx, "lsof", "-nP", "-iTCP:"+strings.TrimSpace(port), "-sTCP:LISTEN", "-t").Output() if err != nil { return 0 } diff --git a/cmd/gc/dolt_runtime_publication.go b/cmd/gc/dolt_runtime_publication.go index 7e2cfc6f3..b0ddf13b2 100644 --- a/cmd/gc/dolt_runtime_publication.go +++ b/cmd/gc/dolt_runtime_publication.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" @@ -90,11 +91,12 @@ func managedDoltLifecycleOwned(cityPath string) (bool, error) { } func syncManagedDoltPortMirrors(cityPath string) error { - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) if err != nil { removeDoltPortFile(cityPath) return nil } + emitLoadCityConfigWarnings(io.Discard, prov) return syncConfiguredDoltPortFiles(cityPath, cfg.Dolt, config.EffectiveHQPrefix(cfg), cfg.Rigs) } diff --git a/cmd/gc/init_provider_readiness.go b/cmd/gc/init_provider_readiness.go index 204bd7aaa..4669c4670 100644 --- a/cmd/gc/init_provider_readiness.go +++ b/cmd/gc/init_provider_readiness.go @@ -86,13 +86,14 @@ func finalizeInit(cityPath string, stdout, stderr io.Writer, opts initFinalizeOp // Canonicalize bd-owned store files before any provider-readiness block. // A failed provider auth/login check must not leave the city half-initialized. - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) if err != nil { fmt.Fprintf(stderr, "%s: city created, but startup is blocked by configuration loading\n", opts.commandName) //nolint:errcheck // best-effort stderr fmt.Fprintf(stderr, "%s: loading config for provider readiness: %v\n", opts.commandName, err) //nolint:errcheck // best-effort stderr fmt.Fprintf(stderr, "%s: fix the config issue, then run 'gc start'\n", opts.commandName) //nolint:errcheck // best-effort stderr return 1 } + emitLoadCityConfigWarnings(stderr, prov) if cityUsesBdStoreContract(cityPath) && (cfg.Dolt.Host != "" || cfg.Dolt.Port != 0) { cityDoltConfigs.Store(cityPath, cfg.Dolt) defer cityDoltConfigs.Delete(cityPath) @@ -204,13 +205,14 @@ func runInitProviderPreflight(cityPath string, stdout, stderr io.Writer, command fmt.Fprintf(stderr, "%s: materializing gastown packs: %v\n", commandName, err) //nolint:errcheck // best-effort stderr return errInitProviderPreflight } - cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + cfg, prov, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) if err != nil { fmt.Fprintf(stderr, "%s: city created, but startup is blocked by configuration loading\n", commandName) //nolint:errcheck // best-effort stderr fmt.Fprintf(stderr, "%s: loading config for provider readiness: %v\n", commandName, err) //nolint:errcheck // best-effort stderr fmt.Fprintf(stderr, "%s: fix the config issue, then run 'gc start'\n", commandName) //nolint:errcheck // best-effort stderr return errInitProviderPreflight } + emitLoadCityConfigWarnings(stderr, prov) ensureInitArtifacts(cityPath, cfg, stderr, commandName) targets, warnings, err := collectInitProviderTargets(cfg) if err != nil { @@ -521,7 +523,7 @@ func checkHardDependencies(cityPath string) []missingDep { } needsBd := false - if cfg, err := loadCityConfig(cityPath); err == nil { + if cfg, err := loadCityConfig(cityPath, io.Discard); err == nil { resolveRigPaths(cityPath, cfg.Rigs) needsBd = workspaceUsesManagedBdStoreContract(cityPath, cfg.Rigs) } else { diff --git a/cmd/gc/main.go b/cmd/gc/main.go index a054d2471..8a91385cd 100644 --- a/cmd/gc/main.go +++ b/cmd/gc/main.go @@ -531,7 +531,7 @@ func rigFromCwd(cityPath string) string { // rigFromCwdDir matches cwd against registered rigs in a city's config. func rigFromCwdDir(cityPath, cwd string) string { - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return "" } @@ -573,7 +573,7 @@ func rigCityEntries(reg *supervisor.Registry, rigPath string) []supervisor.CityE } var matched []supervisor.CityEntry for _, c := range cities { - cfg, err := loadCityConfigSuppressDeprecatedOrderWarnings(c.Path) + cfg, err := loadCityConfigSuppressDeprecatedOrderWarnings(c.Path, io.Discard) if err != nil { continue } @@ -729,7 +729,7 @@ func openStoreAtForCity(storePath, cityPath string) (beads.Store, error) { env := gcExecStoreEnv(runtimeCityPath, target, provider) if execProviderNeedsScopedDoltStoreEnv(provider) { if target.ScopeKind == "rig" { - cfg, err := loadCityConfig(runtimeCityPath) + cfg, err := loadCityConfig(runtimeCityPath, io.Discard) if err != nil { return nil, err } @@ -775,7 +775,7 @@ func openBdStoreAt(storePath, cityPath string) (beads.Store, error) { if filepath.Clean(storePath) == filepath.Clean(cityPath) { return bdStoreForCity(storePath, cityPath), nil } - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { cfg = nil } diff --git a/cmd/gc/main_test.go b/cmd/gc/main_test.go index 421bb111e..43752b28a 100644 --- a/cmd/gc/main_test.go +++ b/cmd/gc/main_test.go @@ -3286,6 +3286,38 @@ func TestInitNameFlagWithFrom(t *testing.T) { } } +func TestDoInitFromDirEmitsLoadWarningsFromCopiedConfig(t *testing.T) { + t.Setenv("GC_BEADS", "file") + t.Setenv("GC_DOLT", "skip") + configureIsolatedRuntimeEnv(t) + + dir := t.TempDir() + srcDir := filepath.Join(dir, "template") + if err := os.MkdirAll(srcDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(srcDir, "city.toml"), []byte(`[workspace] +name = "template" +provider = "claude" + +[agent_defaults] +skills = ["demo"] +`), 0o644); err != nil { + t.Fatal(err) + } + + cityPath := filepath.Join(dir, "target-dir") + + var stdout, stderr bytes.Buffer + code := doInitFromDirWithOptions(srcDir, cityPath, "", &stdout, &stderr, true) + if code != 0 { + t.Fatalf("doInitFromDirWithOptions = %d, want 0; stderr: %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "attachment-list fields") { + t.Fatalf("stderr = %q, want deprecated attachment-list warning", stderr.String()) + } +} + func TestInitNameFlagWithFile(t *testing.T) { t.Setenv("GC_BEADS", "file") t.Setenv("GC_DOLT", "skip") @@ -4605,6 +4637,75 @@ prompt_template = "prompts/mayor.md" } } +func TestDoPrimeImportedPackAppendFragmentsLayerBeforeCityDefaults(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, ".gc"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(dir, "packs", "imported", "agents", "mayor", "template-fragments"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "pack.toml"), []byte("[pack]\nname = \"root\"\nschema = 2\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "city.toml"), []byte(` +[workspace] +name = "test" +includes = ["packs/imported"] + +[agent_defaults] +append_fragments = ["city-footer"] +`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "packs", "imported", "pack.toml"), []byte(` +[pack] +name = "imported" +schema = 2 + +[agent_defaults] +append_fragments = ["pack-footer"] + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +prompt_template = "agents/mayor/prompt.template.md" +`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "packs", "imported", "agents", "mayor", "prompt.template.md"), []byte("Hello"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "packs", "imported", "agents", "mayor", "template-fragments", "pack-footer.template.md"), []byte(`{{ define "pack-footer" }}Pack Footer{{ end }}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "packs", "imported", "agents", "mayor", "template-fragments", "city-footer.template.md"), []byte(`{{ define "city-footer" }}City Footer{{ end }}`), 0o644); err != nil { + t.Fatal(err) + } + + orig, _ := os.Getwd() + t.Cleanup(func() { _ = os.Chdir(orig) }) + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + + var stdout, stderr bytes.Buffer + code := doPrime([]string{"mayor"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("doPrime = %d, want 0; stderr: %s", code, stderr.String()) + } + out := stdout.String() + packIdx := strings.Index(out, "Pack Footer") + cityIdx := strings.Index(out, "City Footer") + if packIdx < 0 || cityIdx < 0 { + t.Fatalf("prompt missing inherited fragments: %q", out) + } + if packIdx > cityIdx { + t.Fatalf("pack fragment should render before city fragment: %q", out) + } +} + // --- findEnclosingRig tests --- func TestFindEnclosingRig(t *testing.T) { diff --git a/cmd/gc/pool.go b/cmd/gc/pool.go index ab4cf2f22..a5b779546 100644 --- a/cmd/gc/pool.go +++ b/cmd/gc/pool.go @@ -214,11 +214,12 @@ func deepCopyAgent(src *config.Agent, name, dir string) config.Agent { PromptFlag: src.PromptFlag, ReadyPromptPrefix: src.ReadyPromptPrefix, // DefaultSlingFormula: deep-copied below with other pointer fields. - WorkQuery: src.WorkQuery, - SlingQuery: src.SlingQuery, - SessionSetupScript: src.SessionSetupScript, - OverlayDir: src.OverlayDir, - SourceDir: src.SourceDir, + WorkQuery: src.WorkQuery, + SlingQuery: src.SlingQuery, + SessionSetupScript: src.SessionSetupScript, + OverlayDir: src.OverlayDir, + SourceDir: src.SourceDir, + // InheritedDefaultSlingFormula: deep-copied below with other pointer fields. Fallback: src.Fallback, IdleTimeout: src.IdleTimeout, SleepAfterIdle: src.SleepAfterIdle, @@ -266,6 +267,10 @@ func deepCopyAgent(src *config.Agent, name, dir string) config.Agent { dst.InjectFragments = make([]string, len(src.InjectFragments)) copy(dst.InjectFragments, src.InjectFragments) } + if len(src.InheritedAppendFragments) > 0 { + dst.InheritedAppendFragments = make([]string, len(src.InheritedAppendFragments)) + copy(dst.InheritedAppendFragments, src.InheritedAppendFragments) + } if len(src.InstallAgentHooks) > 0 { dst.InstallAgentHooks = make([]string, len(src.InstallAgentHooks)) copy(dst.InstallAgentHooks, src.InstallAgentHooks) @@ -306,6 +311,10 @@ func deepCopyAgent(src *config.Agent, name, dir string) config.Agent { v := *src.DefaultSlingFormula dst.DefaultSlingFormula = &v } + if src.InheritedDefaultSlingFormula != nil { + v := *src.InheritedDefaultSlingFormula + dst.InheritedDefaultSlingFormula = &v + } if src.Attach != nil { v := *src.Attach dst.Attach = &v diff --git a/cmd/gc/pool_test.go b/cmd/gc/pool_test.go index 51c27e2f9..2678672f8 100644 --- a/cmd/gc/pool_test.go +++ b/cmd/gc/pool_test.go @@ -547,61 +547,63 @@ func TestDeepCopyAgentCoversAllFields(t *testing.T) { trueVal := true intVal := 42 src := config.Agent{ - Name: "original", - Description: "test agent description", - Dir: "original-dir", - WorkDir: ".gc/agents/original", - Scope: "city", - Suspended: true, - PreStart: []string{"pre-cmd"}, - PromptTemplate: "prompts/test.md", - Nudge: "nudge text", - Session: "acp", - Provider: "claude", - StartCommand: "claude --dangerously", - Args: []string{"--arg1"}, - PromptMode: "flag", - PromptFlag: "--prompt", - ReadyDelayMs: &intVal, - ReadyPromptPrefix: "ready>", - ProcessNames: []string{"claude"}, - EmitsPermissionWarning: &trueVal, - Env: map[string]string{"K": "V"}, - MaxActiveSessions: intPtr(5), - MinActiveSessions: intPtr(1), - ScaleCheck: "echo 3", - WorkQuery: "bd ready", - SlingQuery: "bd update {}", - IdleTimeout: "15m", - SleepAfterIdle: "30s", - SleepAfterIdleSource: "agent", - InstallAgentHooks: []string{"claude"}, - SkillsDir: "/skills", - MCPDir: "/mcp", - HooksInstalled: &trueVal, - InjectAssignedSkills: &trueVal, - SessionSetup: []string{"setup-cmd"}, - SessionSetupScript: "scripts/setup.sh", - SessionLive: []string{"live-cmd"}, - OverlayDir: "overlays/test", - SourceDir: "/src", - DefaultSlingFormula: strPtr("mol-work"), - InjectFragments: []string{"frag1"}, - Attach: &trueVal, - Fallback: true, - PoolName: "template/name", - ResumeCommand: "claude --resume {{.SessionKey}} --dangerously", - DependsOn: []string{"other-agent"}, - WakeMode: "fresh", - Implicit: true, - DrainTimeout: "10m", - OnBoot: "echo boot", - OnDeath: "echo death", - Namepool: "names.txt", - NamepoolNames: []string{"alpha", "bravo"}, - OptionDefaults: map[string]string{"effort": "max"}, - BindingName: "gastown", - PackName: "gastown", + Name: "original", + Description: "test agent description", + Dir: "original-dir", + WorkDir: ".gc/agents/original", + Scope: "city", + Suspended: true, + PreStart: []string{"pre-cmd"}, + PromptTemplate: "prompts/test.md", + Nudge: "nudge text", + Session: "acp", + Provider: "claude", + StartCommand: "claude --dangerously", + Args: []string{"--arg1"}, + PromptMode: "flag", + PromptFlag: "--prompt", + ReadyDelayMs: &intVal, + ReadyPromptPrefix: "ready>", + ProcessNames: []string{"claude"}, + EmitsPermissionWarning: &trueVal, + Env: map[string]string{"K": "V"}, + MaxActiveSessions: intPtr(5), + MinActiveSessions: intPtr(1), + ScaleCheck: "echo 3", + WorkQuery: "bd ready", + SlingQuery: "bd update {}", + IdleTimeout: "15m", + SleepAfterIdle: "30s", + SleepAfterIdleSource: "agent", + InstallAgentHooks: []string{"claude"}, + SkillsDir: "/skills", + MCPDir: "/mcp", + HooksInstalled: &trueVal, + InjectAssignedSkills: &trueVal, + SessionSetup: []string{"setup-cmd"}, + SessionSetupScript: "scripts/setup.sh", + SessionLive: []string{"live-cmd"}, + OverlayDir: "overlays/test", + SourceDir: "/src", + DefaultSlingFormula: strPtr("mol-work"), + InheritedDefaultSlingFormula: strPtr("mol-pack"), + InjectFragments: []string{"frag1"}, + InheritedAppendFragments: []string{"pack-footer"}, + Attach: &trueVal, + Fallback: true, + PoolName: "template/name", + ResumeCommand: "claude --resume {{.SessionKey}} --dangerously", + DependsOn: []string{"other-agent"}, + WakeMode: "fresh", + Implicit: true, + DrainTimeout: "10m", + OnBoot: "echo boot", + OnDeath: "echo death", + Namepool: "names.txt", + NamepoolNames: []string{"alpha", "bravo"}, + OptionDefaults: map[string]string{"effort": "max"}, + BindingName: "gastown", + PackName: "gastown", } // Tombstone fields (deprecated in v0.15.1, removed in v0.16) are not @@ -659,6 +661,7 @@ func TestDeepCopyAgentCoversAllFields(t *testing.T) { src.Args[0] = "MUTATED" src.ProcessNames[0] = "MUTATED" src.InjectFragments[0] = "MUTATED" + src.InheritedAppendFragments[0] = "MUTATED" src.InstallAgentHooks[0] = "MUTATED" newMin := 999 src.MinActiveSessions = &newMin @@ -681,6 +684,9 @@ func TestDeepCopyAgentCoversAllFields(t *testing.T) { if dst.InjectFragments[0] == "MUTATED" { t.Error("InjectFragments is not a deep copy") } + if dst.InheritedAppendFragments[0] == "MUTATED" { + t.Error("InheritedAppendFragments is not a deep copy") + } if dst.InstallAgentHooks[0] == "MUTATED" { t.Error("InstallAgentHooks is not a deep copy") } diff --git a/cmd/gc/prompt.go b/cmd/gc/prompt.go index dc52e9a32..e33c39414 100644 --- a/cmd/gc/prompt.go +++ b/cmd/gc/prompt.go @@ -181,11 +181,28 @@ func mergeFragmentLists(global, perAgent []string) []string { return nil } merged := make([]string, 0, len(global)+len(perAgent)) + seen := make(map[string]struct{}, len(global)+len(perAgent)) merged = append(merged, global...) - merged = append(merged, perAgent...) + for _, name := range global { + seen[name] = struct{}{} + } + for _, name := range perAgent { + if _, dup := seen[name]; dup { + continue + } + seen[name] = struct{}{} + merged = append(merged, name) + } return merged } +// effectivePromptFragments applies the runtime fragment layering contract. +func effectivePromptFragments(global, inject, inherited, defaults []string) []string { + fragments := mergeFragmentLists(global, inject) + fragments = mergeFragmentLists(fragments, inherited) + return mergeFragmentLists(fragments, defaults) +} + // buildTemplateData merges Env (lower priority) with SDK fields (higher // priority) into a single map for template execution. func buildTemplateData(ctx PromptContext) map[string]string { diff --git a/cmd/gc/prompt_test.go b/cmd/gc/prompt_test.go index a7742d4f4..f2a5c320d 100644 --- a/cmd/gc/prompt_test.go +++ b/cmd/gc/prompt_test.go @@ -649,6 +649,7 @@ func TestMergeFragmentLists(t *testing.T) { {"global only", []string{"a"}, nil, []string{"a"}, false}, {"agent only", nil, []string{"b"}, []string{"b"}, false}, {"both", []string{"a", "b"}, []string{"c"}, []string{"a", "b", "c"}, false}, + {"dedup preserves first occurrence", []string{"a", "b"}, []string{"b", "c"}, []string{"a", "b", "c"}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -670,3 +671,42 @@ func TestMergeFragmentLists(t *testing.T) { }) } } + +func TestEffectivePromptFragments(t *testing.T) { + got := effectivePromptFragments( + []string{"global"}, + []string{"inject"}, + []string{"inherited"}, + []string{"default"}, + ) + want := []string{"global", "inject", "inherited", "default"} + if len(got) != len(want) { + t.Fatalf("len = %d, want %d", len(got), len(want)) + } + for i := range got { + if got[i] != want[i] { + t.Fatalf("[%d] = %q, want %q", i, got[i], want[i]) + } + } + if effectivePromptFragments(nil, nil, nil, nil) != nil { + t.Fatal("effectivePromptFragments(nil, nil, nil, nil) = non-nil, want nil") + } +} + +func TestEffectivePromptFragmentsDedupsAcrossLayers(t *testing.T) { + got := effectivePromptFragments( + []string{"shared"}, + []string{"inject"}, + []string{"shared", "pack"}, + []string{"pack", "city"}, + ) + want := []string{"shared", "inject", "pack", "city"} + if len(got) != len(want) { + t.Fatalf("len = %d, want %d", len(got), len(want)) + } + for i := range got { + if got[i] != want[i] { + t.Fatalf("[%d] = %q, want %q", i, got[i], want[i]) + } + } +} diff --git a/cmd/gc/providers.go b/cmd/gc/providers.go index bf2b06aa7..1669b4eae 100644 --- a/cmd/gc/providers.go +++ b/cmd/gc/providers.go @@ -44,7 +44,7 @@ func loadSessionProviderContext() sessionProviderContext { providerName: os.Getenv("GC_SESSION"), } if cp, err := resolveCity(); err == nil { - if cfg, err := loadCityConfig(cp); err == nil { + if cfg, err := loadCityConfig(cp, io.Discard); err == nil { return sessionProviderContextForCity(cfg, cp, ctx.providerName) } } @@ -411,7 +411,7 @@ func mailProviderName() string { return v } if cp, err := resolveCity(); err == nil { - if cfg, err := loadCityConfig(cp); err == nil && cfg.Mail.Provider != "" { + if cfg, err := loadCityConfig(cp, io.Discard); err == nil && cfg.Mail.Provider != "" { return cfg.Mail.Provider } } @@ -464,7 +464,7 @@ func eventsProviderName() string { return v } if cp, err := resolveCity(); err == nil { - if cfg, err := loadCityConfig(cp); err == nil && cfg.Events.Provider != "" { + if cfg, err := loadCityConfig(cp, io.Discard); err == nil && cfg.Events.Provider != "" { return cfg.Events.Provider } } diff --git a/cmd/gc/session_beads_test.go b/cmd/gc/session_beads_test.go index 48cd2039b..40167459d 100644 --- a/cmd/gc/session_beads_test.go +++ b/cmd/gc/session_beads_test.go @@ -5,6 +5,8 @@ import ( "context" "errors" "io" + "os" + "path/filepath" "strings" "testing" "time" @@ -14,6 +16,7 @@ import ( "github.com/gastownhall/gascity/internal/clock" "github.com/gastownhall/gascity/internal/config" "github.com/gastownhall/gascity/internal/extmsg" + "github.com/gastownhall/gascity/internal/fsys" "github.com/gastownhall/gascity/internal/runtime" "github.com/gastownhall/gascity/internal/session" ) @@ -107,6 +110,94 @@ func TestSyncSessionBeads_CreatesNewBeads(t *testing.T) { } } +func TestSyncSessionBeads_CreatesImportedConfiguredNamedSessionBeads(t *testing.T) { + cityPath := t.TempDir() + rigPath := filepath.Join(cityPath, "repo") + for path, contents := range map[string]string{ + filepath.Join(cityPath, "pack.toml"): ` +[pack] +name = "import-regression" +schema = 2 + +[imports.gs] +source = "./assets/sidecar" +`, + filepath.Join(cityPath, "city.toml"): ` +[workspace] +name = "import-regression" +provider = "claude" + +[[rigs]] +name = "repo" +path = "./repo" + +[rigs.imports.gs] +source = "./assets/sidecar" +`, + filepath.Join(cityPath, "assets", "sidecar", "pack.toml"): ` +[pack] +name = "sidecar" +schema = 2 + +[[named_session]] +template = "captain" +scope = "city" +mode = "always" + +[[named_session]] +template = "watcher" +scope = "rig" +mode = "always" +`, + filepath.Join(cityPath, "assets", "sidecar", "agents", "captain", "agent.toml"): "scope = \"city\"\nstart_command = \"true\"\n", + filepath.Join(cityPath, "assets", "sidecar", "agents", "captain", "prompt.md"): "You are the imported captain.\n", + filepath.Join(cityPath, "assets", "sidecar", "agents", "watcher", "agent.toml"): "scope = \"rig\"\nstart_command = \"true\"\n", + filepath.Join(cityPath, "assets", "sidecar", "agents", "watcher", "prompt.md"): "You are the imported watcher.\n", + } { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%q): %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + t.Fatalf("WriteFile(%q): %v", path, err) + } + } + if err := os.MkdirAll(rigPath, 0o755); err != nil { + t.Fatalf("MkdirAll(%q): %v", rigPath, err) + } + + cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + store := beads.NewMemStore() + sp := runtime.NewFake() + clk := &clock.Fake{Time: time.Date(2026, 4, 18, 12, 0, 0, 0, time.UTC)} + ds := buildDesiredState(cfg.EffectiveCityName(), cityPath, clk.Now(), cfg, sp, store, io.Discard).State + + var stderr bytes.Buffer + syncSessionBeads(cityPath, store, ds, sp, allConfiguredDS(ds), cfg, clk, &stderr, false) + if stderr.Len() > 0 { + t.Fatalf("unexpected stderr: %s", stderr.String()) + } + + all := allSessionBeads(t, store) + if len(all) != 2 { + t.Fatalf("session bead count = %d, want 2: %+v", len(all), all) + } + + found := map[string]string{} + for _, b := range all { + found[strings.TrimSpace(b.Metadata["configured_named_identity"])] = strings.TrimSpace(b.Metadata["session_name"]) + } + if found["gs.captain"] != "gs__captain" { + t.Fatalf("captain session_name = %q, want %q", found["gs.captain"], "gs__captain") + } + if found["repo/gs.watcher"] != "repo--gs__watcher" { + t.Fatalf("watcher session_name = %q, want %q", found["repo/gs.watcher"], "repo--gs__watcher") + } +} + func TestSyncSessionBeads_SetsManagedAlias(t *testing.T) { store := beads.NewMemStore() clk := &clock.Fake{Time: time.Date(2026, 3, 7, 12, 0, 0, 0, time.UTC)} diff --git a/cmd/gc/session_target.go b/cmd/gc/session_target.go index e280e4be6..ef44df6dd 100644 --- a/cmd/gc/session_target.go +++ b/cmd/gc/session_target.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "io" "os" "strings" ) @@ -42,8 +43,8 @@ func currentSessionRuntimeTarget() (sessionRuntimeTarget, error) { }, nil } -func resolveSessionRuntimeTarget(identifier string) (sessionRuntimeTarget, error) { - target, err := resolveNudgeTarget(identifier) +func resolveSessionRuntimeTarget(identifier string, warningWriter ...io.Writer) (sessionRuntimeTarget, error) { + target, err := resolveNudgeTarget(identifier, warningWriter...) if err != nil { return sessionRuntimeTarget{}, err } diff --git a/cmd/gc/store_target_exec.go b/cmd/gc/store_target_exec.go index 20051c16a..6dc2e4d11 100644 --- a/cmd/gc/store_target_exec.go +++ b/cmd/gc/store_target_exec.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "io" "os" "path/filepath" "strings" @@ -73,7 +74,7 @@ func gcExecLifecycleInitProcessEnv(cityPath string, target execStoreTarget, prov return mergeRuntimeEnv(os.Environ(), env), nil } if target.ScopeKind == "rig" { - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return nil, err } @@ -106,7 +107,7 @@ func execProviderNeedsScopedDoltStoreEnv(provider string) bool { func resolveConfiguredExecStoreTarget(cityPath, storePath string) (execStoreTarget, error) { scopeRoot := resolveStoreScopeRoot(cityPath, storePath) - cfg, err := loadCityConfig(cityPath) + cfg, err := loadCityConfig(cityPath, io.Discard) if err != nil { return execStoreTarget{}, err } diff --git a/cmd/gc/template_resolve.go b/cmd/gc/template_resolve.go index 4a19ed8eb..a048ab0d6 100644 --- a/cmd/gc/template_resolve.go +++ b/cmd/gc/template_resolve.go @@ -247,11 +247,14 @@ func resolveTemplate(p *agentBuildParams, cfgAgent *config.Agent, qualifiedName // Step 9: Render prompt with beacon. var prompt string // Merge fragment sources: V1 global_fragments + inject_fragments, - // plus V2 append_fragments from agent defaults. - fragments := mergeFragmentLists(p.globalFragments, cfgAgent.InjectFragments) - if len(p.appendFragments) > 0 { - fragments = mergeFragmentLists(fragments, p.appendFragments) - } + // imported-pack [agent_defaults].append_fragments, then city-level + // [agent_defaults].append_fragments. + fragments := effectivePromptFragments( + p.globalFragments, + cfgAgent.InjectFragments, + cfgAgent.InheritedAppendFragments, + p.appendFragments, + ) prompt = renderPrompt(p.fs, p.cityPath, p.cityName, cfgAgent.PromptTemplate, PromptContext{ CityRoot: p.cityPath, AgentName: qualifiedName, diff --git a/cmd/gc/template_resolve_prompt_test.go b/cmd/gc/template_resolve_prompt_test.go index 6f874ca65..5bc8b90d9 100644 --- a/cmd/gc/template_resolve_prompt_test.go +++ b/cmd/gc/template_resolve_prompt_test.go @@ -448,3 +448,367 @@ func TestResolveTemplateClaudeProjectsCityDotClaudeSettingsIntoRuntimeFile(t *te t.Fatalf("runtime settings lost default Claude hooks:\n%s", rendered) } } + +func TestResolveTemplateImportedPackAppendFragmentsLayerBeforeCityDefaults(t *testing.T) { + cityPath := t.TempDir() + write := func(rel, data string) { + path := filepath.Join(cityPath, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%s): %v", path, err) + } + if err := os.WriteFile(path, []byte(data), 0o644); err != nil { + t.Fatalf("WriteFile(%s): %v", path, err) + } + } + + write("city.toml", ` +[workspace] +name = "test" +includes = ["packs/imported"] + +[agent_defaults] +append_fragments = ["city-footer"] +`) + write("packs/imported/pack.toml", ` +[pack] +name = "imported" +schema = 2 + +[agent_defaults] +append_fragments = ["pack-footer"] + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +prompt_template = "agents/mayor/prompt.template.md" +`) + write("packs/imported/agents/mayor/prompt.template.md", "Hello") + write("packs/imported/agents/mayor/template-fragments/pack-footer.template.md", `{{ define "pack-footer" }}Pack Footer{{ end }}`) + write("packs/imported/agents/mayor/template-fragments/city-footer.template.md", `{{ define "city-footer" }}City Footer{{ end }}`) + + cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + var agentCfg config.Agent + found := false + for _, a := range cfg.Agents { + if !a.Implicit && a.Name == "mayor" { + agentCfg = a + found = true + break + } + } + if !found { + t.Fatalf("expected explicit imported mayor agent, got %v", cfg.Agents) + } + params := &agentBuildParams{ + fs: fsys.OSFS{}, + cityName: "test", + cityPath: cityPath, + workspace: &cfg.Workspace, + providers: config.BuiltinProviders(), + lookPath: func(string) (string, error) { return "/usr/bin/claude", nil }, + beaconTime: testBeaconTime, + packDirs: cfg.PackDirs, + globalFragments: cfg.Workspace.GlobalFragments, + appendFragments: mergeFragmentLists(cfg.AgentDefaults.AppendFragments, cfg.AgentsDefaults.AppendFragments), + beadNames: make(map[string]string), + stderr: io.Discard, + } + + tp, err := resolveTemplate(params, &agentCfg, agentCfg.QualifiedName(), nil) + if err != nil { + t.Fatalf("resolveTemplate: %v", err) + } + packIdx := strings.Index(tp.Prompt, "Pack Footer") + cityIdx := strings.Index(tp.Prompt, "City Footer") + if packIdx < 0 || cityIdx < 0 { + t.Fatalf("prompt missing inherited fragments: %q", tp.Prompt) + } + if packIdx > cityIdx { + t.Fatalf("pack fragment should render before city fragment: %q", tp.Prompt) + } +} + +func TestResolveTemplateNestedIncludedPackAppendFragmentsLayerBeforeCityDefaults(t *testing.T) { + cityPath := t.TempDir() + write := func(rel, data string) { + path := filepath.Join(cityPath, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%s): %v", path, err) + } + if err := os.WriteFile(path, []byte(data), 0o644); err != nil { + t.Fatalf("WriteFile(%s): %v", path, err) + } + } + + write("city.toml", ` +[workspace] +name = "test" +includes = ["packs/imported"] + +[agent_defaults] +append_fragments = ["city-footer"] +`) + write("packs/imported/pack.toml", ` +[pack] +name = "imported" +schema = 2 +includes = ["../base"] + +[agent_defaults] +append_fragments = ["pack-footer"] +`) + write("packs/base/pack.toml", ` +[pack] +name = "base" +schema = 2 + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +prompt_template = "agents/mayor/prompt.template.md" +`) + write("packs/base/agents/mayor/prompt.template.md", "Hello") + write("packs/base/agents/mayor/template-fragments/pack-footer.template.md", `{{ define "pack-footer" }}Pack Footer{{ end }}`) + write("packs/base/agents/mayor/template-fragments/city-footer.template.md", `{{ define "city-footer" }}City Footer{{ end }}`) + + cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + var agentCfg config.Agent + found := false + for _, a := range cfg.Agents { + if !a.Implicit && a.Name == "mayor" { + agentCfg = a + found = true + break + } + } + if !found { + t.Fatalf("expected explicit imported mayor agent, got %v", cfg.Agents) + } + params := &agentBuildParams{ + fs: fsys.OSFS{}, + cityName: "test", + cityPath: cityPath, + workspace: &cfg.Workspace, + providers: config.BuiltinProviders(), + lookPath: func(string) (string, error) { return "/usr/bin/claude", nil }, + beaconTime: testBeaconTime, + packDirs: cfg.PackDirs, + globalFragments: cfg.Workspace.GlobalFragments, + appendFragments: mergeFragmentLists(cfg.AgentDefaults.AppendFragments, cfg.AgentsDefaults.AppendFragments), + beadNames: make(map[string]string), + stderr: io.Discard, + } + + tp, err := resolveTemplate(params, &agentCfg, agentCfg.QualifiedName(), nil) + if err != nil { + t.Fatalf("resolveTemplate: %v", err) + } + packIdx := strings.Index(tp.Prompt, "Pack Footer") + cityIdx := strings.Index(tp.Prompt, "City Footer") + if packIdx < 0 || cityIdx < 0 { + t.Fatalf("prompt missing inherited fragments: %q", tp.Prompt) + } + if packIdx > cityIdx { + t.Fatalf("pack fragment should render before city fragment: %q", tp.Prompt) + } +} + +func TestResolveTemplateWrapperPackDefaultsDoNotBleedAcrossImports(t *testing.T) { + cityPath := t.TempDir() + write := func(rel, data string) { + path := filepath.Join(cityPath, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%s): %v", path, err) + } + if err := os.WriteFile(path, []byte(data), 0o644); err != nil { + t.Fatalf("WriteFile(%s): %v", path, err) + } + } + + write("city.toml", ` +[workspace] +name = "test" +includes = ["packs/wrapper"] + +[agent_defaults] +append_fragments = ["city-footer"] +`) + write("packs/wrapper/pack.toml", ` +[pack] +name = "wrapper" +schema = 2 + +[agent_defaults] +append_fragments = ["wrapper-footer"] + +[imports.dep] +source = "../dep" +`) + write("packs/dep/pack.toml", ` +[pack] +name = "dep" +schema = 2 + +[agent_defaults] +append_fragments = ["dep-footer"] + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +prompt_template = "agents/mayor/prompt.template.md" +`) + write("packs/dep/agents/mayor/prompt.template.md", "Hello") + write("packs/dep/agents/mayor/template-fragments/dep-footer.template.md", `{{ define "dep-footer" }}Dep Footer{{ end }}`) + write("packs/dep/agents/mayor/template-fragments/wrapper-footer.template.md", `{{ define "wrapper-footer" }}Wrapper Footer{{ end }}`) + write("packs/dep/agents/mayor/template-fragments/city-footer.template.md", `{{ define "city-footer" }}City Footer{{ end }}`) + + cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + var agentCfg config.Agent + found := false + for _, a := range cfg.Agents { + if !a.Implicit && a.BindingName == "dep" && a.Name == "mayor" { + agentCfg = a + found = true + break + } + } + if !found { + t.Fatalf("expected explicit imported dep.mayor agent, got %v", cfg.Agents) + } + params := &agentBuildParams{ + fs: fsys.OSFS{}, + cityName: "test", + cityPath: cityPath, + workspace: &cfg.Workspace, + providers: config.BuiltinProviders(), + lookPath: func(string) (string, error) { return "/usr/bin/claude", nil }, + beaconTime: testBeaconTime, + packDirs: cfg.PackDirs, + globalFragments: cfg.Workspace.GlobalFragments, + appendFragments: mergeFragmentLists(cfg.AgentDefaults.AppendFragments, cfg.AgentsDefaults.AppendFragments), + beadNames: make(map[string]string), + stderr: io.Discard, + } + + tp, err := resolveTemplate(params, &agentCfg, agentCfg.QualifiedName(), nil) + if err != nil { + t.Fatalf("resolveTemplate: %v", err) + } + if strings.Contains(tp.Prompt, "Wrapper Footer") { + t.Fatalf("wrapper fragment should not bleed across imports: %q", tp.Prompt) + } + if !strings.Contains(tp.Prompt, "Dep Footer") || !strings.Contains(tp.Prompt, "City Footer") { + t.Fatalf("prompt missing expected fragments: %q", tp.Prompt) + } +} + +func TestResolveTemplateIncludingPackDefaultsDoNotBleedAcrossNestedImportBoundaries(t *testing.T) { + cityPath := t.TempDir() + write := func(rel, data string) { + path := filepath.Join(cityPath, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%s): %v", path, err) + } + if err := os.WriteFile(path, []byte(data), 0o644); err != nil { + t.Fatalf("WriteFile(%s): %v", path, err) + } + } + + write("city.toml", ` +[workspace] +name = "test" +includes = ["packs/outer"] + +[agent_defaults] +append_fragments = ["city-footer"] +`) + write("packs/outer/pack.toml", ` +[pack] +name = "outer" +schema = 2 +includes = ["../mid"] + +[agent_defaults] +append_fragments = ["outer-footer"] +`) + write("packs/mid/pack.toml", ` +[pack] +name = "mid" +schema = 2 + +[imports.dep] +source = "../dep" +`) + write("packs/dep/pack.toml", ` +[pack] +name = "dep" +schema = 2 + +[agent_defaults] +append_fragments = ["dep-footer"] + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +prompt_template = "agents/mayor/prompt.template.md" +`) + write("packs/dep/agents/mayor/prompt.template.md", "Hello") + write("packs/dep/agents/mayor/template-fragments/dep-footer.template.md", `{{ define "dep-footer" }}Dep Footer{{ end }}`) + write("packs/dep/agents/mayor/template-fragments/outer-footer.template.md", `{{ define "outer-footer" }}Outer Footer{{ end }}`) + write("packs/dep/agents/mayor/template-fragments/city-footer.template.md", `{{ define "city-footer" }}City Footer{{ end }}`) + + cfg, _, err := config.LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityPath, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + var agentCfg config.Agent + found := false + for _, a := range cfg.Agents { + if !a.Implicit && a.BindingName == "dep" && a.Name == "mayor" { + agentCfg = a + found = true + break + } + } + if !found { + t.Fatalf("expected explicit imported dep.mayor agent, got %v", cfg.Agents) + } + params := &agentBuildParams{ + fs: fsys.OSFS{}, + cityName: "test", + cityPath: cityPath, + workspace: &cfg.Workspace, + providers: config.BuiltinProviders(), + lookPath: func(string) (string, error) { return "/usr/bin/claude", nil }, + beaconTime: testBeaconTime, + packDirs: cfg.PackDirs, + globalFragments: cfg.Workspace.GlobalFragments, + appendFragments: mergeFragmentLists(cfg.AgentDefaults.AppendFragments, cfg.AgentsDefaults.AppendFragments), + beadNames: make(map[string]string), + stderr: io.Discard, + } + + tp, err := resolveTemplate(params, &agentCfg, agentCfg.QualifiedName(), nil) + if err != nil { + t.Fatalf("resolveTemplate: %v", err) + } + if strings.Contains(tp.Prompt, "Outer Footer") { + t.Fatalf("including-pack fragment should not bleed across nested import boundaries: %q", tp.Prompt) + } + if !strings.Contains(tp.Prompt, "Dep Footer") || !strings.Contains(tp.Prompt, "City Footer") { + t.Fatalf("prompt missing expected fragments: %q", tp.Prompt) + } +} diff --git a/docs/guides/migrating-to-pack-vnext.md b/docs/guides/migrating-to-pack-vnext.md index eb1545044..16ae81726 100644 --- a/docs/guides/migrating-to-pack-vnext.md +++ b/docs/guides/migrating-to-pack-vnext.md @@ -3,6 +3,11 @@ title: "PackV2: The New Package System for Gas City" description: How to move an existing Gas City 0.14.0 city or pack to the PackV2 schema and directory conventions. --- +> [!IMPORTANT] +> This document describes the pre-release Gas City v0.15.0 rollout. +> Some PackV2 surfaces are still under active development; release-gated +> caveats below use the form "As of release v0.15.0, ...". + This guide is the practical migration companion for moving from the 0.14.0 PackV1 world into the PackV2 model that first landed in 0.14.1 and is being finished in the 0.15.0 wave. @@ -34,7 +39,7 @@ pack directory tree. The target public migration flow is `gc doctor`, then `gc doctor --fix` for the safe mechanical rewrites, then `gc doctor` again to confirm the result. Some old cities may hard-break until -migrated; that is intentional in this wave. +migrated; that is intentional as of release v0.15.0. > **Current rollout note:** The doctor-first remediation slice lands > separately from the Skills/MCP, infix, and rig-path slices. Until that @@ -460,9 +465,9 @@ template inclusion. | `inject_fragments_append` on patches | Gone — same approach | | All `.md` files run through Go templates | Only `.template.md` files run through Go templates | -For migration convenience, `append_fragments` in `agent.toml` or -`[agent_defaults]` auto-appends named fragments to `.template.md` -prompts without editing each prompt file: +For migration convenience, `[agent_defaults].append_fragments` +auto-appends named fragments to `.template.md` prompts without editing +each prompt file: ```toml # pack.toml or city.toml @@ -473,7 +478,7 @@ append_fragments = ["operational-awareness", "command-glossary"] Plain `.md` prompts are inert — no fragments attach, no template engine runs. -> **NYI in this wave:** `[agent_defaults].append_fragments` is the +> **As of release v0.15.0:** `[agent_defaults].append_fragments` is the > proven migration bridge in the current release. Agent-local > `append_fragments` is still tracked as a spec/runtime parity gap in > [#671](https://github.com/gastownhall/gascity/issues/671). @@ -599,16 +604,16 @@ schema, plus the qualified rows that matter most during migration. > **Current rollout note:** Some rows below describe the target PackV2 > destination rather than the exact state of every in-flight branch. In -> the current 15.0 wave, `workspace.name` still lives in `city.toml`. +> release v0.15.0, `workspace.name` still lives in `city.toml`. > Phase A rig-binding work removes machine-local `rigs.path` from newly > written city configs, but `rigs.prefix` and `rigs.suspended` remain in -> `city.toml` in this release. +> `city.toml` as of release v0.15.0. | 0.14.0 element | What it did | New home or action | |---|---|---| | `include` | Merged extra config fragments into `city.toml` before load | Remove as part of migration. Move real composition to imports and move remaining config to `pack.toml`, `city.toml`, or discovered directories. | | `[workspace]` | Held city metadata and pack composition in one place | Split across the root `pack.toml`, `city.toml`, and `.gc/`. | -| `workspace.name` | Workspace identity | Transitional in this wave. Keep it in `city.toml` for the current 0.15.0 migration slice. Fresh `gc init` keeps it aligned with `pack.name`; `gc register` uses it when present, otherwise falls back to `pack.name`, and stores the selected registration name in the machine-local supervisor registry without backfilling `city.toml`. Full removal from `city.toml` still waits for the broader site-binding cutover; track [#602](https://github.com/gastownhall/gascity/issues/602). | +| `workspace.name` | Workspace identity | As of release v0.15.0, this remains transitional. Keep it in `city.toml` for the current migration slice. Fresh `gc init` keeps it aligned with `pack.name`; `gc register` uses it when present, otherwise falls back to `pack.name`, and stores the selected registration name in the machine-local supervisor registry without backfilling `city.toml`. Full removal from `city.toml` still waits for the broader site-binding cutover; track [#602](https://github.com/gastownhall/gascity/issues/602). | | `workspace.includes` | City-level pack composition | Move to `[imports.*]` in the root city `pack.toml`. | | `workspace.default_rig_includes` | Default pack composition for newly added rigs | Move to `[defaults.rig.imports]` in the root city `pack.toml`. This is the target shape, but loader-backed support is still tracked in [#360](https://github.com/gastownhall/gascity/issues/360). | | `[providers.*]` | Named provider presets | Usually move to `[providers.*]` in the root city `pack.toml`, unless the setting is truly deployment-only. | @@ -640,7 +645,7 @@ schema, plus the qualified rows that matter most during migration. | `[session_sleep]` | Sleep policy defaults | Keep in `city.toml`. | | `[convergence]` | Convergence limits | Keep in `city.toml`. | | `[[service]]` | Workspace-owned service declarations | Keep in `city.toml` if they are deployment-owned services. | -| `[agent_defaults]` | Defaults applied to agents in this city | Lives in both `pack.toml` (pack-wide portable defaults) and `city.toml` (city-level deployment overrides). City layers on top of pack. | +| `[agent_defaults]` | Defaults applied to agents in this city | Lives in both `pack.toml` (pack-wide portable defaults) and `city.toml` (city-level deployment overrides). City layers on top of pack. As of release v0.15.0, the actively-applied defaults are still narrow: `default_sling_formula` plus `[agent_defaults].append_fragments`. | ## Reference: Gas City 0.14.0 `pack.toml` elements to PackV2 diff --git a/docs/packv2/doc-agent-v2.md b/docs/packv2/doc-agent-v2.md index c6d989147..4582e2026 100644 --- a/docs/packv2/doc-agent-v2.md +++ b/docs/packv2/doc-agent-v2.md @@ -8,6 +8,11 @@ This is a companion to [doc-pack-v2.md](doc-pack-v2.md), which covers the pack/c > **Keeping in sync:** This file is the source of truth. When updating, edit here, then update the issue body with `gh issue edit 356 --repo gastownhall/gascity --body-file <(sed -n '/^---BEGIN ISSUE---$/,/^---END ISSUE---$/{ /^---/d; p; }' issues/doc-agent-v2.md)`. +> [!IMPORTANT] +> This document describes the pre-release Gas City v0.15.0 rollout. +> Some PackV2 surfaces are still under active development; release-gated +> caveats below use the form "As of release v0.15.0, ...". + ---BEGIN ISSUE--- ## Problem @@ -99,17 +104,22 @@ my-city/ ```toml # pack.toml — pack-wide defaults [agent_defaults] -provider = "claude" -scope = "rig" +default_sling_formula = "mol-do-work" ``` ```toml # city.toml — city-level overrides (optional) [agent_defaults] -model = "claude-sonnet-4-20250514" +append_fragments = ["operational-awareness"] ``` -These apply to every agent in the city. Individual agents override in their own `agent.toml`: +As of release v0.15.0, the actively-applied defaults are still narrow: +`default_sling_formula` plus `[agent_defaults].append_fragments` during +prompt rendering. Other `AgentDefaults` fields are parsed and composed, +but are not yet auto-inherited at runtime. Per-agent fields such as +`provider` and `scope` still live in `agents//agent.toml`. + +Individual agents override in their own `agent.toml`: ```toml # agents/mayor/agent.toml — only what differs from defaults @@ -389,21 +399,19 @@ The three-layer injection pipeline (inline templates → global_fragments → in #### Auto-append (opt-in) -For migration and convenience, an agent can opt into auto-appending fragments via `append_fragments` in agent.toml: - -```toml -# agents/polecat/agent.toml -append_fragments = ["operational-awareness", "command-glossary"] -``` - -City-wide defaults can set this for all agents: +For migration and convenience, city-wide or pack-wide defaults can +auto-append fragments via `[agent_defaults].append_fragments`: ```toml -# city.toml +# pack.toml or city.toml [agent_defaults] append_fragments = ["operational-awareness", "command-glossary"] ``` +Agent-local `append_fragments` remains a follow-up tracked in +[#671](https://github.com/gastownhall/gascity/issues/671); it is not part +of the supported migration contract as of release v0.15.0. + `append_fragments` only works on `.template.md` prompts. Plain `.md` prompts are inert — nothing is injected, no template engine runs. ### Implicit agents @@ -507,7 +515,7 @@ A rig patch can undo a city-level patch for that one rig. ## Scope and impact - **Breaking:** `[[agent]]` tables move to `agents/` directories. Migration tooling needed. -- **Config:** city.toml gains `[agent_defaults]` defaults section, loses `[[agent]]` tables. `agent.toml` is new per-agent. +- **Config:** city.toml gains canonical `[agent_defaults]` defaults, loses `[[agent]]` tables. `agent.toml` is new per-agent. `[agents]` remains a compatibility alias only. - **Prompts:** `.template.md` infix becomes required for template processing. Existing `.md` prompts using `{{` need renaming to `.template.md`. - **New features:** Skills, MCP TOML abstraction, `per-provider/` overlays, `template-fragments/` convention, `patches/` directory. - **Naming:** Current `[[rigs.overrides]]` renamed to `[[rigs.patches]]` for consistency with `[[patches.agent]]`. diff --git a/docs/packv2/doc-conformance-matrix.md b/docs/packv2/doc-conformance-matrix.md index 031111a83..036d1e12c 100644 --- a/docs/packv2/doc-conformance-matrix.md +++ b/docs/packv2/doc-conformance-matrix.md @@ -3,6 +3,11 @@ This document turns the reconciled pack/city v.next docs into an executable conformance plan for the current Pack/City v2 rollout. +> [!IMPORTANT] +> This document describes the pre-release Gas City v0.15.0 rollout. +> Some PackV2 surfaces are still under active development; release-gated +> caveats below use the form "As of release v0.15.0, ...". + The goal is not to restate the full design. The goal is to answer three practical questions: @@ -52,15 +57,16 @@ These are settled enough, and implemented enough, to block CI now. | Import target taxonomy | `source` stays the only public locator field; `gc import add` classifies the resolved target as plain directory, tagged git, untagged git, or invalid pack target, then synthesizes `version` accordingly (`none`, semver default, or `sha:`) | Unit + testscript | `cmd/gc/cmd_import.go`, `internal/packman/resolve.go` | | Rig imports | `[rigs.imports.]` in `city.toml` resolves for the targeted rig | Unit + testscript | `internal/config/pack.go`, `internal/config/compose.go` | | Agent discovery | `agents//` creates an agent without requiring `[[agent]]` | Unit | `internal/config/agent_discovery.go` | -| Current runtime provider resolution | Gate only the implemented runtime chain we are willing to freeze in this release wave: `agent.start_command` escape hatch, then `agent.provider`, then `workspace.provider`, then auto-detect; `workspace.start_command` is only the no-provider escape hatch. Do not treat the replacement/deprecation direction from `skew-analysis.md` as part of this row. | Unit | `internal/config/resolve.go` | +| Current runtime provider resolution | Gate only the implemented runtime chain we are willing to freeze as of release v0.15.0: `agent.start_command` escape hatch, then `agent.provider`, then `workspace.provider`, then auto-detect; `workspace.start_command` is only the no-provider escape hatch. Do not treat the replacement/deprecation direction from `skew-analysis.md` as part of this row. | Unit | `internal/config/resolve.go` | | Provider preset merge and lookup | Imported pack providers merge into the city provider map additively, city/local providers shadow imported ones on name collision, and provider lookup layers city overrides onto builtins when supported | Unit | `internal/config/pack.go`, `internal/config/resolve.go` | | Prompt naming | `prompt.md` is inert markdown and `prompt.template.md` enables template processing | Unit + testscript | `internal/config/agent_discovery.go`, `cmd/gc/prompt.go` | | Overlay discovery | pack-wide `overlay/` and agent-local `agents//overlay/` are discovered by convention | Unit | `internal/config/pack.go`, `internal/config/agent_discovery.go`, `internal/overlay/overlay.go` | | Provider overlay filtering | only `per-provider//` content for the effective provider is materialized | Unit | `internal/overlay/overlay.go` | | Namepool convention | `agents//namepool.txt` is discovered by convention | Unit | `internal/config/agent_discovery.go` | | Template fragments | `template-fragments/` and `agents//template-fragments/` are discovered and rendered into template prompts | Unit + testscript | `cmd/gc/prompt.go` | -| Agent-local auto-append bridge | `append_fragments` declared on an agent applies only to `.template.md` prompts and does nothing to plain `.md` prompts | Unit + testscript | `cmd/gc/prompt.go` | | `[agent_defaults]` auto-append bridge | `[agent_defaults].append_fragments` composes and auto-appends only for `.template.md` prompts | Unit + testscript | `internal/config/compose.go`, `cmd/gc/prompt.go` | +| `[agents]` compatibility alias | `[agents]` still parses as a compatibility alias for `[agent_defaults]`, normalizes to the canonical table, and emits a deprecation warning | Unit | `internal/config/compose.go`, `internal/config/undecoded.go` | +| Unsupported `AgentDefaults` migration keys | `[agent_defaults].provider`, `.scope`, and `.install_agent_hooks` emit migration-oriented warnings instead of generic unknown-field hints | Unit | `internal/config/undecoded.go` | | Agent defaults layering | `[agent_defaults]` is legal in both `pack.toml` and `city.toml`, with city winning on merge; runtime inheritance is gated only for fields the implementation actually applies today | Unit | `internal/config/compose.go`, `internal/config/config.go` | | Qualified patch targeting | imported agents can be targeted by qualified name in `[[patches.agent]]` | Unit | `internal/config/patch.go` | | Patch prompt template gating | An explicitly patched `prompt_template` path follows the same `.template.` rule as agent prompt files: `.template.md` renders, plain `.md` stays inert | Unit | `internal/config/patch.go`, `cmd/gc/prompt.go` | @@ -85,7 +91,7 @@ warning surface is implemented end to end. | Legacy prompt injection | `global_fragments`, `inject_fragments`, and `inject_fragments_append` emit deprecation warnings toward `append_fragments` or explicit `{{ template }}` | Testscript + unit | | Legacy fallback model | `fallback` emits a loud warning and is not part of the v.next authoring surface | Testscript | | Legacy path wiring | `prompt_template`, `overlay_dir`, and `namepool` on legacy agent definitions warn during migration-facing flows | Testscript | -| Workspace soft deprecations | `workspace.provider`, `workspace.start_command`, `workspace.install_agent_hooks`, `workspace.name`, and `workspace.prefix` warn with the documented replacement path, but warning coverage must not imply that runtime precedence already matches the post-migration ideal | Testscript | +| Workspace soft deprecations | `workspace.start_command`, `workspace.name`, `workspace.prefix`, and any future workspace-surface migration warnings must not imply that runtime precedence already matches a later design | Testscript | | Formula directory path | `[formulas].dir = "formulas"` soft-warns; any other value is rejected | Unit + testscript | | Rig override naming | `rig.overrides` is accepted with a soft warning in favor of `rig.patches` | Unit + testscript | | Fragment-only include | top-level `include` stays fragment-only and rejects pack-composition content such as `[imports]`, include-based composition, or `pack.toml` references | Unit + testscript | @@ -98,7 +104,7 @@ unsettled to be reliable release gates. | Area | Current status | Why it is non-gating for now | |---|---|---| | `[defaults.rig.imports]` loader support | documented intent, not implemented | Migration tooling may write it, but the loader does not yet honor it | -| `[agent_defaults] provider` driving runtime provider selection | migration target is documented, but runtime behavior is not aligned enough to gate | Current implementation still resolves runtime defaults through `workspace.provider` / `ResolveProvider`; locking in the future rule now would create false failures | +| Agent-local `append_fragments` | documented intent, not implemented | Current runtime only supports `[agent_defaults].append_fragments`; agent-local parity stays tracked in [#671](https://github.com/gastownhall/gascity/issues/671) | | `patches/` directory convention for imported prompt replacements | documented in v.next docs, not implemented | Current implementation still relies on explicit patch fields rather than full loader-discovered patch files | | Pack `skills/` discovery | documented, not implemented | First slice is current-city-pack only with list-only visibility; imported-pack catalogs are later | | `mcp/` TOML abstraction | documented, not implemented | Same first-slice scope as skills: current-city-pack only, list-only visibility first, provider projection later | @@ -156,8 +162,7 @@ that would materially raise confidence without exploding scope. - `internal/config/agent_discovery.go`: `agents//`, prompt naming, overlay and namepool conventions - `cmd/gc/prompt.go`: `.template.` gating, fragment lookup, and - `append_fragments` behavior for both agent-local and - `[agent_defaults]` sources + `[agent_defaults].append_fragments` behavior - `internal/overlay/overlay.go`: provider filtering and overlay layering - `internal/config/patch.go`: qualified-name patch targeting and patched prompt-template path handling diff --git a/docs/packv2/doc-loader-v2.md b/docs/packv2/doc-loader-v2.md index acf993092..ec8c09cad 100644 --- a/docs/packv2/doc-loader-v2.md +++ b/docs/packv2/doc-loader-v2.md @@ -6,6 +6,11 @@ > design captured here. > Read them side-by-side to see the diff. +> [!IMPORTANT] +> This document describes the pre-release Gas City v0.15.0 rollout. +> Some PackV2 surfaces are still under active development; release-gated +> caveats below use the form "As of release v0.15.0, ...". + ## Conceptual overview V2 reframes loading around five ideas, all of which are missing or weak in @@ -364,8 +369,8 @@ version = "^1.4" source = "./packs/gastown" [agent_defaults] -provider = "claude" -scope = "rig" +default_sling_formula = "mol-do-work" +append_fragments = ["operational-awareness"] [providers.claude] model = "claude-sonnet-4" @@ -568,7 +573,10 @@ visible to the city scope, including transitive re-exports): it under) and `PackName`. 4. Filter by `scope`: keep `scope="city"` and unscoped agents; drop `scope="rig"`. -5. Apply the pack's `[agent_defaults]` defaults to its own agents. +5. Compose the pack's `[agent_defaults]` defaults onto its own agents. + As of release v0.15.0, the actively-applied defaults are narrow: + `default_sling_formula` plus `append_fragments` during prompt + rendering. 6. Add to `City.Agents`. The city pack itself is processed last so its agents win against any @@ -655,9 +663,12 @@ explicitly configured or referenced. This logic is unchanged from V1. ### 18. Apply agent defaults -Same as V1 step 11: `[agent_defaults]` defaults from the city pack apply to all -agents that don't override. Imported pack `[agent_defaults]` defaults apply only -to that pack's own agents (already handled in step 11). +Same as V1 step 11, but only for the currently implemented +`[agent_defaults]` behavior: city-pack defaults apply to all agents that +don't override, and imported-pack defaults apply only to that pack's own +agents (already handled in step 11). As of release v0.15.0, the +actively-applied defaults are still narrow: `default_sling_formula` plus +`append_fragments` during prompt rendering. ### 19. Bind site state diff --git a/docs/packv2/doc-pack-v2.md b/docs/packv2/doc-pack-v2.md index ee23f783c..c8f8943bc 100644 --- a/docs/packv2/doc-pack-v2.md +++ b/docs/packv2/doc-pack-v2.md @@ -84,7 +84,8 @@ source = "./assets/maintenance" # Pack-wide agent defaults — individual agents defined in agents/ directories # City-level [agent_defaults] in city.toml can override these. [agent_defaults] -provider = "claude" +default_sling_formula = "mol-do-work" +append_fragments = ["operational-awareness"] [[named_session]] template = "mayor" @@ -174,8 +175,8 @@ name = "gastown" version = "1.2.0" [agent_defaults] -provider = "claude" -scope = "rig" +default_sling_formula = "mol-do-work" +append_fragments = ["operational-awareness"] ``` ``` diff --git a/docs/packv2/doc-packman.md b/docs/packv2/doc-packman.md index 040876667..a764c9a00 100644 --- a/docs/packv2/doc-packman.md +++ b/docs/packv2/doc-packman.md @@ -8,6 +8,11 @@ Companion to [doc-pack-v2.md](doc-pack-v2.md) ([gastownhall/gascity#360](https:/ > **Keeping in sync:** This file is the source of truth. When a GitHub issue is created, edit here, then update the issue body with `gh issue edit --repo gastownhall/gascity --body-file <(sed -n '/^---BEGIN ISSUE---$/,/^---END ISSUE---$/{ /^---/d; p; }' issues/doc-packman.md)`. +> [!IMPORTANT] +> This document describes the pre-release Gas City v0.15.0 rollout. +> Some PackV2 surfaces are still under active development; release-gated +> caveats below use the form "As of release v0.15.0, ...". + ## Status update — 2026-04-10 The merge-wave design decisions now settled around `packs.lock` are: @@ -24,8 +29,8 @@ The merge-wave design decisions now settled around `packs.lock` are: - no version-lock / upgrade semantics - `gc import install` should fail if the city depends on them, because install is the reproducibility gate -- `pack.version` remains descriptive metadata for now. It does not - become the managed remote version source in this wave. +- As of release v0.15.0, `pack.version` remains descriptive metadata. It + does not become the managed remote version source. - The remaining Track 6 work should be a narrow loader-read-path harvest that fits this contract, not a wholesale merge of the old branch. diff --git a/docs/packv2/skew-analysis.md b/docs/packv2/skew-analysis.md index 9452fdbb5..aa845687e 100644 --- a/docs/packv2/skew-analysis.md +++ b/docs/packv2/skew-analysis.md @@ -4,7 +4,12 @@ > from the release branch Go structs) against the reconciled pack v2 specs. > Revised through field-by-field walkthrough to reflect the **current > Pack/City v2 desired state** — not the ideal end-state, but what should -> ship in this release wave. +> ship as of release v0.15.0. + +> [!IMPORTANT] +> This document describes the pre-release Gas City v0.15.0 rollout. +> Some PackV2 surfaces are still under active development; release-gated +> caveats below use the form "As of release v0.15.0, ...". ## Color key @@ -66,7 +71,7 @@ | 🟢 | `named_session` | []NamedSession | **Keep.** Legal in both pack.toml and city.toml, city wins. | | 🟢 | `rigs` | []Rig | **Keep in city.toml.** | | 🟢 | `patches` | Patches | **Keep.** `[[patches.agent]]` and `[[patches.providers]]` legal in both, city wins. `[[patches.rigs]]` city.toml only. | -| 🟢 | `agent_defaults` | AgentDefaults | **Keep.** Legal in both pack.toml and city.toml, city wins. Surface stays as-is (no expansion in this wave). | +| 🟢 | `agent_defaults` | AgentDefaults | **Keep.** Legal in both pack.toml and city.toml, city wins. As of release v0.15.0, the surface stays as-is (no expansion). | | 🟢 | `providers` | map[string]ProviderSpec | **Keep.** Legal in both, city wins. | | 🟡 | `formulas` | FormulasConfig | See `[formulas].dir` below. | | 🟢 | `beads` | BeadsConfig | **Keep in city.toml.** | @@ -86,14 +91,14 @@ | Status | Field | As-built | Current rollout disposition | Later destination | |--------|-------|----------|--------------------|-----------------------| -| 🟡 | `name` | Required string | **Optional.** Transitional runtime identity field in this wave. Fresh `gc init` keeps it aligned with `pack.name`; `gc register` reads it when present but stores registration aliases in the machine-local supervisor registry without backfilling `city.toml`. Soft warning: full site-binding cutover remains later. | `.gc/` site binding (#600) | -| 🟡 | `prefix` | String | **Optional.** Same treatment as `name`. Soft warning. | `.gc/` site binding (#600) | -| 🟡 | `provider` | String | **Soft warning.** "Use `[agent_defaults] provider = ...` instead." | `[agent_defaults]` in pack.toml | +| 🟡 | `name` | Required string | **Optional.** As of release v0.15.0, this remains a transitional runtime identity field. Fresh `gc init` keeps it aligned with `pack.name`; `gc register` reads it when present but stores registration aliases in the machine-local supervisor registry without backfilling `city.toml`. Soft warning: full site-binding cutover remains later. | `.gc/` site binding (#600) | +| 🟡 | `prefix` | String | **Optional.** As of release v0.15.0, this gets the same treatment as `name`. Soft warning. | `.gc/` site binding (#600) | +| 🟢 | `provider` | String | **Keep.** As of release v0.15.0, this is still the current runtime default-provider field. Corresponding `[agent_defaults].provider` is still unsupported and emits a migration warning — see `doc-conformance-matrix.md`. | Later default-provider redesign, not part of this rollout | | 🟡 | `start_command` | String | **Soft warning.** "Use per-agent `start_command` in `agent.toml` instead." | Per-agent `agent.toml` | | 🟡 | `suspended` | Boolean | **Soft warning.** "Use `gc suspend`/`gc resume` instead." | `.gc/` site binding | | 🟢 | `max_active_sessions` | Integer | **Keep as-is.** Deployment capacity. | Top-level city.toml field when `[workspace]` is dismantled | | 🟢 | `session_template` | String | **Keep as-is.** Deployment. | `[session]` when `[workspace]` is dismantled | -| 🟡 | `install_agent_hooks` | []string | **Soft warning.** "Use `[agent_defaults]` instead." | `[agent_defaults]` in pack.toml | +| 🟢 | `install_agent_hooks` | []string | **Keep.** As of release v0.15.0, no `[agent_defaults]` replacement is implemented yet; `[agent_defaults].install_agent_hooks` still warns. | Later hooks-default redesign | | 🟡 | `global_fragments` | []string | **Soft warning.** "Use `[agent_defaults] append_fragments` or explicit `{{ template }}` instead." | Removed (replaced by template-fragments) | | 🟡 | `includes` | []string | **Loud warning on schema 2.** V1 composition, use `[imports]`. | Removed | | 🟡 | `default_rig_includes` | []string | **Loud warning on schema 2.** Use `[defaults.rig.imports]` in pack.toml. | Removed | @@ -121,13 +126,14 @@ In this rollout, `[[agent]]` gets a loud warning on schema 2. Agent fields below ### Legal in agent.toml -All other agent fields are legal in `agent.toml`. `[agent_defaults]` surface stays as-is in this wave (no expansion). +All other agent fields are legal in `agent.toml`. As of release +v0.15.0, the `[agent_defaults]` surface stays as-is (no expansion). | Status | Field | Notes | |--------|-------|-------| | 🟢 | `description` | | | 🟢 | `scope` | `"city"` or `"rig"` | -| 🟢 | `suspended` | Stays in agent.toml in this wave; moves to `.gc/` post-release | +| 🟢 | `suspended` | As of release v0.15.0, stays in agent.toml; moves to `.gc/` post-release | | 🟢 | `provider` | | | 🟢 | `start_command` | | | 🟢 | `args` | | @@ -153,7 +159,7 @@ All other agent fields are legal in `agent.toml`. `[agent_defaults]` surface sta | 🟢 | `session_setup` | | | 🟢 | `session_setup_script` | Path resolves against pack root | | 🟢 | `session_live` | | -| 🟢 | `install_agent_hooks` | Overrides agent_defaults | +| 🟢 | `install_agent_hooks` | Overrides workspace.install_agent_hooks | | 🟢 | `hooks_installed` | | | 🟢 | `idle_timeout` | | | 🟢 | `sleep_after_idle` | | @@ -175,7 +181,8 @@ All other agent fields are legal in `agent.toml`. `[agent_defaults]` surface sta | 🟢 | `allow_env_override` | Present | **Keep.** Not yet auto-applied at runtime. | | 🟢 | `append_fragments` | Present | **Keep.** Migration bridge for global_fragments/inject_fragments. | -No expansion of `[agent_defaults]` surface in this wave. +As of release v0.15.0, there is no expansion of the +`[agent_defaults]` surface. ## FormulasConfig @@ -220,9 +227,9 @@ All Import fields match spec. No changes needed. |--------|-------|----------|--------------------| | 🟡 | `inject_fragments` | Present | **Loud warning.** V1 remnant. | | 🟡 | `inject_fragments_append` | Present | **Loud warning.** V1 remnant. | -| 🟢 | `prompt_template` | Path string | **Keep in this wave.** Post-release: convention-based via `patches/`. | -| 🟢 | `overlay_dir` | Path string | **Keep in this wave.** Post-release: convention-based. | -| 🟢 | `dir` + `name` targeting (AgentPatch) | Present | **Keep in this wave.** Qualified name targeting already works. | +| 🟢 | `prompt_template` | Path string | **Keep.** As of release v0.15.0, this remains supported. Post-release: convention-based via `patches/`. | +| 🟢 | `overlay_dir` | Path string | **Keep.** As of release v0.15.0, this remains supported. Post-release: convention-based. | +| 🟢 | `dir` + `name` targeting (AgentPatch) | Present | **Keep.** As of release v0.15.0, qualified name targeting already works. | | 🟢 | All other override fields | Present | **Keep.** | ## PackSource diff --git a/docs/reference/config.md b/docs/reference/config.md index 72859bcc7..8107971f4 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -32,7 +32,7 @@ City is the top-level configuration for a Gas City instance. | `session_sleep` | SessionSleepConfig | | | SessionSleep configures idle sleep policy defaults for managed sessions. | | `convergence` | ConvergenceConfig | | | Convergence configures convergence loop limits. | | `service` | []Service | | | Services declares workspace-owned HTTP services mounted on the controller edge under /svc/{name}. | -| `agent_defaults` | AgentDefaults | | | AgentDefaults provides city-level defaults for agents that don't override them (canonical TOML key: agent_defaults). The runtime currently applies default_sling_formula plus shared skill/MCP attachment baselines; other fields are parsed/composed but not yet inherited automatically. | +| `agent_defaults` | AgentDefaults | | | AgentDefaults provides city-level defaults for agents that don't override them (canonical TOML key: agent_defaults). The runtime currently applies default_sling_formula and uses append_fragments during prompt rendering; other fields are parsed/composed but not yet inherited automatically. | ## ACPSessionConfig diff --git a/docs/schema/city-schema.json b/docs/schema/city-schema.json index fdcc88e79..f455498a6 100644 --- a/docs/schema/city-schema.json +++ b/docs/schema/city-schema.json @@ -971,7 +971,7 @@ }, "agent_defaults": { "$ref": "#/$defs/AgentDefaults", - "description": "AgentDefaults provides city-level defaults for agents that don't\noverride them (canonical TOML key: agent_defaults). The runtime\ncurrently applies default_sling_formula plus shared skill/MCP\nattachment baselines; other fields are parsed/composed but not yet\ninherited automatically." + "description": "AgentDefaults provides city-level defaults for agents that don't\noverride them (canonical TOML key: agent_defaults). The runtime\ncurrently applies default_sling_formula and uses append_fragments\nduring prompt rendering; other fields are parsed/composed but not\nyet inherited automatically." } }, "additionalProperties": false, diff --git a/internal/config/compose.go b/internal/config/compose.go index 5b61a7293..c49173f7e 100644 --- a/internal/config/compose.go +++ b/internal/config/compose.go @@ -79,14 +79,14 @@ func LoadWithIncludesOptions(fs fsys.FS, path string, opts LoadOptions, extraInc packPath := filepath.Join(cityRoot, packFile) if packData, pErr := fs.ReadFile(packPath); pErr == nil { packExists = true - var pc packConfig - md, decErr := toml.Decode(string(packData), &pc) + pc, md, packWarnings, decErr := parsePackConfigWithMetadata(packData, packPath) if decErr != nil { return nil, nil, fmt.Errorf("parsing city pack.toml: %w", decErr) } - if warnings := CheckUndecodedKeys(md, packPath); len(warnings) > 0 { - return nil, nil, fmt.Errorf("parsing city pack.toml: %s", strings.Join(warnings, "; ")) + if fatalWarnings := fatalUndecodedWarnings(md, packPath); len(fatalWarnings) > 0 { + return nil, nil, fmt.Errorf("parsing city pack.toml: %s", strings.Join(fatalWarnings, "; ")) } + prov.Warnings = append(prov.Warnings, packWarnings...) if err := validatePackMeta(&pc.Pack); err != nil { return nil, nil, fmt.Errorf("city pack.toml: %w", err) } @@ -369,6 +369,9 @@ func LoadWithIncludesOptions(fs fsys.FS, path string, opts LoadOptions, extraInc } cityReqs = append(cityReqs, rootPackRequires...) prov.Warnings = append(prov.Warnings, shadowWarnings...) + if len(root.LoadWarnings) > 0 { + prov.Warnings = appendUnique(prov.Warnings, root.LoadWarnings...) + } // Track city pack agents in provenance. for _, ref := range root.Workspace.Includes { topoDir, _ := resolvePackRef(ref, cityRoot, cityRoot) @@ -396,6 +399,9 @@ func LoadWithIncludesOptions(fs fsys.FS, path string, opts LoadOptions, extraInc if err := expandPacks(root, fs, cityRoot, rigFormulaDirs, opts); err != nil { return nil, nil, fmt.Errorf("expanding packs: %w", err) } + if len(root.LoadWarnings) > 0 { + prov.Warnings = appendUnique(prov.Warnings, root.LoadWarnings...) + } // Track pack-expanded agents in provenance. for _, r := range root.Rigs { topoRefs := r.Includes @@ -467,7 +473,9 @@ func LoadWithIncludesOptions(fs fsys.FS, path string, opts LoadOptions, extraInc // still populates the v0.15.0 attachment-list tombstone fields. The // fields still parse (TOML won't error) but are ignored by the new // materializer. - WarnDeprecatedAttachmentFields(root) + if warning := WarnDeprecatedAttachmentFields(root); warning != "" { + prov.Warnings = append(prov.Warnings, warning) + } siteBindingWarnings, err := ApplySiteBindings(fs, cityRoot, root) if err != nil { @@ -1023,7 +1031,8 @@ func parseWithMeta(data []byte, source string) (*City, toml.MetaData, []string, return nil, md, nil, fmt.Errorf("parsing config: %w", err) } normalizeAgentDefaultsAlias(&cfg, md) - warnings := CheckUndecodedKeys(md, source) + warnings := agentDefaultsCompatibilityWarnings(md, source) + warnings = append(warnings, CheckUndecodedKeys(md, source)...) return &cfg, md, warnings, nil } diff --git a/internal/config/compose_test.go b/internal/config/compose_test.go index 180fa7a49..0cf364977 100644 --- a/internal/config/compose_test.go +++ b/internal/config/compose_test.go @@ -271,6 +271,340 @@ mcp = ["shared-mcp", "common-mcp"] } } +func TestLoadWithIncludes_WarnsOnPackAgentDefaultsCompatibilityAndMigrationKeys(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(` +[workspace] +name = "test" + +[[rigs]] +name = "hw" +path = "/tmp/hw" +includes = ["packs/rigpack"] +`) + fs.Files["/city/pack.toml"] = []byte(` +[pack] +name = "test" +schema = 2 + +[agents] +append_fragments = ["root-footer"] +`) + fs.Files["/city/packs/rigpack/pack.toml"] = []byte(` +[pack] +name = "rigpack" +schema = 2 + +[agent_defaults] +provider = "claude" + +[[agent]] +name = "worker" +scope = "rig" +`) + + cfg, prov, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + if got := cfg.AgentDefaults.AppendFragments; len(got) != 1 || got[0] != "root-footer" { + t.Fatalf("AgentDefaults.AppendFragments = %v, want [root-footer]", got) + } + warnings := strings.Join(prov.Warnings, "\n") + if !strings.Contains(warnings, "/city/pack.toml: "+agentsAliasWarning) { + t.Fatalf("expected root pack alias warning, got: %v", prov.Warnings) + } + if !strings.Contains(warnings, `/city/packs/rigpack/pack.toml: "agent_defaults.provider" is not supported`) { + t.Fatalf("expected rig pack migration warning, got: %v", prov.Warnings) + } +} + +func TestLoadWithIncludes_ImportedPackAgentDefaultsLayerIntoEffectiveFormula(t *testing.T) { + tests := []struct { + name string + cityDefault string + nested bool + want string + }{ + {name: "pack default only", want: "mol-pack"}, + {name: "city override wins", cityDefault: "mol-city", want: "mol-city"}, + {name: "nested pack include inherits default", nested: true, want: "mol-pack"}, + {name: "city override wins for nested pack include", cityDefault: "mol-city", nested: true, want: "mol-city"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs := fsys.NewFake() + cityDefaults := "" + if tt.cityDefault != "" { + cityDefaults = "\n[agent_defaults]\ndefault_sling_formula = \"" + tt.cityDefault + "\"\n" + } + fs.Files["/city/city.toml"] = []byte(` +[workspace] +name = "test" +includes = ["packs/imported"] +` + cityDefaults) + if tt.nested { + fs.Files["/city/packs/imported/pack.toml"] = []byte(` +[pack] +name = "imported" +schema = 2 +includes = ["../base"] + +[agent_defaults] +default_sling_formula = "mol-pack" +`) + fs.Files["/city/packs/base/pack.toml"] = []byte(` +[pack] +name = "base" +schema = 2 + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +`) + } else { + fs.Files["/city/packs/imported/pack.toml"] = []byte(` +[pack] +name = "imported" +schema = 2 + +[agent_defaults] +default_sling_formula = "mol-pack" + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +`) + } + + cfg, _, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + if len(explicitAgents(cfg.Agents)) != 1 { + t.Fatalf("len(explicit agents) = %d, want 1", len(explicitAgents(cfg.Agents))) + } + got := explicitAgents(cfg.Agents)[0].EffectiveDefaultSlingFormula() + if got != tt.want { + t.Fatalf("EffectiveDefaultSlingFormula = %q, want %q", got, tt.want) + } + }) + } +} + +func TestLoadWithIncludes_PackAgentDefaultsMergesNonOverlappingAgentsAliasFields(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(` +[workspace] +name = "test" +includes = ["packs/imported"] +`) + fs.Files["/city/packs/imported/pack.toml"] = []byte(` +[pack] +name = "imported" +schema = 2 + +[agent_defaults] +append_fragments = ["canonical-footer"] + +[agents] +default_sling_formula = "mol-legacy" + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +`) + + cfg, _, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + if len(explicitAgents(cfg.Agents)) != 1 { + t.Fatalf("len(explicit agents) = %d, want 1", len(explicitAgents(cfg.Agents))) + } + agent := explicitAgents(cfg.Agents)[0] + if got := agent.EffectiveDefaultSlingFormula(); got != "mol-legacy" { + t.Fatalf("EffectiveDefaultSlingFormula = %q, want %q", got, "mol-legacy") + } + if !reflect.DeepEqual(agent.InheritedAppendFragments, []string{"canonical-footer"}) { + t.Fatalf("InheritedAppendFragments = %v, want %v", agent.InheritedAppendFragments, []string{"canonical-footer"}) + } +} + +func TestLoadWithIncludes_ImportedPackWarningsSurfaceInProvenanceWithoutRigPacks(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(` +[workspace] +name = "test" +includes = ["packs/imported"] +`) + fs.Files["/city/packs/imported/pack.toml"] = []byte(` +[pack] +name = "imported" +schema = 2 + +[agents] +append_fragments = ["footer"] +`) + + _, prov, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + warnings := strings.Join(prov.Warnings, "\n") + if !strings.Contains(warnings, "/city/packs/imported/pack.toml: "+agentsAliasWarning) { + t.Fatalf("expected imported-pack alias warning in provenance, got: %v", prov.Warnings) + } +} + +func TestLoadWithIncludes_WrapperPackDefaultsDoNotBleedAcrossImports(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(` +[workspace] +name = "test" +includes = ["packs/wrapper"] +`) + fs.Files["/city/packs/wrapper/pack.toml"] = []byte(` +[pack] +name = "wrapper" +schema = 2 + +[agent_defaults] +default_sling_formula = "mol-wrapper" + +[imports.dep] +source = "../dep" +`) + fs.Files["/city/packs/dep/pack.toml"] = []byte(` +[pack] +name = "dep" +schema = 2 + +[agent_defaults] +default_sling_formula = "mol-dep" + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +`) + + cfg, _, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + for _, a := range explicitAgents(cfg.Agents) { + if a.BindingName != "dep" || a.Name != "mayor" { + continue + } + if got := a.EffectiveDefaultSlingFormula(); got != "mol-dep" { + t.Fatalf("dep.mayor EffectiveDefaultSlingFormula = %q, want %q", got, "mol-dep") + } + return + } + t.Fatalf("expected dep.mayor agent, got %v", explicitAgents(cfg.Agents)) +} + +func TestLoadWithIncludes_IncludingPackDefaultsKeepInnermostScalarDefault(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(` +[workspace] +name = "test" +includes = ["packs/outer"] +`) + fs.Files["/city/packs/outer/pack.toml"] = []byte(` +[pack] +name = "outer" +schema = 2 +includes = ["../base"] + +[agent_defaults] +default_sling_formula = "mol-outer" +`) + fs.Files["/city/packs/base/pack.toml"] = []byte(` +[pack] +name = "base" +schema = 2 + +[agent_defaults] +default_sling_formula = "mol-base" + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +`) + + cfg, _, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + if len(explicitAgents(cfg.Agents)) != 1 { + t.Fatalf("len(explicit agents) = %d, want 1", len(explicitAgents(cfg.Agents))) + } + if got := explicitAgents(cfg.Agents)[0].EffectiveDefaultSlingFormula(); got != "mol-base" { + t.Fatalf("EffectiveDefaultSlingFormula = %q, want %q", got, "mol-base") + } +} + +func TestLoadWithIncludes_IncludingPackDefaultsDoNotBleedAcrossNestedImportBoundaries(t *testing.T) { + fs := fsys.NewFake() + fs.Files["/city/city.toml"] = []byte(` +[workspace] +name = "test" +includes = ["packs/outer"] +`) + fs.Files["/city/packs/outer/pack.toml"] = []byte(` +[pack] +name = "outer" +schema = 2 +includes = ["../mid"] + +[agent_defaults] +default_sling_formula = "mol-outer" +`) + fs.Files["/city/packs/mid/pack.toml"] = []byte(` +[pack] +name = "mid" +schema = 2 + +[imports.dep] +source = "../dep" +`) + fs.Files["/city/packs/dep/pack.toml"] = []byte(` +[pack] +name = "dep" +schema = 2 + +[agent_defaults] +default_sling_formula = "mol-dep" + +[[agent]] +name = "mayor" +provider = "claude" +scope = "city" +`) + + cfg, _, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + for _, a := range explicitAgents(cfg.Agents) { + if a.BindingName != "dep" || a.Name != "mayor" { + continue + } + if got := a.EffectiveDefaultSlingFormula(); got != "mol-dep" { + t.Fatalf("dep.mayor EffectiveDefaultSlingFormula = %q, want %q", got, "mol-dep") + } + return + } + t.Fatalf("expected dep.mayor agent, got %v", explicitAgents(cfg.Agents)) +} + func TestLoadWithIncludes_ConcatRigs(t *testing.T) { fs := fsys.NewFake() fs.Files["/city/city.toml"] = []byte(` diff --git a/internal/config/config.go b/internal/config/config.go index 1d8815182..2f0c70182 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,8 +4,6 @@ package config import ( "bytes" "fmt" - "io" - "os" "path/filepath" "regexp" "sort" @@ -173,14 +171,18 @@ type City struct { Services []Service `toml:"service,omitempty"` // AgentDefaults provides city-level defaults for agents that don't // override them (canonical TOML key: agent_defaults). The runtime - // currently applies default_sling_formula plus shared skill/MCP - // attachment baselines; other fields are parsed/composed but not yet - // inherited automatically. + // currently applies default_sling_formula and uses append_fragments + // during prompt rendering; other fields are parsed/composed but not + // yet inherited automatically. AgentDefaults AgentDefaults `toml:"agent_defaults,omitempty"` // AgentsDefaults is a temporary compatibility alias for [agent_defaults]. // Parse/load normalize it into AgentDefaults and prefer [agent_defaults] // when both tables are present. AgentsDefaults AgentDefaults `toml:"agents,omitempty" jsonschema:"-"` + // LoadWarnings accumulates non-fatal warnings discovered while expanding + // imported packs so LoadWithIncludes can surface them through provenance. + // Runtime-only — not persisted to TOML or JSON. + LoadWarnings []string `toml:"-" json:"-"` // ResolvedWorkspaceName is the effective city name derived from the // config file path when workspace.name is omitted. Runtime-only. ResolvedWorkspaceName string `toml:"-" json:"-"` @@ -614,6 +616,8 @@ type PackMeta struct { Version string `toml:"version"` // Schema is the pack format version (currently 1). Schema int `toml:"schema" jsonschema:"required"` + // Description is an optional human-readable pack summary. + Description string `toml:"description,omitempty"` // RequiresGC is an optional minimum gc version requirement. RequiresGC string `toml:"requires_gc,omitempty"` // Includes lists other packs to compose into this one (V1 mechanism). @@ -1382,8 +1386,9 @@ func (c *City) FormulasDir() string { // AgentDefaults provides city-level agent defaults declared via // [agent_defaults] in city.toml. The runtime currently applies -// default_sling_formula and append_fragments; the remaining fields are -// parsed and composed but are not yet inherited onto agents automatically. +// default_sling_formula and uses append_fragments during prompt +// rendering; the remaining fields are parsed and composed but are not +// yet inherited onto agents automatically. type AgentDefaults struct { // Model is the parsed/composed default model name for agents // (e.g., "claude-sonnet-4-6"), but it is not yet auto-applied at @@ -1423,8 +1428,38 @@ type AgentDefaults struct { MCP []string `toml:"mcp,omitempty"` } +func mergeAgentDefaultsAliasPreferCanonical(dst *AgentDefaults, src AgentDefaults, meta toml.MetaData) { + if !meta.IsDefined("agent_defaults", "model") { + dst.Model = src.Model + } + if !meta.IsDefined("agent_defaults", "wake_mode") { + dst.WakeMode = src.WakeMode + } + if !meta.IsDefined("agent_defaults", "default_sling_formula") { + dst.DefaultSlingFormula = src.DefaultSlingFormula + } + if !meta.IsDefined("agent_defaults", "allow_overlay") { + dst.AllowOverlay = append([]string(nil), src.AllowOverlay...) + } + if !meta.IsDefined("agent_defaults", "allow_env_override") { + dst.AllowEnvOverride = append([]string(nil), src.AllowEnvOverride...) + } + if !meta.IsDefined("agent_defaults", "append_fragments") { + dst.AppendFragments = append([]string(nil), src.AppendFragments...) + } + if !meta.IsDefined("agent_defaults", "skills") { + dst.Skills = append([]string(nil), src.Skills...) + } + if !meta.IsDefined("agent_defaults", "mcp") { + dst.MCP = append([]string(nil), src.MCP...) + } +} + func normalizeAgentDefaultsAlias(cfg *City, meta toml.MetaData) { if meta.IsDefined("agent_defaults") { + if meta.IsDefined("agents") { + mergeAgentDefaultsAliasPreferCanonical(&cfg.AgentDefaults, cfg.AgentsDefaults, meta) + } cfg.AgentsDefaults = AgentDefaults{} return } @@ -1633,10 +1668,21 @@ type Agent struct { // when beads are slung to this agent, unless --no-formula is set. // Example: "mol-polecat-work" DefaultSlingFormula *string `toml:"default_sling_formula,omitempty"` + // InheritedDefaultSlingFormula records the pack-scoped default formula for + // agents loaded from imported packs. City-level [agent_defaults] can still + // override it later because the explicit DefaultSlingFormula pointer remains + // nil until a higher-precedence layer sets it. + // Runtime-only — not persisted to TOML or JSON. + InheritedDefaultSlingFormula *string `toml:"-" json:"-"` // InjectFragments lists named template fragments to append to this agent's // rendered prompt. Fragments come from shared template directories across // all loaded packs. Each name must match a {{ define "name" }} block. InjectFragments []string `toml:"inject_fragments,omitempty"` + // InheritedAppendFragments records pack-scoped append_fragments inherited + // from an imported pack's [agent_defaults]. City-level append_fragments are + // layered separately during prompt rendering. + // Runtime-only — not persisted to TOML or JSON. + InheritedAppendFragments []string `toml:"-" json:"-"` // InjectAssignedSkills controls whether gc appends an // "assigned skills" appendix to the agent's rendered prompt. The // appendix lists every skill visible to this agent, partitioned @@ -1843,10 +1889,13 @@ func (a *Agent) DefaultSlingQuery() string { // EffectiveDefaultSlingFormula returns the default sling formula for // this agent, or "" if none is set. func (a *Agent) EffectiveDefaultSlingFormula() string { - if a.DefaultSlingFormula == nil { - return "" + if a.DefaultSlingFormula != nil { + return *a.DefaultSlingFormula } - return *a.DefaultSlingFormula + if a.InheritedDefaultSlingFormula != nil { + return *a.InheritedDefaultSlingFormula + } + return "" } // DrainTimeoutDuration returns the drain timeout as a time.Duration. @@ -2099,24 +2148,18 @@ func ApplyAgentDefaults(cfg *City) { // warning test can assert on its substring. const deprecatedAttachmentWarning = "gc: warning: attachment-list fields (`skills`, `mcp`, `skills_append`, `mcp_append`, `shared_skills`) are deprecated as of v0.15.1 and ignored. They may appear on agents, [agent_defaults], [[patches.agent]], [[rigs.overrides]], or [[rigs.patches]]. Remove them from your config (or run `gc doctor --fix` once available). Hard parse error lands in v0.16." -// deprecationWarningSink is the writer used by WarnDeprecatedAttachmentFields. -// Overridable from tests. -var deprecationWarningSink io.Writer = os.Stderr - -// WarnDeprecatedAttachmentFields emits a one-time deprecation warning to -// deprecationWarningSink (defaults to os.Stderr) if any of the v0.15.0 -// attachment-list tombstone fields appears populated anywhere in the -// loaded config — agents, agent_defaults, patches, or rig-level overrides. -// The check is best-effort and does not error; it only notifies the user -// so they can clean up ahead of the v0.16 hard parse error. -func WarnDeprecatedAttachmentFields(cfg *City) { +// WarnDeprecatedAttachmentFields returns the canonical deprecation warning if +// any v0.15.0 attachment-list tombstone field appears populated anywhere in +// the loaded config — agents, agent_defaults, patches, or rig-level overrides. +// Callers are responsible for routing the warning through their chosen sink. +func WarnDeprecatedAttachmentFields(cfg *City) string { if cfg == nil { - return + return "" } if !hasDeprecatedAttachmentFields(cfg) { - return + return "" } - fmt.Fprintln(deprecationWarningSink, deprecatedAttachmentWarning) //nolint:errcheck // best-effort warning sink + return deprecatedAttachmentWarning } func hasDeprecatedAttachmentFields(cfg *City) bool { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index befbdc428..aed987e5a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,7 +1,6 @@ package config import ( - "bytes" "fmt" "os" "os/exec" @@ -198,6 +197,36 @@ append_fragments = [] } } +func TestParseAgentDefaultsMergesNonOverlappingAgentsAliasFields(t *testing.T) { + data := []byte(` +[workspace] +name = "test-city" + +[agent_defaults] +append_fragments = ["canonical-fragment"] + +[agents] +default_sling_formula = "mol-legacy" +skills = ["shared-skill"] +`) + cfg, err := Parse(data) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if got := cfg.AgentDefaults.DefaultSlingFormula; got != "mol-legacy" { + t.Errorf("AgentDefaults.DefaultSlingFormula = %q, want %q", got, "mol-legacy") + } + if !reflect.DeepEqual(cfg.AgentDefaults.AppendFragments, []string{"canonical-fragment"}) { + t.Errorf("AgentDefaults.AppendFragments = %v, want %v", cfg.AgentDefaults.AppendFragments, []string{"canonical-fragment"}) + } + if !reflect.DeepEqual(cfg.AgentDefaults.Skills, []string{"shared-skill"}) { + t.Errorf("AgentDefaults.Skills = %v, want %v", cfg.AgentDefaults.Skills, []string{"shared-skill"}) + } + if !reflect.DeepEqual(cfg.AgentsDefaults, AgentDefaults{}) { + t.Errorf("AgentsDefaults = %#v, want zero value after normalization", cfg.AgentsDefaults) + } +} + func TestParseNoAgents(t *testing.T) { data := []byte(` [workspace] @@ -4442,24 +4471,10 @@ scale_check = "echo 5" } } -// withDeprecationWarningSink swaps the package-level sink for a capturing -// buffer and returns a restore function. Tests should defer restore() so -// the sink is restored even if the test fails. -func withDeprecationWarningSink(t *testing.T) (*bytes.Buffer, func()) { - t.Helper() - var buf bytes.Buffer - prev := deprecationWarningSink - deprecationWarningSink = &buf - return &buf, func() { deprecationWarningSink = prev } -} - // TestLoadWithIncludes_DeprecatedAttachmentWarning confirms that a config // containing the v0.15.0 attachment-list tombstone fields still parses, -// and that a single deprecation warning is emitted to the sink. +// and that a single deprecation warning is surfaced through provenance. func TestLoadWithIncludes_DeprecatedAttachmentWarning(t *testing.T) { - buf, restore := withDeprecationWarningSink(t) - defer restore() - fs := fsys.NewFake() fs.Files["/city/city.toml"] = []byte(` [workspace] @@ -4471,7 +4486,7 @@ skills = ["code-review", "incident-response"] mcp = ["beads-health"] `) - cfg, _, err := LoadWithIncludes(fs, "/city/city.toml") + cfg, prov, err := LoadWithIncludes(fs, "/city/city.toml") if err != nil { t.Fatalf("LoadWithIncludes: %v", err) } @@ -4495,13 +4510,13 @@ mcp = ["beads-health"] t.Errorf("mayor.MCP = %v, want tombstone parse-through", mayor.MCP) } - got := buf.String() + got := strings.Join(prov.Warnings, "\n") if !strings.Contains(got, "deprecated as of v0.15.1 and ignored") { - t.Fatalf("deprecation warning not emitted, got:\n%s", got) + t.Fatalf("deprecation warning not surfaced, got:\n%s", got) } - // Exactly one warning line — the emission is one-per-load. + // Exactly one warning line — the warning is one-per-load. if n := strings.Count(got, "gc: warning:"); n != 1 { - t.Errorf("emitted %d warnings, want exactly 1\nstderr:\n%s", n, got) + t.Errorf("emitted %d warnings, want exactly 1\nwarnings:\n%s", n, got) } } @@ -4511,9 +4526,6 @@ mcp = ["beads-health"] // [[rigs.overrides]]. Prior to this test, the scan only covered // rig.Overrides and would silently miss the rig.RigPatches surface. func TestLoadWithIncludes_DeprecatedAttachmentWarning_RigPatches(t *testing.T) { - buf, restore := withDeprecationWarningSink(t) - defer restore() - fs := fsys.NewFake() fs.Files["/city/city.toml"] = []byte(` [workspace] @@ -4528,25 +4540,23 @@ name = "polecat" skills_append = ["incident-response"] `) - if _, _, err := LoadWithIncludes(fs, "/city/city.toml"); err != nil { + _, prov, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { t.Fatalf("LoadWithIncludes: %v", err) } - got := buf.String() + got := strings.Join(prov.Warnings, "\n") if !strings.Contains(got, "deprecated as of v0.15.1 and ignored") { t.Fatalf("rig.patches attachment fields should trigger deprecation warning, got:\n%s", got) } if n := strings.Count(got, "gc: warning:"); n != 1 { - t.Errorf("emitted %d warnings, want exactly 1\nstderr:\n%s", n, got) + t.Errorf("emitted %d warnings, want exactly 1\nwarnings:\n%s", n, got) } } // TestLoadWithIncludes_NoAttachmentsSilent confirms that a clean config // (no attachment-list tombstone fields) emits no deprecation warning. func TestLoadWithIncludes_NoAttachmentsSilent(t *testing.T) { - buf, restore := withDeprecationWarningSink(t) - defer restore() - fs := fsys.NewFake() fs.Files["/city/city.toml"] = []byte(` [workspace] @@ -4556,11 +4566,12 @@ name = "clean-city" name = "mayor" `) - if _, _, err := LoadWithIncludes(fs, "/city/city.toml"); err != nil { + _, prov, err := LoadWithIncludes(fs, "/city/city.toml") + if err != nil { t.Fatalf("LoadWithIncludes: %v", err) } - if got := buf.String(); got != "" { - t.Errorf("expected no warning, got:\n%s", got) + if len(prov.Warnings) != 0 { + t.Errorf("expected no warning, got:\n%s", strings.Join(prov.Warnings, "\n")) } } diff --git a/internal/config/field_sync_test.go b/internal/config/field_sync_test.go index 87b96b431..2dfc0da31 100644 --- a/internal/config/field_sync_test.go +++ b/internal/config/field_sync_test.go @@ -23,34 +23,36 @@ func TestAgentFieldSync(t *testing.T) { // Provider-level fields: set during ResolveProvider, not typically // overridden per-rig. Agent-level overrides happen in the Agent // struct itself (which feeds into ResolveProvider). - "Args": "provider field, set via ResolveProvider", - "PromptMode": "provider field, set via ResolveProvider", - "PromptFlag": "provider field, set via ResolveProvider", - "ReadyDelayMs": "provider field, set via ResolveProvider", - "ReadyPromptPrefix": "provider field, set via ResolveProvider", - "ProcessNames": "provider field, set via ResolveProvider", - "EmitsPermissionWarning": "provider field, set via ResolveProvider", - "WorkQuery": "agent-specific, derived from name — not a patch concern", - "SlingQuery": "agent-specific, derived from name/pool — not a patch concern", - "MaxActiveSessions": "cap field, inherits from rig/workspace — not a patch concern", - "MinActiveSessions": "cap field, inherits from rig/workspace — not a patch concern", - "ScaleCheck": "agent-specific scaling, derived from pool config — not a patch concern", - "SourceDir": "runtime-only, set during pack/fragment loading", - "SharedSkills": "runtime-only, inherited baseline seeded from agent_defaults.skills", - "SharedMCP": "runtime-only, inherited baseline seeded from agent_defaults.mcp", - "SkillsDir": "runtime-only, set during agent discovery from agents//skills/", - "MCPDir": "runtime-only, set during agent discovery from agents//mcp/", - "Fallback": "pack composition hint, not overridable at runtime", - "PoolName": "internal field set during pool expansion, not user-configurable", - "Implicit": "runtime-only, set during InjectImplicitAgents, not user-configurable", - "SleepAfterIdleSource": "runtime-only provenance, derived from the layer that set SleepAfterIdle", - "DrainTimeout": "scaling field, patched via PoolOverride.DrainTimeout", - "OnBoot": "scaling field, patched via PoolOverride.OnBoot", - "OnDeath": "scaling field, patched via PoolOverride.OnDeath", - "Namepool": "agent-specific file path, not a patch concern", - "NamepoolNames": "runtime-only, loaded from Namepool file at config load time", - "BindingName": "runtime-only, set during V2 import expansion, not user-configurable", - "PackName": "runtime-only, set during V2 import expansion, not user-configurable", + "Args": "provider field, set via ResolveProvider", + "PromptMode": "provider field, set via ResolveProvider", + "PromptFlag": "provider field, set via ResolveProvider", + "ReadyDelayMs": "provider field, set via ResolveProvider", + "ReadyPromptPrefix": "provider field, set via ResolveProvider", + "ProcessNames": "provider field, set via ResolveProvider", + "EmitsPermissionWarning": "provider field, set via ResolveProvider", + "WorkQuery": "agent-specific, derived from name — not a patch concern", + "SlingQuery": "agent-specific, derived from name/pool — not a patch concern", + "MaxActiveSessions": "cap field, inherits from rig/workspace — not a patch concern", + "MinActiveSessions": "cap field, inherits from rig/workspace — not a patch concern", + "ScaleCheck": "agent-specific scaling, derived from pool config — not a patch concern", + "SourceDir": "runtime-only, set during pack/fragment loading", + "InheritedDefaultSlingFormula": "runtime-only, derived from imported pack [agent_defaults]", + "InheritedAppendFragments": "runtime-only, derived from imported pack [agent_defaults]", + "SharedSkills": "runtime-only legacy tombstone field retained for backwards compatibility", + "SharedMCP": "runtime-only legacy tombstone field retained for backwards compatibility", + "SkillsDir": "runtime-only, set during agent discovery from agents//skills/", + "MCPDir": "runtime-only, set during agent discovery from agents//mcp/", + "Fallback": "pack composition hint, not overridable at runtime", + "PoolName": "internal field set during pool expansion, not user-configurable", + "Implicit": "runtime-only, set during InjectImplicitAgents, not user-configurable", + "SleepAfterIdleSource": "runtime-only provenance, derived from the layer that set SleepAfterIdle", + "DrainTimeout": "scaling field, patched via PoolOverride.DrainTimeout", + "OnBoot": "scaling field, patched via PoolOverride.OnBoot", + "OnDeath": "scaling field, patched via PoolOverride.OnDeath", + "Namepool": "agent-specific file path, not a patch concern", + "NamepoolNames": "runtime-only, loaded from Namepool file at config load time", + "BindingName": "runtime-only, set during V2 import expansion, not user-configurable", + "PackName": "runtime-only, set during V2 import expansion, not user-configurable", } // Fields on AgentOverride/AgentPatch that don't map 1:1 to Agent fields. diff --git a/internal/config/import_negative_test.go b/internal/config/import_negative_test.go index 7ddd4ca02..dd2589de4 100644 --- a/internal/config/import_negative_test.go +++ b/internal/config/import_negative_test.go @@ -5,6 +5,7 @@ package config import ( "path/filepath" + "slices" "strings" "testing" @@ -42,6 +43,529 @@ includes = ["../mypk"] } } +func TestImport_TransitiveFalseSuppressesNestedPackWarnings(t *testing.T) { + dir := t.TempDir() + cityDir := filepath.Join(dir, "city") + for _, name := range []string{"city", "b", "c"} { + mustMkdirAll(t, filepath.Join(dir, name), 0o755) + } + + writeTestFile(t, cityDir, "city.toml", ` +[workspace] +name = "test" + +[imports.b] +source = "../b" +transitive = false +`) + writeTestFile(t, filepath.Join(dir, "b"), "pack.toml", ` +[pack] +name = "b" +schema = 1 + +[agents] +append_fragments = ["direct"] + +[imports.c] +source = "../c" + +[[agent]] +name = "direct" +scope = "city" +`) + writeTestFile(t, filepath.Join(dir, "c"), "pack.toml", ` +[pack] +name = "c" +schema = 1 + +[agent_defaults] +provider = "claude" + +[[agent]] +name = "transitive" +scope = "city" +`) + + _, prov, err := LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityDir, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + warnings := strings.Join(prov.Warnings, "\n") + if !strings.Contains(warnings, filepath.Join(dir, "b", "pack.toml")) { + t.Fatalf("expected direct import warning from b pack.toml; got %q", warnings) + } + if strings.Contains(warnings, filepath.Join(dir, "c", "pack.toml")) { + t.Fatalf("nested pack warnings should be suppressed by transitive=false; got %q", warnings) + } +} + +func TestImport_TransitiveFalseSuppressesNestedNamedSessions(t *testing.T) { + dir := t.TempDir() + cityDir := filepath.Join(dir, "city") + for _, name := range []string{"city", "b", "c"} { + mustMkdirAll(t, filepath.Join(dir, name), 0o755) + } + + writeTestFile(t, cityDir, "city.toml", ` +[workspace] +name = "test" + +[imports.b] +source = "../b" +transitive = false +`) + writeTestFile(t, filepath.Join(dir, "b"), "pack.toml", ` +[pack] +name = "b" +schema = 1 + +[imports.c] +source = "../c" + +[[agent]] +name = "direct" +scope = "city" + +[[named_session]] +template = "direct" +scope = "city" +mode = "always" +`) + writeTestFile(t, filepath.Join(dir, "c"), "pack.toml", ` +[pack] +name = "c" +schema = 1 + +[[agent]] +name = "transitive" +scope = "city" + +[[named_session]] +template = "transitive" +scope = "city" +mode = "always" +`) + + cfg, _, err := LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityDir, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + found := map[string]bool{} + for _, named := range cfg.NamedSessions { + found[named.QualifiedName()] = true + } + if !found["b.direct"] { + t.Fatalf("expected direct named session from b; got %v", found) + } + for qn := range found { + if strings.Contains(qn, "transitive") { + t.Fatalf("transitive=false should block nested named sessions; got %v", found) + } + } +} + +func TestImport_TransitiveFalseSuppressesLegacyIncludedPackDeps(t *testing.T) { + dir := t.TempDir() + cityDir := filepath.Join(dir, "city") + for _, name := range []string{"city", "b", "c"} { + mustMkdirAll(t, filepath.Join(dir, name), 0o755) + } + + writeTestFile(t, cityDir, "city.toml", ` +[workspace] +name = "test" + +[imports.b] +source = "../b" +transitive = false +`) + writeTestFile(t, filepath.Join(dir, "b"), "pack.toml", ` +[pack] +name = "b" +schema = 1 +includes = ["../c"] + +[[agent]] +name = "direct" +scope = "city" +`) + writeTestFile(t, filepath.Join(dir, "c"), "pack.toml", ` +[pack] +name = "c" +schema = 1 + +[[agent]] +name = "legacy-transitive" +scope = "city" +`) + + cfg, _, err := LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityDir, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + found := map[string]bool{} + for _, agent := range cfg.Agents { + found[agent.QualifiedName()] = true + } + if !found["b.direct"] { + t.Fatalf("expected direct agent from b; got %v", found) + } + for qn := range found { + if strings.Contains(qn, "legacy-transitive") { + t.Fatalf("transitive=false should block nested legacy includes; got %v", found) + } + } +} + +func TestImport_TransitiveFalseSuppressesNestedCityImportArtifacts(t *testing.T) { + dir := t.TempDir() + cityDir := filepath.Join(dir, "city") + for _, name := range []string{"city", "b", "c"} { + mustMkdirAll(t, filepath.Join(dir, name), 0o755) + } + + writeTestFile(t, cityDir, "city.toml", ` +[workspace] +name = "test" + +[imports.b] +source = "../b" +transitive = false +`) + writeTestFile(t, filepath.Join(dir, "b"), "pack.toml", ` +[pack] +name = "b" +schema = 1 + +[imports.c] +source = "../c" + +[providers.direct-helper] +command = "direct-provider" + +[global] +session_live = ["echo direct-global"] + +[[agent]] +name = "direct" +scope = "city" +`) + writeTestFile(t, filepath.Join(dir, "b"), "formulas/direct.md", "direct") + writeTestFile(t, filepath.Join(dir, "c"), "pack.toml", ` +[pack] +name = "c" +schema = 1 + +[providers.nested-helper] +command = "nested-provider" + +[global] +session_live = ["echo nested-global"] + +[[pack.requires]] +scope = "city" +agent = "missing-nested" + +[[agent]] +name = "nested" +scope = "city" +`) + writeTestFile(t, filepath.Join(dir, "c"), "formulas/nested.md", "nested") + + cfg, _, err := LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityDir, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + if _, ok := cfg.Providers["direct-helper"]; !ok { + t.Fatalf("expected direct provider from b; got %v", cfg.Providers) + } + if _, ok := cfg.Providers["nested-helper"]; ok { + t.Fatalf("transitive=false should block nested provider definitions; got %v", cfg.Providers) + } + + var directAgent *Agent + for i := range cfg.Agents { + if cfg.Agents[i].QualifiedName() == "b.direct" { + directAgent = &cfg.Agents[i] + break + } + } + if directAgent == nil { + t.Fatalf("expected direct imported agent b.direct; got %v", explicitAgents(cfg.Agents)) + } + if !slices.Contains(directAgent.SessionLive, "echo direct-global") { + t.Fatalf("expected direct global on b.direct; got %v", directAgent.SessionLive) + } + if slices.Contains(directAgent.SessionLive, "echo nested-global") { + t.Fatalf("transitive=false should block nested globals; got %v", directAgent.SessionLive) + } + + directFormulaDir := filepath.Join(dir, "b", "formulas") + nestedFormulaDir := filepath.Join(dir, "c", "formulas") + if !slices.Contains(cfg.FormulaLayers.City, directFormulaDir) { + t.Fatalf("expected direct formula layer %q; got %v", directFormulaDir, cfg.FormulaLayers.City) + } + if slices.Contains(cfg.FormulaLayers.City, nestedFormulaDir) { + t.Fatalf("transitive=false should block nested formula layers; got %v", cfg.FormulaLayers.City) + } +} + +func TestImport_TransitiveFalseSuppressesNestedPackImportArtifacts(t *testing.T) { + dir := t.TempDir() + cityDir := filepath.Join(dir, "city") + for _, name := range []string{"city", "outer", "b", "c"} { + mustMkdirAll(t, filepath.Join(dir, name), 0o755) + } + + writeTestFile(t, cityDir, "city.toml", ` +[workspace] +name = "test" +includes = ["../outer"] +`) + writeTestFile(t, filepath.Join(dir, "outer"), "pack.toml", ` +[pack] +name = "outer" +schema = 1 + +[imports.tools] +source = "../b" +transitive = false +`) + writeTestFile(t, filepath.Join(dir, "b"), "pack.toml", ` +[pack] +name = "b" +schema = 1 + +[imports.c] +source = "../c" + +[providers.direct-helper] +command = "direct-provider" + +[global] +session_live = ["echo direct-global"] + +[[agent]] +name = "direct" +scope = "city" +`) + writeTestFile(t, filepath.Join(dir, "b"), "formulas/direct.md", "direct") + writeTestFile(t, filepath.Join(dir, "c"), "pack.toml", ` +[pack] +name = "c" +schema = 1 + +[providers.nested-helper] +command = "nested-provider" + +[global] +session_live = ["echo nested-global"] + +[[pack.requires]] +scope = "city" +agent = "missing-nested" + +[[agent]] +name = "nested" +scope = "city" +`) + writeTestFile(t, filepath.Join(dir, "c"), "formulas/nested.md", "nested") + + cfg, _, err := LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityDir, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + if _, ok := cfg.Providers["direct-helper"]; !ok { + t.Fatalf("expected direct provider from b; got %v", cfg.Providers) + } + if _, ok := cfg.Providers["nested-helper"]; ok { + t.Fatalf("transitive=false should block nested provider definitions from pack imports; got %v", cfg.Providers) + } + + var directAgent *Agent + for i := range cfg.Agents { + if cfg.Agents[i].QualifiedName() == "tools.direct" { + directAgent = &cfg.Agents[i] + break + } + } + if directAgent == nil { + t.Fatalf("expected direct imported agent tools.direct; got %v", explicitAgents(cfg.Agents)) + } + if !slices.Contains(directAgent.SessionLive, "echo direct-global") { + t.Fatalf("expected direct global on tools.direct; got %v", directAgent.SessionLive) + } + if slices.Contains(directAgent.SessionLive, "echo nested-global") { + t.Fatalf("transitive=false should block nested globals from pack imports; got %v", directAgent.SessionLive) + } + + directFormulaDir := filepath.Join(dir, "b", "formulas") + nestedFormulaDir := filepath.Join(dir, "c", "formulas") + if !slices.Contains(cfg.FormulaLayers.City, directFormulaDir) { + t.Fatalf("expected direct formula layer %q; got %v", directFormulaDir, cfg.FormulaLayers.City) + } + if slices.Contains(cfg.FormulaLayers.City, nestedFormulaDir) { + t.Fatalf("transitive=false should block nested formula layers from pack imports; got %v", cfg.FormulaLayers.City) + } +} + +func TestImport_RigImportTransitiveFalseSuppressesNestedDeps(t *testing.T) { + dir := t.TempDir() + cityDir := filepath.Join(dir, "city") + for _, name := range []string{"city", "b", "c"} { + mustMkdirAll(t, filepath.Join(dir, name), 0o755) + } + + writeTestFile(t, cityDir, "city.toml", ` +[workspace] +name = "test" + +[[rigs]] +name = "proj" +path = "/tmp/proj" + +[rigs.imports.tools] +source = "../b" +transitive = false +`) + writeTestFile(t, filepath.Join(dir, "b"), "pack.toml", ` +[pack] +name = "b" +schema = 1 + +[imports.c] +source = "../c" + +[providers.direct-helper] +command = "direct-provider" + +[global] +session_live = ["echo direct-global"] + +[[agent]] +name = "direct" +scope = "rig" + +[[named_session]] +template = "direct" +scope = "rig" +mode = "always" +`) + writeTestFile(t, filepath.Join(dir, "b"), "doctor/direct-check/run.sh", "#!/bin/sh\nexit 0\n") + writeTestFile(t, filepath.Join(dir, "b"), "formulas/direct.md", "direct") + writeTestFile(t, filepath.Join(dir, "c"), "pack.toml", ` +[pack] +name = "c" +schema = 1 + +[providers.nested-helper] +command = "nested-provider" + +[global] +session_live = ["echo nested-global"] + +[[pack.requires]] +scope = "rig" +agent = "missing-nested" + +[[agent]] +name = "nested" +scope = "rig" + +[[named_session]] +template = "nested" +scope = "rig" +mode = "always" + +[[service]] +name = "nested-webhook" +kind = "workflow" +`) + writeTestFile(t, filepath.Join(dir, "c"), "doctor/nested-check/run.sh", "#!/bin/sh\nexit 0\n") + writeTestFile(t, filepath.Join(dir, "c"), "formulas/nested.md", "nested") + + cfg, _, err := LoadWithIncludes(fsys.OSFS{}, filepath.Join(cityDir, "city.toml")) + if err != nil { + t.Fatalf("LoadWithIncludes: %v", err) + } + + foundAgents := map[string]bool{} + for _, agent := range cfg.Agents { + foundAgents[agent.QualifiedName()] = true + } + if !foundAgents["proj/tools.direct"] { + t.Fatalf("expected direct rig agent from b; got %v", foundAgents) + } + for qn := range foundAgents { + if strings.Contains(qn, "nested") { + t.Fatalf("transitive=false should block nested rig agents; got %v", foundAgents) + } + } + + foundSessions := map[string]bool{} + for _, named := range cfg.NamedSessions { + foundSessions[named.QualifiedName()] = true + } + if !foundSessions["proj/tools.direct"] { + t.Fatalf("expected direct rig named session from b; got %v", foundSessions) + } + for qn := range foundSessions { + if strings.Contains(qn, "nested") { + t.Fatalf("transitive=false should block nested rig named sessions; got %v", foundSessions) + } + } + + foundDoctors := map[string]bool{} + for _, doctor := range cfg.PackDoctors { + foundDoctors[doctor.Name] = true + } + if !foundDoctors["direct-check"] { + t.Fatalf("expected direct rig import doctor from b; got %v", foundDoctors) + } + if foundDoctors["nested-check"] { + t.Fatalf("transitive=false should block nested rig import doctors; got %v", foundDoctors) + } + + if _, ok := cfg.Providers["direct-helper"]; !ok { + t.Fatalf("expected direct provider from rig import; got %v", cfg.Providers) + } + if _, ok := cfg.Providers["nested-helper"]; ok { + t.Fatalf("transitive=false should block nested rig import providers; got %v", cfg.Providers) + } + + var directAgent *Agent + for i := range cfg.Agents { + if cfg.Agents[i].QualifiedName() == "proj/tools.direct" { + directAgent = &cfg.Agents[i] + break + } + } + if directAgent == nil { + t.Fatalf("expected direct rig agent proj/tools.direct; got %v", cfg.Agents) + } + if !slices.Contains(directAgent.SessionLive, "echo direct-global") { + t.Fatalf("expected direct rig global on proj/tools.direct; got %v", directAgent.SessionLive) + } + if slices.Contains(directAgent.SessionLive, "echo nested-global") { + t.Fatalf("transitive=false should block nested rig globals; got %v", directAgent.SessionLive) + } + + directFormulaDir := filepath.Join(dir, "b", "formulas") + nestedFormulaDir := filepath.Join(dir, "c", "formulas") + rigFormulas := cfg.FormulaLayers.SearchPaths("proj") + if !slices.Contains(rigFormulas, directFormulaDir) { + t.Fatalf("expected direct rig formula layer %q; got %v", directFormulaDir, rigFormulas) + } + if slices.Contains(rigFormulas, nestedFormulaDir) { + t.Fatalf("transitive=false should block nested rig formula layers; got %v", rigFormulas) + } +} + func TestImport_InvalidPackSchemaInCityPackToml(t *testing.T) { // A city pack.toml with invalid schema should produce a clear error. dir := t.TempDir() diff --git a/internal/config/pack.go b/internal/config/pack.go index cd6f4c3ef..3706d0ccd 100644 --- a/internal/config/pack.go +++ b/internal/config/pack.go @@ -25,19 +25,20 @@ const currentPackSchema = 2 // packConfig is the TOML structure of a pack.toml file. // It has a [pack] metadata header and agent definitions. type packConfig struct { - Pack PackMeta `toml:"pack"` - Imports map[string]Import `toml:"imports,omitempty"` - AgentDefaults AgentDefaults `toml:"agent_defaults,omitempty"` - Defaults packDefaults `toml:"defaults,omitempty"` - Agents []Agent `toml:"agent"` - NamedSessions []NamedSession `toml:"named_session,omitempty"` - Services []Service `toml:"service,omitempty"` - Providers map[string]ProviderSpec `toml:"providers,omitempty"` - Formulas FormulasConfig `toml:"formulas,omitempty"` - Patches Patches `toml:"patches,omitempty"` - Doctor []PackDoctorEntry `toml:"doctor,omitempty"` - Commands []PackCommandEntry `toml:"commands,omitempty"` - Global PackGlobal `toml:"global,omitempty"` + Pack PackMeta `toml:"pack"` + Imports map[string]Import `toml:"imports,omitempty"` + AgentDefaults AgentDefaults `toml:"agent_defaults,omitempty"` + AgentsDefaults AgentDefaults `toml:"agents,omitempty" jsonschema:"-"` + Defaults packDefaults `toml:"defaults,omitempty"` + Agents []Agent `toml:"agent"` + NamedSessions []NamedSession `toml:"named_session,omitempty"` + Services []Service `toml:"service,omitempty"` + Providers map[string]ProviderSpec `toml:"providers,omitempty"` + Formulas FormulasConfig `toml:"formulas,omitempty"` + Patches Patches `toml:"patches,omitempty"` + Doctor []PackDoctorEntry `toml:"doctor,omitempty"` + Commands []PackCommandEntry `toml:"commands,omitempty"` + Global PackGlobal `toml:"global,omitempty"` } type packDefaults struct { @@ -100,6 +101,7 @@ func expandPacks(cfg *City, fs fsys.FS, cityRoot string, rigFormulaDirs map[stri if err != nil { return fmt.Errorf("rig %q pack %q: %w", rig.Name, ref, err) } + cfg.LoadWarnings = appendUnique(cfg.LoadWarnings, cachedPackWarnings(cache, topoDir)...) if len(services) > 0 { return fmt.Errorf("rig %q pack %q: [[service]] is only allowed in city-scoped packs", rig.Name, ref) } @@ -195,13 +197,12 @@ func expandPacks(cfg *City, fs fsys.FS, cityRoot string, rigFormulaDirs map[stri if err != nil { return fmt.Errorf("rig %q import %q: %w", rig.Name, bindingName, err) } - if len(services) > 0 { - return fmt.Errorf("rig %q import %q: [[service]] is only allowed in city-scoped packs", rig.Name, bindingName) - } + warnings := cachedPackWarnings(cache, impDir) commands := cachedPackCommands(cache, impDir) doctors := cachedPackDoctors(cache, impDir) skills := cachedPackSkills(cache, impDir) if !imp.ImportIsTransitive() { + warnings = cachedPackLocalWarnings(cache, impDir) absImpDir, _ := filepath.Abs(impDir) var direct []Agent for _, a := range agents { @@ -211,10 +212,20 @@ func expandPacks(cfg *City, fs fsys.FS, cityRoot string, rigFormulaDirs map[stri } } agents = direct + namedSessions = filterNamedSessionsBySourceDir(namedSessions, impDir) + services = filterServicesBySourceDir(services, impDir) commands = filterCommandsByPackDir(commands, impDir) doctors = filterDoctorsByPackDir(doctors, impDir) + providers = cachedPackLocalProviders(cache, impDir) + topoDirs = cachedPackLocalTopoDirs(cache, impDir) + reqs = cachedPackLocalRequires(cache, impDir) + globals = cachedPackLocalGlobals(cache, impDir) skills = filterSkillsByPackDir(skills, impDir) } + cfg.LoadWarnings = appendUnique(cfg.LoadWarnings, warnings...) + if len(services) > 0 { + return fmt.Errorf("rig %q import %q: [[service]] is only allowed in city-scoped packs", rig.Name, bindingName) + } rigGlobals = append(rigGlobals, globals...) if cfg.RigPackSkills == nil { cfg.RigPackSkills = make(map[string][]DiscoveredSkillCatalog) @@ -459,6 +470,7 @@ func expandCityPacks(cfg *City, fs fsys.FS, cityRoot string, opts LoadOptions) ( var bootstrapImportPackDirs []string var allRequires []PackRequirement var allGlobals []ResolvedPackGlobal + var packWarnings []string // Shared cache across all pack loads to deduplicate diamond DAGs. cache := &packLoadCache{results: make(map[string]*packLoadResult)} @@ -495,6 +507,7 @@ func expandCityPacks(cfg *City, fs fsys.FS, cityRoot string, opts LoadOptions) ( } return nil, nil, nil, fmt.Errorf("city pack %q: %w", ref, err) } + packWarnings = appendUnique(packWarnings, cachedPackWarnings(cache, topoDir)...) allRequires = append(allRequires, reqs...) allGlobals = append(allGlobals, globals...) cfg.Services = append(cfg.Services, services...) @@ -569,13 +582,19 @@ func expandCityPacks(cfg *City, fs fsys.FS, cityRoot string, opts LoadOptions) ( if err != nil { return nil, nil, nil, fmt.Errorf("city import %q: %w", bindingName, err) } + warnings := cachedPackWarnings(cache, impDir) + if !imp.ImportIsTransitive() { + warnings = cachedPackLocalWarnings(cache, impDir) + } + packWarnings = appendUnique(packWarnings, warnings...) commands := cachedPackCommands(cache, impDir) doctors := cachedPackDoctors(cache, impDir) skills := cachedPackSkills(cache, impDir) mcpTopoDirs := topoDirs - // When transitive = false, keep only agents directly defined - // by this import (not its own transitive dependencies). + // by this import. Nested pack dependencies reached through + // either [imports] or legacy [pack].includes stay hidden from + // the consumer. if !imp.ImportIsTransitive() { absImpDir, _ := filepath.Abs(impDir) var direct []Agent @@ -586,8 +605,14 @@ func expandCityPacks(cfg *City, fs fsys.FS, cityRoot string, opts LoadOptions) ( } } agents = direct + namedSessions = filterNamedSessionsBySourceDir(namedSessions, impDir) + services = filterServicesBySourceDir(services, impDir) commands = filterCommandsByPackDir(commands, impDir) doctors = filterDoctorsByPackDir(doctors, impDir) + providers = cachedPackLocalProviders(cache, impDir) + topoDirs = cachedPackLocalTopoDirs(cache, impDir) + reqs = cachedPackLocalRequires(cache, impDir) + globals = cachedPackLocalGlobals(cache, impDir) skills = filterSkillsByPackDir(skills, impDir) mcpTopoDirs = filterPackDirsByRoot(topoDirs, impDir) } @@ -782,6 +807,7 @@ func expandCityPacks(cfg *City, fs fsys.FS, cityRoot string, opts LoadOptions) ( // Store city-level pack globals. cfg.PackGlobals = append(cfg.PackGlobals, allGlobals...) + shadowWarnings = appendUnique(shadowWarnings, packWarnings...) return formulaDirs, allRequires, shadowWarnings, nil } @@ -977,16 +1003,52 @@ type packLoadCache struct { } type packLoadResult struct { - agents []Agent - namedSessions []NamedSession - providers map[string]ProviderSpec - services []Service - topoDirs []string - requires []PackRequirement - globals []ResolvedPackGlobal - commands []DiscoveredCommand - doctors []DiscoveredDoctor - skills []DiscoveredSkillCatalog + agents []Agent + namedSessions []NamedSession + providers map[string]ProviderSpec + localProviders map[string]ProviderSpec + services []Service + topoDirs []string + localTopoDirs []string + requires []PackRequirement + localRequires []PackRequirement + globals []ResolvedPackGlobal + localGlobals []ResolvedPackGlobal + commands []DiscoveredCommand + doctors []DiscoveredDoctor + skills []DiscoveredSkillCatalog + localWarnings []string + warnings []string +} + +func parsePackConfigWithMeta(data []byte, source string) (packConfig, []string, error) { + cfg, _, warnings, err := parsePackConfigWithMetadata(data, source) + return cfg, warnings, err +} + +func parsePackConfigWithMetadata(data []byte, source string) (packConfig, toml.MetaData, []string, error) { + var cfg packConfig + md, err := toml.Decode(string(data), &cfg) + if err != nil { + return packConfig{}, md, nil, err + } + normalizePackAgentDefaultsAlias(&cfg, md) + warnings := agentDefaultsCompatibilityWarnings(md, source) + warnings = append(warnings, CheckUndecodedKeys(md, source)...) + return cfg, md, warnings, nil +} + +func normalizePackAgentDefaultsAlias(cfg *packConfig, meta toml.MetaData) { + if !meta.IsDefined("agents") { + cfg.AgentsDefaults = AgentDefaults{} + return + } + if meta.IsDefined("agent_defaults") { + mergeAgentDefaultsAliasPreferCanonical(&cfg.AgentDefaults, cfg.AgentsDefaults, meta) + } else { + cfg.AgentDefaults = cfg.AgentsDefaults + } + cfg.AgentsDefaults = AgentDefaults{} } //nolint:unparam // compatibility wrapper keeps the recursion-set argument at the public helper boundary. @@ -1038,13 +1100,12 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("loading %s: %w", packFile, err) } - var tc packConfig - md, err := toml.Decode(string(data), &tc) + tc, md, packWarnings, err := parsePackConfigWithMetadata(data, topoPath) if err != nil { return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("parsing %s: %w", packFile, err) } - if warnings := CheckUndecodedKeys(md, topoPath); len(warnings) > 0 { - return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("parsing %s: %s", packFile, strings.Join(warnings, "; ")) + if fatalWarnings := fatalUndecodedWarnings(md, topoPath); len(fatalWarnings) > 0 { + return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("parsing %s: %s", packFile, strings.Join(fatalWarnings, "; ")) } if len(tc.Defaults.Rig.Imports) > 0 { return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("parsing %s: [defaults.rig.imports] is only supported in a city root pack.toml", packFile) @@ -1065,6 +1126,7 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s var includedCommands []DiscoveredCommand var includedDoctors []DiscoveredDoctor var includedSkills []DiscoveredSkillCatalog + var inheritedWarnings []string includedProviders := make(map[string]ProviderSpec) for _, inc := range tc.Pack.Includes { @@ -1079,6 +1141,7 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s if err != nil { return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("include %q: %w", inc, err) } + inheritedWarnings = appendUnique(inheritedWarnings, cachedPackWarnings(cache, incTopoDir)...) includedAgents = append(includedAgents, incAgents...) includedNamedSessions = append(includedNamedSessions, incNamedSessions...) @@ -1097,6 +1160,7 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s } } } + applyInheritedPackAgentDefaults(includedAgents, tc.AgentDefaults) // Process V2 [imports.X] entries. These are named bindings that // produce agents with qualified names (bindingName.agentName). @@ -1126,13 +1190,19 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s if err != nil { return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("import %q: %w", bindingName, err) } + warnings := cachedPackWarnings(cache, impDir) + if !imp.ImportIsTransitive() { + warnings = cachedPackLocalWarnings(cache, impDir) + } + inheritedWarnings = appendUnique(inheritedWarnings, warnings...) impCommands := cachedPackCommands(cache, impDir) impDoctors := cachedPackDoctors(cache, impDir) impSkills := cachedPackSkills(cache, impDir) // When transitive = false, strip agents that came from the - // imported pack's own imports (i.e., transitive deps). We keep - // only agents whose SourceDir matches the import's own directory. + // imported pack's nested dependencies. We keep only agents + // whose SourceDir matches the import's own directory, which + // suppresses both nested [imports] and legacy [pack].includes. if !imp.ImportIsTransitive() { absImpDir, _ := filepath.Abs(impDir) var direct []Agent @@ -1143,8 +1213,14 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s } } impAgents = direct + impNamedSessions = filterNamedSessionsBySourceDir(impNamedSessions, impDir) + impServices = filterServicesBySourceDir(impServices, impDir) impCommands = filterCommandsByPackDir(impCommands, impDir) impDoctors = filterDoctorsByPackDir(impDoctors, impDir) + impProviders = cachedPackLocalProviders(cache, impDir) + impTopoDirs = cachedPackLocalTopoDirs(cache, impDir) + impReqs = cachedPackLocalRequires(cache, impDir) + impGlobals = cachedPackLocalGlobals(cache, impDir) impSkills = filterSkillsByPackDir(impSkills, impDir) } @@ -1237,6 +1313,7 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s return nil, nil, nil, nil, nil, nil, nil, dErr } tc.Agents = append(tc.Agents, discovered...) + applyInheritedPackAgentDefaults(tc.Agents, tc.AgentDefaults) commands, err := DiscoverPackCommands(fs, topoDir, tc.Pack.Name) if err != nil { @@ -1366,27 +1443,35 @@ func loadPackWithCacheOptions(fs fsys.FS, topoPath, topoDir, cityRoot, rigName s topoDirs = append(topoDirs, topoDir) // Collect globals: included globals first, then this pack's own. - var allGlobals []ResolvedPackGlobal - allGlobals = append(allGlobals, includedGlobals...) + var localGlobals []ResolvedPackGlobal if len(tc.Global.SessionLive) > 0 { - allGlobals = append(allGlobals, ResolvedPackGlobal{ + localGlobals = append(localGlobals, ResolvedPackGlobal{ SessionLive: resolveConfigDirInCommands(tc.Global.SessionLive, topoDir), PackName: tc.Pack.Name, }) } + var allGlobals []ResolvedPackGlobal + allGlobals = append(allGlobals, includedGlobals...) + allGlobals = append(allGlobals, localGlobals...) // Cache result for diamond-DAG dedup. cache.results[absTopoDir] = clonePackLoadResult(&packLoadResult{ - agents: includedAgents, - namedSessions: includedNamedSessions, - providers: mergedProviders, - services: includedServices, - topoDirs: topoDirs, - requires: allRequires, - globals: allGlobals, - commands: includedCommands, - doctors: includedDoctors, - skills: includedSkills, + agents: includedAgents, + namedSessions: includedNamedSessions, + providers: mergedProviders, + localProviders: tc.Providers, + services: includedServices, + topoDirs: topoDirs, + localTopoDirs: []string{topoDir}, + requires: allRequires, + localRequires: append([]PackRequirement(nil), tc.Pack.Requires...), + globals: allGlobals, + localGlobals: localGlobals, + commands: includedCommands, + doctors: includedDoctors, + skills: includedSkills, + localWarnings: append([]string(nil), packWarnings...), + warnings: appendUnique(append([]string(nil), inheritedWarnings...), packWarnings...), }) return includedAgents, includedNamedSessions, mergedProviders, includedServices, topoDirs, allRequires, allGlobals, nil @@ -1397,16 +1482,22 @@ func clonePackLoadResult(in *packLoadResult) *packLoadResult { return nil } return &packLoadResult{ - agents: deepCopyAgents(in.agents), - namedSessions: deepCopyNamedSessions(in.namedSessions), - providers: deepCopyProviderSpecs(in.providers), - services: deepCopyServices(in.services), - topoDirs: append([]string(nil), in.topoDirs...), - requires: append([]PackRequirement(nil), in.requires...), - globals: deepCopyResolvedPackGlobals(in.globals), - commands: deepCopyCommands(in.commands), - doctors: deepCopyDoctors(in.doctors), - skills: deepCopySkills(in.skills), + agents: deepCopyAgents(in.agents), + namedSessions: deepCopyNamedSessions(in.namedSessions), + providers: deepCopyProviderSpecs(in.providers), + localProviders: deepCopyProviderSpecs(in.localProviders), + services: deepCopyServices(in.services), + topoDirs: append([]string(nil), in.topoDirs...), + localTopoDirs: append([]string(nil), in.localTopoDirs...), + requires: append([]PackRequirement(nil), in.requires...), + localRequires: append([]PackRequirement(nil), in.localRequires...), + globals: deepCopyResolvedPackGlobals(in.globals), + localGlobals: deepCopyResolvedPackGlobals(in.localGlobals), + commands: deepCopyCommands(in.commands), + doctors: deepCopyDoctors(in.doctors), + skills: deepCopySkills(in.skills), + localWarnings: append([]string(nil), in.localWarnings...), + warnings: append([]string(nil), in.warnings...), } } @@ -1432,6 +1523,8 @@ func deepCopyAgents(in []Agent) []Agent { out[i].HooksInstalled = copyBoolPtr(in[i].HooksInstalled) out[i].InjectAssignedSkills = copyBoolPtr(in[i].InjectAssignedSkills) out[i].DefaultSlingFormula = copyStringPtr(in[i].DefaultSlingFormula) + out[i].InheritedDefaultSlingFormula = copyStringPtr(in[i].InheritedDefaultSlingFormula) + out[i].InheritedAppendFragments = append([]string(nil), in[i].InheritedAppendFragments...) out[i].Attach = copyBoolPtr(in[i].Attach) } return out @@ -1537,6 +1630,22 @@ func copyStringPtr(in *string) *string { return &out } +func applyInheritedPackAgentDefaults(agents []Agent, defaults AgentDefaults) { + for i := range agents { + if agents[i].BindingName != "" { + continue + } + // Includes compose from the inside out: once an included agent has + // inherited a scalar default, outer packs do not replace it. + if defaults.DefaultSlingFormula != "" && agents[i].DefaultSlingFormula == nil && agents[i].InheritedDefaultSlingFormula == nil { + agents[i].InheritedDefaultSlingFormula = copyStringPtr(&defaults.DefaultSlingFormula) + } + if len(defaults.AppendFragments) > 0 { + agents[i].InheritedAppendFragments = appendUnique(agents[i].InheritedAppendFragments, defaults.AppendFragments...) + } + } +} + func cachedPackCommands(cache *packLoadCache, topoDir string) []DiscoveredCommand { if cache == nil { return nil @@ -1553,6 +1662,111 @@ func cachedPackCommands(cache *packLoadCache, topoDir string) []DiscoveredComman return out } +func cachedPackWarnings(cache *packLoadCache, topoDir string) []string { + if cache == nil { + return nil + } + absDir, err := filepath.Abs(topoDir) + if err != nil { + absDir = topoDir + } + result, ok := cache.results[absDir] + if !ok { + return nil + } + return append([]string(nil), result.warnings...) +} + +func cachedPackLocalWarnings(cache *packLoadCache, topoDir string) []string { + if cache == nil { + return nil + } + absDir, err := filepath.Abs(topoDir) + if err != nil { + absDir = topoDir + } + result, ok := cache.results[absDir] + if !ok { + return nil + } + return append([]string(nil), result.localWarnings...) +} + +func cachedPackLocalProviders(cache *packLoadCache, topoDir string) map[string]ProviderSpec { + if cache == nil { + return nil + } + absDir, err := filepath.Abs(topoDir) + if err != nil { + absDir = topoDir + } + result, ok := cache.results[absDir] + if !ok { + return nil + } + return deepCopyProviderSpecs(result.localProviders) +} + +func cachedPackLocalTopoDirs(cache *packLoadCache, topoDir string) []string { + if cache == nil { + return nil + } + absDir, err := filepath.Abs(topoDir) + if err != nil { + absDir = topoDir + } + result, ok := cache.results[absDir] + if !ok { + return nil + } + return append([]string(nil), result.localTopoDirs...) +} + +func cachedPackLocalRequires(cache *packLoadCache, topoDir string) []PackRequirement { + if cache == nil { + return nil + } + absDir, err := filepath.Abs(topoDir) + if err != nil { + absDir = topoDir + } + result, ok := cache.results[absDir] + if !ok { + return nil + } + return append([]PackRequirement(nil), result.localRequires...) +} + +func cachedPackLocalGlobals(cache *packLoadCache, topoDir string) []ResolvedPackGlobal { + if cache == nil { + return nil + } + absDir, err := filepath.Abs(topoDir) + if err != nil { + absDir = topoDir + } + result, ok := cache.results[absDir] + if !ok { + return nil + } + return deepCopyResolvedPackGlobals(result.localGlobals) +} + +func filterNamedSessionsBySourceDir(namedSessions []NamedSession, sourceDir string) []NamedSession { + if len(namedSessions) == 0 { + return nil + } + absWant, _ := filepath.Abs(sourceDir) + var out []NamedSession + for _, named := range namedSessions { + absDir, _ := filepath.Abs(named.SourceDir) + if absDir == absWant { + out = append(out, named) + } + } + return out +} + func cachedPackDoctors(cache *packLoadCache, topoDir string) []DiscoveredDoctor { if cache == nil { return nil @@ -1596,6 +1810,18 @@ func filterCommandsByPackDir(commands []DiscoveredCommand, packDir string) []Dis return out } +func filterServicesBySourceDir(services []Service, sourceDir string) []Service { + absSource, _ := filepath.Abs(sourceDir) + var out []Service + for _, service := range services { + absDir, _ := filepath.Abs(service.SourceDir) + if absDir == absSource || strings.HasPrefix(absDir, absSource+string(filepath.Separator)) { + out = append(out, service) + } + } + return out +} + func filterDoctorsByPackDir(doctors []DiscoveredDoctor, packDir string) []DiscoveredDoctor { absPackDir, _ := filepath.Abs(packDir) var out []DiscoveredDoctor diff --git a/internal/config/pack_test.go b/internal/config/pack_test.go index b173a1279..d10635883 100644 --- a/internal/config/pack_test.go +++ b/internal/config/pack_test.go @@ -419,15 +419,15 @@ name = "test" [pack] name = "test" schema = 2 -description = "silently accepted before issue 783" +legacy_unknown = "silently accepted before issue 783" `) _, _, err := LoadWithIncludes(fsys.OSFS{}, filepath.Join(dir, "city.toml")) if err == nil { t.Fatal("expected error for unknown root pack.toml field") } - if !strings.Contains(err.Error(), `unknown field "pack.description"`) { - t.Fatalf("error = %v, want unknown field detail for pack.description", err) + if !strings.Contains(err.Error(), `unknown field "pack.legacy_unknown"`) { + t.Fatalf("error = %v, want unknown field detail for pack.legacy_unknown", err) } } diff --git a/internal/config/undecoded.go b/internal/config/undecoded.go index 9025fc2f1..6ce113154 100644 --- a/internal/config/undecoded.go +++ b/internal/config/undecoded.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "path/filepath" "reflect" "sort" "strings" @@ -9,6 +10,17 @@ import ( "github.com/BurntSushi/toml" ) +const agentsAliasWarning = "[agents] is a deprecated compatibility alias for [agent_defaults]; rewrite the table name to [agent_defaults]" + +var agentDefaultsCompatibilityOverlapKeys = []string{ + "model", + "wake_mode", + "default_sling_formula", + "allow_overlay", + "allow_env_override", + "append_fragments", +} + // CheckUndecodedKeys examines TOML metadata for keys that were present in // the input but not mapped to any struct field. For each unknown key, it // computes edit distance against known field names and suggests the closest @@ -23,18 +35,81 @@ func CheckUndecodedKeys(md toml.MetaData, source string) []string { var warnings []string for _, key := range undecoded { keyStr := key.String() - // Skip deeply nested keys — we only warn on section-level and - // field-level keys, not on sub-sub-fields of tables. - suggestion := suggestKey(keyStr, known) - w := fmt.Sprintf("%s: unknown field %q", source, keyStr) - if suggestion != "" { - w += fmt.Sprintf(" (did you mean %q?)", suggestion) + if special, ok := specializedUndecodedWarning(source, keyStr); ok { + warnings = append(warnings, special) + continue + } + warnings = append(warnings, unknownFieldWarning(source, keyStr, known)) + } + return warnings +} + +func fatalUndecodedWarnings(md toml.MetaData, source string) []string { + undecoded := md.Undecoded() + if len(undecoded) == 0 { + return nil + } + known := knownTOMLKeys() + var warnings []string + for _, key := range undecoded { + keyStr := key.String() + if _, ok := specializedUndecodedWarning(source, keyStr); ok { + continue } - warnings = append(warnings, w) + warnings = append(warnings, unknownFieldWarning(source, keyStr, known)) } return warnings } +func unknownFieldWarning(source, key string, known []string) string { + suggestion := suggestKey(key, known) + w := fmt.Sprintf("%s: unknown field %q", source, key) + if suggestion != "" { + w += fmt.Sprintf(" (did you mean %q?)", suggestion) + } + return w +} + +func agentDefaultsCompatibilityWarnings(md toml.MetaData, source string) []string { + if !md.IsDefined("agents") { + return nil + } + warnings := []string{fmt.Sprintf("%s: %s", source, agentsAliasWarning)} + if md.IsDefined("agent_defaults") && agentDefaultsTablesOverlap(md) { + warnings = append(warnings, fmt.Sprintf("%s: both [agent_defaults] and [agents] are present; canonical [agent_defaults] wins for overlapping keys", source)) + } + return warnings +} + +func agentDefaultsTablesOverlap(md toml.MetaData) bool { + for _, key := range agentDefaultsCompatibilityOverlapKeys { + if md.IsDefined("agent_defaults", key) && md.IsDefined("agents", key) { + return true + } + } + return false +} + +func specializedUndecodedWarning(source, key string) (string, bool) { + isPackSource := filepath.Base(source) == "pack.toml" + switch key { + case "agent_defaults.provider", "agents.provider": + if isPackSource { + return fmt.Sprintf("%s: %q is not supported in this release wave; keep setting provider per agent in agents//agent.toml", source, key), true + } + return fmt.Sprintf("%s: %q is not supported in this release wave; keep using workspace.provider (or set provider per agent in agents//agent.toml)", source, key), true + case "agent_defaults.scope", "agents.scope": + return fmt.Sprintf("%s: %q is not supported in this release wave; keep setting scope per agent in agents//agent.toml", source, key), true + case "agent_defaults.install_agent_hooks", "agents.install_agent_hooks": + if isPackSource { + return fmt.Sprintf("%s: %q is not supported in this release wave; keep setting install_agent_hooks per agent in agents//agent.toml", source, key), true + } + return fmt.Sprintf("%s: %q is not supported in this release wave; keep using workspace.install_agent_hooks (or set install_agent_hooks per agent in agents//agent.toml)", source, key), true + default: + return "", false + } +} + // suggestKey finds the closest known key to the given unknown key using // edit distance. Returns the suggestion if the distance is <= 2, or "". func suggestKey(unknown string, known []string) string { diff --git a/internal/config/undecoded_test.go b/internal/config/undecoded_test.go index cdb58c76d..e763c52c3 100644 --- a/internal/config/undecoded_test.go +++ b/internal/config/undecoded_test.go @@ -183,3 +183,271 @@ proivder = "claude" t.Errorf("warning should suggest provider, got: %s", warnings[0]) } } + +func TestParseWithMetaWarnsOnAgentsAlias(t *testing.T) { + input := ` +[workspace] +name = "test" + +[agents] +append_fragments = ["footer"] +` + _, _, warnings, err := parseWithMeta([]byte(input), "test.toml") + if err != nil { + t.Fatalf("parseWithMeta: %v", err) + } + if len(warnings) == 0 { + t.Fatal("expected warning for [agents] alias") + } + found := false + for _, w := range warnings { + if strings.Contains(w, agentsAliasWarning) { + found = true + break + } + } + if !found { + t.Fatalf("expected warning containing %q, got: %v", agentsAliasWarning, warnings) + } +} + +func TestParseWithMetaWarnsWhenCanonicalAndAliasAgentDefaultsBothPresent(t *testing.T) { + input := ` +[workspace] +name = "test" + +[agent_defaults] +append_fragments = ["canonical"] + +[agents] +append_fragments = ["legacy"] +` + _, _, warnings, err := parseWithMeta([]byte(input), "test.toml") + if err != nil { + t.Fatalf("parseWithMeta: %v", err) + } + found := false + for _, w := range warnings { + if strings.Contains(w, "both [agent_defaults] and [agents] are present") { + found = true + break + } + } + if !found { + t.Fatalf("expected mixed-table warning, got: %v", warnings) + } +} + +func TestParseWithMetaSkipsMixedTableWarningWhenCanonicalAndAliasAreDisjoint(t *testing.T) { + input := ` +[workspace] +name = "test" + +[agent_defaults] +append_fragments = ["canonical"] + +[agents] +allow_overlay = ["GC_HOME"] +` + _, _, warnings, err := parseWithMeta([]byte(input), "test.toml") + if err != nil { + t.Fatalf("parseWithMeta: %v", err) + } + for _, w := range warnings { + if strings.Contains(w, "both [agent_defaults] and [agents] are present") { + t.Fatalf("expected no mixed-table warning for disjoint keys, got: %v", warnings) + } + } + foundAlias := false + for _, w := range warnings { + if strings.Contains(w, agentsAliasWarning) { + foundAlias = true + break + } + } + if !foundAlias { + t.Fatalf("expected alias warning, got: %v", warnings) + } +} + +func TestParseWithMetaSkipsMixedTableWarningWhenOverlapIsOnlyUnsupportedFutureKeys(t *testing.T) { + input := ` +[workspace] +name = "test" + +[agent_defaults] +provider = "claude" + +[agents] +provider = "codex" +` + _, _, warnings, err := parseWithMeta([]byte(input), "test.toml") + if err != nil { + t.Fatalf("parseWithMeta: %v", err) + } + for _, w := range warnings { + if strings.Contains(w, "both [agent_defaults] and [agents] are present") { + t.Fatalf("expected no mixed-table warning for unsupported future keys, got: %v", warnings) + } + } + foundUnsupported := false + foundAlias := false + for _, w := range warnings { + if strings.Contains(w, `keep using workspace.provider`) { + foundUnsupported = true + } + if strings.Contains(w, agentsAliasWarning) { + foundAlias = true + } + } + if !foundUnsupported { + t.Fatalf("expected unsupported-key guidance warning, got: %v", warnings) + } + if !foundAlias { + t.Fatalf("expected alias warning, got: %v", warnings) + } +} + +func TestParseWithMetaWarnsOnUnsupportedAgentDefaultsMigrationKeys(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "provider", + input: ` +[workspace] +name = "test" + +[agent_defaults] +provider = "claude" +`, + want: `keep using workspace.provider`, + }, + { + name: "scope", + input: ` +[workspace] +name = "test" + +[agent_defaults] +scope = "rig" +`, + want: `keep setting scope per agent in agents//agent.toml`, + }, + { + name: "install_agent_hooks", + input: ` +[workspace] +name = "test" + +[agent_defaults] +install_agent_hooks = ["hooks/gascity.json"] +`, + want: `keep using workspace.install_agent_hooks`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, warnings, err := parseWithMeta([]byte(tt.input), "test.toml") + if err != nil { + t.Fatalf("parseWithMeta: %v", err) + } + if len(warnings) == 0 { + t.Fatal("expected warning") + } + found := false + for _, w := range warnings { + if strings.Contains(w, tt.want) { + found = true + } + if strings.Contains(w, "unknown field") { + t.Fatalf("got generic unknown-field warning for %s: %v", tt.name, warnings) + } + } + if !found { + t.Fatalf("expected warning containing %q, got: %v", tt.want, warnings) + } + }) + } +} + +func TestParsePackConfigWithMetaWarnsOnPackLocalUnsupportedAgentDefaultsKeys(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "provider", + input: ` +[pack] +name = "test" +schema = 2 + +[agent_defaults] +provider = "claude" +`, + want: `keep setting provider per agent in agents//agent.toml`, + }, + { + name: "install_agent_hooks", + input: ` +[pack] +name = "test" +schema = 2 + +[agent_defaults] +install_agent_hooks = ["hooks/gascity.json"] +`, + want: `keep setting install_agent_hooks per agent in agents//agent.toml`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, warnings, err := parsePackConfigWithMeta([]byte(tt.input), "/city/packs/test/pack.toml") + if err != nil { + t.Fatalf("parsePackConfigWithMeta: %v", err) + } + if len(warnings) == 0 { + t.Fatal("expected warning") + } + found := false + for _, w := range warnings { + if strings.Contains(w, tt.want) { + found = true + } + if strings.Contains(w, "workspace.") { + t.Fatalf("pack warning should not point at workspace.*, got: %v", warnings) + } + } + if !found { + t.Fatalf("expected warning containing %q, got: %v", tt.want, warnings) + } + }) + } +} + +func TestParsePackConfigWithMetaAllowsPackDescription(t *testing.T) { + input := ` +[pack] +name = "core" +version = "0.1.0" +schema = 2 +description = "Built-in reference skills." +` + + cfg, warnings, err := parsePackConfigWithMeta([]byte(input), "/city/.gc/system/packs/core/pack.toml") + if err != nil { + t.Fatalf("parsePackConfigWithMeta: %v", err) + } + if cfg.Pack.Description != "Built-in reference skills." { + t.Fatalf("Pack.Description = %q, want %q", cfg.Pack.Description, "Built-in reference skills.") + } + if len(warnings) != 0 { + t.Fatalf("warnings = %v, want none", warnings) + } +} diff --git a/internal/migrate/migrate_test.go b/internal/migrate/migrate_test.go index cba471dfa..ec30347fc 100644 --- a/internal/migrate/migrate_test.go +++ b/internal/migrate/migrate_test.go @@ -162,7 +162,7 @@ includes = ["../packs/gastown"] [pack] name = "legacy-city" schema = 2 -description = "silently dropped before strict migration validation" +legacy_unknown = "silently dropped before strict migration validation" `) beforeCity := readFile(t, filepath.Join(cityDir, "city.toml")) @@ -172,8 +172,8 @@ description = "silently dropped before strict migration validation" if err == nil { t.Fatal("expected Apply to reject unknown pack.toml field") } - if !strings.Contains(err.Error(), `unknown field "pack.description"`) { - t.Fatalf("error = %v, want unknown field detail for pack.description", err) + if !strings.Contains(err.Error(), `unknown field "pack.legacy_unknown"`) { + t.Fatalf("error = %v, want unknown field detail for pack.legacy_unknown", err) } if got := readFile(t, filepath.Join(cityDir, "city.toml")); got != beforeCity { t.Fatalf("city.toml changed after validation failure:\n%s", got) @@ -513,18 +513,20 @@ func TestAgentConfigFromAgentCoversPersistedFields(t *testing.T) { } omitted := map[string]bool{ - "Name": true, - "PromptTemplate": true, - "Namepool": true, - "NamepoolNames": true, - "OverlayDir": true, - "SourceDir": true, - "Implicit": true, - "Fallback": true, - "SleepAfterIdleSource": true, - "PoolName": true, - "BindingName": true, - "PackName": true, + "Name": true, + "PromptTemplate": true, + "Namepool": true, + "NamepoolNames": true, + "OverlayDir": true, + "SourceDir": true, + "InheritedDefaultSlingFormula": true, + "InheritedAppendFragments": true, + "Implicit": true, + "Fallback": true, + "SleepAfterIdleSource": true, + "PoolName": true, + "BindingName": true, + "PackName": true, // v0.15.1 tombstones — still on Agent but intentionally not propagated // by migrate (removed in v0.16). "Skills": true, diff --git a/test/agents/graph-dispatch.sh b/test/agents/graph-dispatch.sh index 4974838b6..fa10cdde2 100755 --- a/test/agents/graph-dispatch.sh +++ b/test/agents/graph-dispatch.sh @@ -593,14 +593,7 @@ while true; do fi fi fi - if [ "$sibling_hard_fail" = "true" ]; then - case "$ref" in - *.cleanup-worktree*) - sibling_hard_fail="false" - ;; - esac - fi - if [ "$sibling_hard_fail" = "true" ]; then + if [ "$sibling_hard_fail" = "true" ] && [[ "$ref" != *.cleanup-worktree* ]]; then trace "skip-sibling-hard-fail bead=$bead_id ref=$ref root=$root_id (sibling has outcome=fail class=hard; closing self as skipped)" bd update "$bead_id" --set-metadata "gc.outcome=skipped" --status closed 2>/dev/null || \ trace "skip-close-failed bead=$bead_id ref=$ref" diff --git a/test/integration/e2e_events_test.go b/test/integration/e2e_events_test.go index b34371e19..dc8cd0352 100644 --- a/test/integration/e2e_events_test.go +++ b/test/integration/e2e_events_test.go @@ -107,10 +107,11 @@ func TestE2E_AgentLifecycleEvents(t *testing.T) { // Verify session.woke event exists. verifyEvents(t, cityDir, "session.woke") - // Stop the agent. - out, err := gc("", "stop", cityDir) + // Restart the city so we observe a session stop event without tearing the + // city out of supervisor scope before querying the API. + out, err := gc("", "restart", cityDir) if err != nil { - t.Fatalf("gc stop failed: %v\noutput: %s", err, out) + t.Fatalf("gc restart failed: %v\noutput: %s", err, out) } // Give the event log a moment. diff --git a/test/integration/e2e_helpers_test.go b/test/integration/e2e_helpers_test.go index ba3ebfeb1..ed8435af3 100644 --- a/test/integration/e2e_helpers_test.go +++ b/test/integration/e2e_helpers_test.go @@ -120,33 +120,27 @@ func (r *e2eReport) hasKey(key string) bool { // renderE2EToml generates a full single-file template for gc init --file. func renderE2EToml(city e2eCity) string { var b strings.Builder - writeE2EWorkspaceSection(&b, city.Workspace) b.WriteString("\n[beads]\nprovider = \"file\"\n") writeE2EProviderSections(&b, city.Providers) writeE2EAgentSections(&b, city.Agents) writeE2ENamedSessionSections(&b, city.Agents) - return b.String() } func renderE2ECityRuntimeToml(city e2eCity) string { var b strings.Builder - writeE2EWorkspaceSection(&b, city.Workspace) b.WriteString("\n[beads]\nprovider = \"file\"\n") - return b.String() } func renderE2EPackToml(city e2eCity) string { var b strings.Builder - fmt.Fprintf(&b, "[pack]\nname = %s\nschema = 2\n", quote(city.Workspace.Name)) writeE2EProviderSections(&b, city.Providers) writeE2EAgentSections(&b, city.Agents) writeE2ENamedSessionSections(&b, city.Agents) - return b.String() } @@ -264,11 +258,13 @@ func writeE2EToml(t *testing.T, cityDir string, city e2eCity) { t.Helper() packPath := filepath.Join(cityDir, "pack.toml") - if err := os.WriteFile(packPath, []byte(renderE2EPackToml(city)), 0o644); err != nil { - t.Fatalf("writing pack.toml: %v", err) - } tomlPath := filepath.Join(cityDir, "city.toml") - writeFileAtomic(t, tomlPath, []byte(renderE2ECityRuntimeToml(city))) + if _, err := os.Stat(packPath); err == nil { + writeFileAtomic(t, packPath, []byte(renderE2EPackToml(city))) + writeFileAtomic(t, tomlPath, []byte(renderE2ECityRuntimeToml(city))) + return + } + writeFileAtomic(t, tomlPath, []byte(renderE2EToml(city))) } func writeE2ETomlFile(t *testing.T, tomlPath string, city e2eCity) { diff --git a/test/integration/huma_binary_test.go b/test/integration/huma_binary_test.go index d641e7c1a..2f03e6820 100644 --- a/test/integration/huma_binary_test.go +++ b/test/integration/huma_binary_test.go @@ -39,15 +39,13 @@ func TestHumaBinary_SupervisorBootsAndServesSpec(t *testing.T) { runtimeDir := shortTempDir(t) port := reserveFreePort(t) writeSupervisorConfig(t, gcHome, port) + if err := seedDoltIdentityForRoot(gcHome); err != nil { + t.Fatalf("seed dolt identity: %v", err) + } baseURL := "http://127.0.0.1:" + strconv.Itoa(port) cityRoot := filepath.Join(gcHome, "city") - env := append(os.Environ(), - "GC_HOME="+gcHome, - "XDG_RUNTIME_DIR="+runtimeDir, - "GC_BEADS=file", - "GC_DOLT=skip", - ) + env := integrationEnvFor(gcHome, runtimeDir, true) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -124,8 +122,8 @@ func TestHumaBinary_SupervisorBootsAndServesSpec(t *testing.T) { cityListURL := baseURL + "/v0/cities" waitForCityRegistered(t, cityListURL, "humatest", 5*time.Second) - // 3) `gc city status` — resolves the city path, then calls per-city status. - runCLI(t, bin, env, "gc city status", "--city", cityRoot, "status") + // 3) `gc status` — per-city status against the supervisor-managed city path. + runCLI(t, bin, env, "gc status", "--city", cityRoot, "status") // 4) `gc session list` — per-city, exercises a different domain handler. runCLI(t, bin, env, "gc session list", "--city", cityRoot, "session", "list") diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index ef8e09136..b60553a0e 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -379,26 +379,16 @@ func subprocessTestKillSet(procs map[int]procSnapshot, agentScript string) map[i // gc runs the gc binary with the given args. If dir is non-empty, it sets // the working directory. Returns combined stdout+stderr and any error. func gc(dir string, args ...string) (string, error) { - return runCommand(dir, commandEnvForDir(commandEnvLookupDir(dir, args), false), gcCommandTimeout(args), gcBinary, args...) + envDir := commandCityDirForArgs(dir, args) + return runCommand(dir, commandEnvForDir(envDir, false), gcCommandTimeout(args), gcBinary, args...) } // gcDolt runs the gc binary with the given args using the isolated integration // supervisor state, but without forcing GC_DOLT=skip. Use this for tests that // need the real bd+dolt-backed bead store. func gcDolt(dir string, args ...string) (string, error) { - return runCommand(dir, commandEnvForDir(commandEnvLookupDir(dir, args), true), integrationGCDoltCommandTimeout, gcBinary, args...) -} - -func commandEnvLookupDir(dir string, args []string) string { - if dir != "" { - return dir - } - for _, arg := range args { - if _, ok := cityCommandEnv.Load(arg); ok { - return arg - } - } - return "" + envDir := commandCityDirForArgs(dir, args) + return runCommand(dir, commandEnvForDir(envDir, true), integrationGCDoltCommandTimeout, gcBinary, args...) } // bd runs the bd binary with the given args. If dir is non-empty, it sets @@ -863,6 +853,23 @@ func commandEnvForDir(dir string, useDolt bool) []string { return integrationEnv() } +func commandCityDirForArgs(dir string, args []string) string { + if dir != "" || len(args) < 2 { + return dir + } + switch args[0] { + case "start", "stop", "restart", "suspend", "resume": + if filepath.IsAbs(args[1]) { + return args[1] + } + } + return dir +} + +func commandEnvLookupDir(dir string, args []string) string { + return commandCityDirForArgs(dir, args) +} + func replaceEnv(env []string, name, value string) []string { env = filterEnv(env, name) return append(env, name+"="+value)