gc CLI prints the always+fresh named_session advisory on every command's stderr — 7.3% of all tool-result text city-wide (gc-dqn8l)

cmd/gc/cmd_agent.go

@@ -163,10 +163,25 @@ func isNonFatalLoadConfigWarning(warning string) bool {
 	return strings.Contains(warning, `"agent_defaults.`) || strings.Contains(warning, `"agents.`)
 }
 
+// shouldEmitLoadCityConfigWarning reports whether a config-load warning is
+// worth printing on the stderr of an arbitrary command. Migration guidance that
+// a user must act on qualifies; static lints about a property of city.toml do
+// not. The shared loadCityConfigFS path runs on nearly every gc invocation, so
+// anything returning true here is reprinted for the life of the config — and
+// agent harnesses merge stderr into the tool result, which turned the
+// always+fresh advisory alone into 7.3% of all tool-result text city-wide
+// (gc-dqn8l). Suppression here is not concealment: gc start and gc config both
+// print raw prov.Warnings without consulting this filter, and strict mode
+// classifies independently via strictWarningIsNonFatal.
 func shouldEmitLoadCityConfigWarning(warning string) bool {
 	if config.IsLegacyWorkspaceFieldWarning(warning) {
 		return false
 	}
+	// A lint about a static property of city.toml — identical on every
+	// invocation, and unchanged by anything the current command did.
+	if config.IsAlwaysFreshWakeModeWarning(warning) {
+		return false
+	}
 	if strings.Contains(warning, "both [agent_defaults] and [agents] are present") {
 		return true
 	}

cmd/gc/cmd_agent_test.go

@@ -426,6 +426,7 @@ func TestEmitLoadCityConfigWarningsFiltersNonMigrationWarnings(t *testing.T) {
 			`/city/pack.toml: both [agent_defaults] and [agents] are present; [agent_defaults] wins on overlapping keys and [agents] only fills gaps`,
 			`/city/city.toml: workspace.global_fragments is deprecated: Use [agent_defaults] append_fragments or explicit template includes instead.`,
 			`gc: warning: attachment-list fields (` + "`skills`, `mcp`, `skills_append`, `mcp_append`, `shared_skills`" + `) are deprecated as of v0.15.1 and ignored.`,
+			`named_session "gastown.mayor": mode "always" with wake_mode "fresh" on template "gastown.mayor" starts a fresh provider session after every drain; use only for a deliberate restart-per-cycle actor`,
 		},
 	})
 
@@ -445,6 +446,9 @@ func TestEmitLoadCityConfigWarningsFiltersNonMigrationWarnings(t *testing.T) {
 	if !strings.Contains(output, "attachment-list fields") {
 		t.Fatalf("expected attachment deprecation warning, got %q", output)
 	}
+	if strings.Contains(output, "starts a fresh provider session after every drain") {
+		t.Fatalf("always+fresh advisory should stay out of generic command stderr, got %q", output)
+	}
 }
 
 func TestDoAgentSuspendRootPackAgent(t *testing.T) {

cmd/gc/cmd_github.go

@@ -161,11 +161,13 @@ func doGitHubPRBackfill(opts githubPRBackfillOptions, stdout, stderr io.Writer)
 		fmt.Fprintf(stderr, "gc github pr backfill: %v\n", err) //nolint:errcheck // best-effort stderr
 		return 1
 	}
-	if !opts.jsonOutput {
-		for _, warning := range prov.Warnings {
-			fmt.Fprintf(stderr, "gc github pr backfill: warning: %s\n", warning) //nolint:errcheck // best-effort stderr
-		}
-	}
+	// Route through the shared filter rather than printing prov.Warnings raw.
+	// This command's subject is GitHub PR readiness, not the config, so it gets
+	// the same treatment as every other non-config command: actionable
+	// migration guidance prints, static city.toml lints stay quiet, and
+	// repeated warnings are deduped. The explicit config surfaces (gc start,
+	// gc config) still print prov.Warnings unfiltered (gc-dqn8l).
+	emitLoadCityConfigWarnings(configWarnWriter(opts.jsonOutput, stderr), prov)
 
 	monitors, err := selectGitHubPRMonitors(cfg, opts.monitorName)
 	if err != nil {

cmd/gc/cmd_github_test.go

@@ -419,3 +419,106 @@ func TestGitHubPRBackfillCommandPropagatesRepairStoreError(t *testing.T) {
 		t.Fatalf("stderr = %q, want store error", stderr.String())
 	}
 }
+
+// appendMixedWarningConfig adds two config shapes to an existing test city, one
+// per side of the shouldEmitLoadCityConfigWarning split:
+//
+//   - a mode="always" named session on a wake_mode="fresh" template, the shape
+//     config.ValidateNamedSessions reports the always+fresh advisory for. The
+//     filter SUPPRESSES it — a static lint about city.toml, unchanged by
+//     whatever the current command did.
+//   - both [agent_defaults] and [agents] tables, whose ambiguity warning the
+//     filter KEEPS — the operator must act on it.
+//
+// A command wired to the filter therefore prints exactly one of the two, which
+// is what distinguishes a real filter from a blanket mute.
+func appendMixedWarningConfig(t *testing.T, cityPath string) {
+	t.Helper()
+	tomlPath := filepath.Join(cityPath, "city.toml")
+	existing, err := os.ReadFile(tomlPath)
+	if err != nil {
+		t.Fatalf("read city.toml: %v", err)
+	}
+	body := string(existing) + `
+[[agent]]
+name = "watchdog"
+wake_mode = "fresh"
+
+[[named_session]]
+template = "watchdog"
+mode = "always"
+
+[agent_defaults]
+model = "sonnet"
+
+[agents]
+model = "sonnet"
+`
+	if err := os.WriteFile(tomlPath, []byte(body), 0o644); err != nil {
+		t.Fatalf("write city.toml: %v", err)
+	}
+}
+
+// TestGitHubPRBackfillSuppressesAlwaysFreshAdvisory proves `gc github pr
+// backfill` does not reprint the static always+fresh city.toml advisory on
+// stderr. The command is not a config surface: its subject is GitHub PR
+// readiness, so a lint about a property of city.toml that is identical on every
+// invocation is pure noise there. gc-dqn8l removed that reprint from the shared
+// emit path, but this command iterated prov.Warnings directly and so kept
+// emitting it — a reachable command path with the bug the branch removes.
+//
+// The absence assertion is guarded two ways so it cannot pass vacuously. It
+// first proves the fixture still provokes the advisory at config-load time, so
+// a validator that stopped emitting it fails here instead of silently turning
+// this into an empty test. And it classifies stderr with
+// config.IsAlwaysFreshWakeModeWarning — the same exported predicate the fix
+// consults — rather than a copy of the message text, so a reworded advisory
+// cannot slip past. The paired positive assertion (a kept migration warning)
+// distinguishes a filter from a blanket mute.
+func TestGitHubPRBackfillSuppressesAlwaysFreshAdvisory(t *testing.T) {
+	cityPath := writeGitHubMonitorTestCity(t)
+	appendMixedWarningConfig(t, cityPath)
+
+	// The fixture must actually provoke the advisory, or the absence assertion
+	// below proves nothing.
+	_, prov, err := loadConfigCommandCityConfig(cityPath)
+	if err != nil {
+		t.Fatalf("load fixture city: %v", err)
+	}
+	advisories := 0
+	for _, w := range prov.Warnings {
+		if config.IsAlwaysFreshWakeModeWarning(w) {
+			advisories++
+		}
+	}
+	if advisories == 0 {
+		t.Fatalf("fixture no longer provokes the always+fresh advisory; warnings = %q", prov.Warnings)
+	}
+
+	oldToken := resolveGitHubTokenForBackfill
+	oldClient := newGitHubPRBackfillClient
+	resolveGitHubTokenForBackfill = func(context.Context) (string, error) { return "token", nil }
+	newGitHubPRBackfillClient = func(string) githubPRLister {
+		return fakeGitHubPRLister{prs: []githubmonitor.PullRequest{
+			{Number: 1, BaseRefName: "main", HeadSHA: "abc", MergeStateStatus: "DIRTY"},
+		}}
+	}
+	t.Cleanup(func() {
+		resolveGitHubTokenForBackfill = oldToken
+		newGitHubPRBackfillClient = oldClient
+	})
+
+	var stdout, stderr bytes.Buffer
+	code := run([]string{"--city", cityPath, "github", "pr", "backfill", "partcl-main"}, &stdout, &stderr)
+	if code != 0 {
+		t.Fatalf("run code = %d, stdout = %q, stderr = %q", code, stdout.String(), stderr.String())
+	}
+	for _, line := range strings.Split(stderr.String(), "\n") {
+		if config.IsAlwaysFreshWakeModeWarning(line) {
+			t.Fatalf("always+fresh advisory must stay off non-config command stderr, got %q", stderr.String())
+		}
+	}
+	if !strings.Contains(stderr.String(), "both [agent_defaults] and [agents] are present") {
+		t.Fatalf("actionable migration warning must survive the filter, got %q", stderr.String())
+	}
+}

cmd/gc/strict_warnings_test.go

@@ -6,18 +6,26 @@ import (
 	"github.com/gastownhall/gascity/internal/config"
 )
 
-// TestAlwaysFreshWakeModeWarningIsNonFatalAndEmitted proves the always+fresh
-// advisory behaves like a warning on both downstream re-classifiers of config
+// TestAlwaysFreshWakeModeWarningIsNonFatalAndUnprinted proves the always+fresh
+// advisory behaves correctly on both downstream re-classifiers of config
 // warnings: strict mode — on by default for `gc start` — keeps it NON-FATAL,
-// and the agent warning-emit path SURFACES it. The bundled gastown pack trips
-// this warning, so without the config.IsAlwaysFreshWakeModeWarning wiring
-// `gc start --foreground` / `--controller` / `--dry-run` exits 1 on the shipped
-// example city, and `gc agent` drops the advisory silently.
+// and the shared per-command warning-emit path SUPPRESSES it. The bundled
+// gastown pack trips this warning, so without the
+// config.IsAlwaysFreshWakeModeWarning wiring `gc start --foreground` /
+// `--controller` / `--dry-run` exits 1 on the shipped example city.
+//
+// The advisory is a lint about a static property of city.toml: it never
+// changes between invocations, so repeating it on the stderr of every `gc bd
+// show` / `gc bd list` buys nothing. Agent harnesses merge
+// stderr into the tool result, which made this one block 7.3% of all
+// tool-result text city-wide (gc-dqn8l). It stays discoverable on the surfaces
+// whose subject IS the config — `gc start` and `gc config` both print raw
+// prov.Warnings, neither of which consults shouldEmitLoadCityConfigWarning.
 //
 // The warning text is derived from config.ValidateNamedSessions rather than
 // hardcoded so this test cannot pass against a string the validator no longer
 // emits.
-func TestAlwaysFreshWakeModeWarningIsNonFatalAndEmitted(t *testing.T) {
+func TestAlwaysFreshWakeModeWarningIsNonFatalAndUnprinted(t *testing.T) {
 	warnings, err := config.ValidateNamedSessions(&config.City{
 		Workspace: config.Workspace{Name: "test-city"},
 		Agents:    []config.Agent{{Name: "watchdog", WakeMode: "fresh"}},
@@ -41,8 +49,8 @@ func TestAlwaysFreshWakeModeWarningIsNonFatalAndEmitted(t *testing.T) {
 	if len(fatal) != 0 || len(nonFatal) != 1 {
 		t.Errorf("strict split: fatal=%v nonFatal=%v, want the always+fresh warning non-fatal", fatal, nonFatal)
 	}
-	if !shouldEmitLoadCityConfigWarning(w) {
-		t.Error("an always+fresh warning must be emitted to the operator, not swallowed")
+	if shouldEmitLoadCityConfigWarning(w) {
+		t.Error("an always+fresh warning must not be repeated on every command's stderr")
 	}
 }
 

internal/config/config.go

@@ -4196,9 +4196,9 @@ func validateNamedSessions(cfg *City, requireBackingTemplate bool) (warnings []s
 const alwaysFreshWakeModeMarker = "starts a fresh provider session after every drain"
 
 // IsAlwaysFreshWakeModeWarning reports whether a load warning is the non-fatal
-// always+fresh advisory. CLI warning filters use this to print the notice and
-// keep it non-fatal in strict mode. Keep in sync with
-// alwaysFreshWakeModeMarker.
+// always+fresh advisory. CLI warning filters use this to keep the notice
+// non-fatal in strict mode and to hold it off the stderr of commands whose
+// subject is not the config. Keep in sync with alwaysFreshWakeModeMarker.
 func IsAlwaysFreshWakeModeWarning(warning string) bool {
 	return strings.Contains(warning, alwaysFreshWakeModeMarker)
 }