fix: exclude pre_start-staged CopyFiles from config fingerprint (#682) (#695)

## Summary

Fixes #682 — polecat config-drift false positive from non-deterministic
CopyFiles hash.

`cmd/gc.stageHookFiles` probes workDir-relative paths (`.claude/skills`,
`.gemini/settings.json`, `.codex/hooks.json`, etc.) and captures their
content hashes into `CopyEntry.ContentHash`. Those destinations are
populated by `pre_start` scripts such as `worktree-setup.sh --sync`, so
the hash computed at session startup differs from the hash the
reconciler re-computes after `pre_start` completes. `CoreFingerprint`
diverges, config-drift fires, and the polecat is drained within 1–2
cycles of starting. Every replacement polecat hits the same race,
producing the thrash loop described in #682 (same `config_revision`,
three distinct `CopyFiles` breakdown hashes across cycles 1/4/6).

## Fix

Add `SkipFingerprint bool` to `runtime.CopyEntry`. All four fingerprint
consumers (`runtime.hashCoreFields`, `runtime.CoreFingerprintBreakdown`,
`runtime.LogCoreFingerprintDrift`, and the dormant
`cmd/gc.canonicalConfigHash`) skip entries where `cf.Probed &&
cf.SkipFingerprint` are both true.

The `Probed` precondition is deliberate: it guarantees that a future
caller who mistakenly sets `SkipFingerprint=true` on a config-derived
entry cannot silently mute a real config change.
`TestSkipFingerprintIgnoredOnConfigDerivedEntries` locks this contract
in.

`stageHookFiles` sets the flag on every workDir-based probed entry (the
7-provider hook loop + the `.claude/skills` stage). It also drops the
`runtime.HashPathContent()` call for those entries since the result is
now discarded — eliminating a recursive SHA-256 walk of `.claude/skills`
on every reconciler tick per polecat. The cityDir fallback
(`.gc/settings.json` / `hooks/claude.json`) is deliberately left
unmarked so real user edits still drive drain.
`TestStageHookFilesIncludesCanonicalClaudeHook` and
`TestStageHookFilesFallsBackToLegacyClaudeHook` both positively assert
`SkipFingerprint==false` on that branch.

## Test plan

- [x] `TestSkipFingerprintExcludesFromCoreHash` — skipped entries
produce the same fingerprint as a baseline Config regardless of
`ContentHash` (proves exclusion is total, not a `ContentHash==""`
shortcut).
- [x] `TestSkipFingerprintIgnoredOnConfigDerivedEntries` —
`Probed=false, SkipFingerprint=true` on a config-derived entry still
contributes to the fingerprint; changes to `Src` still drive drift.
Footgun guard.
- [x] `TestSkipFingerprintStableUnderFilesystemChurn` — regression test
for the exact #682 bug: probe a tmp skills dir, write a file to it,
re-probe. Without `SkipFingerprint` the hash drifts (locked in as a
precondition assertion so the test can't vacuously pass). With
`SkipFingerprint` the hash stays stable. A naive "skip when ContentHash
empty" implementation fails this test.
- [x] `TestLogCoreFingerprintDriftSkipsExcludedEntries` — diagnostic
output does not leak skipped entries.
- [x] `TestStageHookFilesIncludesCodexAndCopilotExecutableHooks`
extended: every workDir-based probed entry has `SkipFingerprint==true`
AND `ContentHash==""` (verifying the dropped `HashPathContent` call).
- [x] `TestStageHookFilesIncludesCanonicalClaudeHook` /
`TestStageHookFilesFallsBackToLegacyClaudeHook` extended: cityDir
fallback entries must NOT have `SkipFingerprint`.
- [x] `go build ./...` clean
- [x] `go vet ./...` clean
- [x] `make check` fmt-check + lint + vet clean
- [x] `go test ./internal/runtime/... ./cmd/gc/` PASS (two baseline
flakes verified failing identically on `origin/main`:
`TestHandleProviderReadinessReturnsNotInstalledWhenBinaryMissing` in
`internal/api`,
`TestControllerReloadsNamedSessionModeAndAppliesIdleTimeout` in `cmd/gc`
— neither is in this PR's code paths)

## Upgrade note

Live sessions persisted a `started_config_hash` computed with the old
(unstable) `CopyFiles` hashing. On the first reconciler tick after
upgrade, those hashes will not match the post-fix `CoreFingerprint`,
triggering one legit drift drain per live session. This is a one-time
cost that was already happening continuously pre-fix; the thrash loop
stops after the single replacement cycle.

---------

Co-authored-by: sjarmak <sjarmak@users.noreply.github.com>

Author: sjarmak

cmd/gc/cmd_start.go

@@ -734,6 +734,9 @@ func stageHookFiles(copyFiles []runtime.CopyEntry, cityPath, workDir string) []r
 
 	// workDir-based hooks: gemini, codex, opencode, copilot, pi, omp.
 	// Use path.Join for RelDst (container-target, always forward slashes).
+	// These paths live inside the agent worktree and are populated by
+	// pre_start staging (e.g. worktree-setup.sh --sync), so hashing their
+	// contents would drift across reconciler cycles. See issue #682.
 	for _, rel := range []string{
 		path.Join(".gemini", "settings.json"),
 		path.Join(".codex", "hooks.json"),
@@ -747,7 +750,7 @@ func stageHookFiles(copyFiles []runtime.CopyEntry, cityPath, workDir string) []r
 		if _, err := os.Stat(abs); err == nil {
 			copyFiles = append(copyFiles, runtime.CopyEntry{
 				Src: abs, RelDst: path.Join(relWorkDir, rel),
-				Probed: true, ContentHash: runtime.HashPathContent(abs),
+				Probed: true, SkipFingerprint: true,
 			})
 		}
 	}
@@ -756,7 +759,7 @@ func stageHookFiles(copyFiles []runtime.CopyEntry, cityPath, workDir string) []r
 	if info, err := os.Stat(skillsDir); err == nil && info.IsDir() {
 		copyFiles = append(copyFiles, runtime.CopyEntry{
 			Src: skillsDir, RelDst: path.Join(relWorkDir, ".claude", "skills"),
-			Probed: true, ContentHash: runtime.HashPathContent(skillsDir),
+			Probed: true, SkipFingerprint: true,
 		})
 	}
 	// cityDir-based hooks: claude (.gc/settings.json).

cmd/gc/cmd_start_test.go

@@ -284,13 +284,15 @@ func TestStageHookFilesIncludesCodexAndCopilotExecutableHooks(t *testing.T) {
 			t.Errorf("stageHookFiles() missing %q (got %v)", want, rels)
 		}
 	}
-	// All filesystem-probed entries must be marked Probed with a ContentHash.
 	for _, entry := range got {
 		if !entry.Probed {
 			t.Errorf("stageHookFiles() entry %q not marked Probed", entry.RelDst)
 		}
-		if entry.ContentHash == "" {
-			t.Errorf("stageHookFiles() entry %q has empty ContentHash", entry.RelDst)
+		if !entry.SkipFingerprint {
+			t.Errorf("stageHookFiles() workDir entry %q must have SkipFingerprint=true (#682)", entry.RelDst)
+		}
+		if entry.ContentHash != "" {
+			t.Errorf("stageHookFiles() workDir entry %q should skip HashPathContent, got %q (#682)", entry.RelDst, entry.ContentHash)
 		}
 	}
 }
@@ -319,6 +321,10 @@ func TestStageHookFilesIncludesCanonicalClaudeHook(t *testing.T) {
 			if entry.ContentHash == "" {
 				t.Fatal("stageHookFiles() .gc/settings.json has empty ContentHash")
 			}
+			// City-level settings MUST still drive config drift — never skip.
+			if entry.SkipFingerprint {
+				t.Fatal("stageHookFiles() .gc/settings.json must not have SkipFingerprint (cityDir fallback must drive drain on real edits)")
+			}
 			return
 		}
 	}
@@ -348,6 +354,10 @@ func TestStageHookFilesFallsBackToLegacyClaudeHook(t *testing.T) {
 			if entry.ContentHash == "" {
 				t.Fatal("stageHookFiles() hooks/claude.json has empty ContentHash")
 			}
+			// City-level legacy fallback must still drive drain.
+			if entry.SkipFingerprint {
+				t.Fatal("stageHookFiles() hooks/claude.json must not have SkipFingerprint (cityDir fallback must drive drain)")
+			}
 			return
 		}
 	}

cmd/gc/config_hash.go

@@ -102,8 +102,15 @@ func canonicalConfigHash(params TemplateParams, overlay map[string]string) strin
 	h.Write([]byte(params.Hints.OverlayDir)) //nolint:errcheck
 	h.Write([]byte{0})                       //nolint:errcheck
 
-	// CopyFiles.
+	// CopyFiles. Mirrors runtime.hashCoreFields only in excluding probed
+	// workDir entries marked SkipFingerprint, so this hash stays stable if
+	// that dormant path is ever wired into drift detection (#682). Unlike
+	// runtime hashing, this canonical hash still fingerprints CopyFiles by
+	// Src and RelDst here; it does not use ContentHash or a sentinel.
 	for _, cf := range params.Hints.CopyFiles {
+		if cf.Probed && cf.SkipFingerprint {
+			continue
+		}
 		h.Write([]byte(cf.Src))    //nolint:errcheck
 		h.Write([]byte{0})         //nolint:errcheck
 		h.Write([]byte(cf.RelDst)) //nolint:errcheck

internal/runtime/fingerprint.go

@@ -164,7 +164,13 @@ func hashCoreFields(h hash.Hash, cfg Config) {
 	// use Src/RelDst paths. When a probed entry has an empty ContentHash
 	// (transient I/O error), a stable sentinel is used instead of falling
 	// back to path-based hashing, which would flip fingerprint modes.
+	// Probed entries with SkipFingerprint are excluded entirely — see
+	// issue #682. SkipFingerprint is ignored on config-derived entries
+	// so real config changes always drive drain.
 	for _, cf := range cfg.CopyFiles {
+		if cf.Probed && cf.SkipFingerprint {
+			continue
+		}
 		if cf.Probed {
 			h.Write([]byte(cf.RelDst)) //nolint:errcheck // hash.Write never errors
 			h.Write([]byte{0})         //nolint:errcheck // hash.Write never errors
@@ -271,6 +277,9 @@ func CoreFingerprintBreakdown(cfg Config) map[string]string {
 		}),
 		"CopyFiles": fieldHash(func(h hash.Hash) {
 			for _, cf := range cfg.CopyFiles {
+				if cf.Probed && cf.SkipFingerprint {
+					continue
+				}
 				if cf.Probed {
 					h.Write([]byte(cf.RelDst))
 					h.Write([]byte{0})
@@ -335,6 +344,9 @@ func LogCoreFingerprintDrift(w io.Writer, name string, storedBreakdown map[strin
 			fmt.Fprintf(w, "    SessionSetupScript len: %d\n", len(current.SessionSetupScript)) //nolint:errcheck // best-effort diag
 		case "CopyFiles":
 			for i, cf := range current.CopyFiles {
+				if cf.Probed && cf.SkipFingerprint {
+					continue
+				}
 				fmt.Fprintf(w, "    CopyFiles[%d]: RelDst=%q ContentHash=%q\n", i, cf.RelDst, cf.ContentHash) //nolint:errcheck // best-effort diag
 			}
 		}

internal/runtime/fingerprint_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"os"
 	"path/filepath"
+	"slices"
 	"testing"
 )
 
@@ -450,3 +451,139 @@ func TestLogCoreFingerprintDriftCopyFiles(t *testing.T) {
 		t.Errorf("expected RelDst detail in CopyFiles drift output, got: %s", out)
 	}
 }
+
+// TestSkipFingerprintExcludesFromCoreHash locks in the fix for issue #682:
+// CopyEntry values marked SkipFingerprint must not influence CoreFingerprint
+// regardless of their ContentHash, Src, or RelDst values.
+func TestSkipFingerprintExcludesFromCoreHash(t *testing.T) {
+	base := Config{Command: "claude"}
+	skipA := Config{Command: "claude", CopyFiles: []CopyEntry{
+		{
+			Src: "/tmp/worktree/.claude/skills", RelDst: ".claude/skills",
+			Probed: true, ContentHash: "aaaa", SkipFingerprint: true,
+		},
+	}}
+	skipB := Config{Command: "claude", CopyFiles: []CopyEntry{
+		{
+			Src: "/tmp/worktree/.claude/skills", RelDst: ".claude/skills",
+			Probed: true, ContentHash: "bbbb", SkipFingerprint: true,
+		},
+	}}
+	skipEmpty := Config{Command: "claude", CopyFiles: []CopyEntry{
+		{
+			Src: "/tmp/worktree/.claude/skills", RelDst: ".claude/skills",
+			Probed: true, ContentHash: "", SkipFingerprint: true,
+		},
+	}}
+
+	baseH := CoreFingerprint(base)
+	if got := CoreFingerprint(skipA); got != baseH {
+		t.Errorf("skipped entry changed fingerprint: base=%s got=%s", baseH, got)
+	}
+	if got := CoreFingerprint(skipB); got != baseH {
+		t.Errorf("skipped entry with different ContentHash changed fingerprint: base=%s got=%s", baseH, got)
+	}
+	if got := CoreFingerprint(skipEmpty); got != baseH {
+		t.Errorf("skipped entry with empty ContentHash changed fingerprint: base=%s got=%s", baseH, got)
+	}
+	// Breakdown must also report the same CopyFiles field hash as the base.
+	baseBD := CoreFingerprintBreakdown(base)
+	skipABD := CoreFingerprintBreakdown(skipA)
+	if baseBD["CopyFiles"] != skipABD["CopyFiles"] {
+		t.Errorf("skipped entry changed breakdown: base=%s got=%s", baseBD["CopyFiles"], skipABD["CopyFiles"])
+	}
+}
+
+// TestSkipFingerprintIgnoredOnConfigDerivedEntries ensures the doc contract
+// is enforced: SkipFingerprint is only honored on probed entries. A
+// config-derived entry (Probed=false) with SkipFingerprint=true must still
+// contribute to CoreFingerprint so real user edits drive drain.
+func TestSkipFingerprintIgnoredOnConfigDerivedEntries(t *testing.T) {
+	base := Config{Command: "claude", CopyFiles: []CopyEntry{
+		{Src: "/user/config.json", RelDst: "config.json"},
+	}}
+	withSkip := Config{Command: "claude", CopyFiles: []CopyEntry{
+		{Src: "/user/config.json", RelDst: "config.json", SkipFingerprint: true},
+	}}
+	if CoreFingerprint(base) != CoreFingerprint(withSkip) {
+		t.Error("SkipFingerprint must be ignored on config-derived (Probed=false) entries")
+	}
+	// Changing the Src on a config-derived entry must still drive drift,
+	// even if SkipFingerprint is set.
+	edited := Config{Command: "claude", CopyFiles: []CopyEntry{
+		{Src: "/user/config-edited.json", RelDst: "config.json", SkipFingerprint: true},
+	}}
+	if CoreFingerprint(withSkip) == CoreFingerprint(edited) {
+		t.Error("config-derived Src change must drive drift even with SkipFingerprint=true")
+	}
+}
+
+// TestSkipFingerprintStableUnderFilesystemChurn is the regression guard for
+// issue #682: a probed entry whose underlying directory is populated between
+// probes (simulating pre_start staging) must produce a stable CoreFingerprint
+// when marked SkipFingerprint, and a drifting one otherwise.
+func TestSkipFingerprintStableUnderFilesystemChurn(t *testing.T) {
+	workDir := t.TempDir()
+	skillsDir := filepath.Join(workDir, ".claude", "skills")
+	if err := os.MkdirAll(skillsDir, 0o755); err != nil {
+		t.Fatalf("MkdirAll: %v", err)
+	}
+	// Phase 1: empty skills dir — resembles template-resolve BEFORE pre_start
+	// has finished populating the worktree.
+	before := Config{Command: "claude", CopyFiles: []CopyEntry{{
+		Src: skillsDir, RelDst: ".claude/skills",
+		Probed: true, ContentHash: HashPathContent(skillsDir),
+	}}}
+	// Phase 2: pre_start completes and drops a file in the worktree.
+	if err := os.WriteFile(filepath.Join(skillsDir, "new.md"), []byte("content"), 0o644); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+	after := Config{Command: "claude", CopyFiles: []CopyEntry{{
+		Src: skillsDir, RelDst: ".claude/skills",
+		Probed: true, ContentHash: HashPathContent(skillsDir),
+	}}}
+
+	// Without SkipFingerprint the hash MUST drift (this is the bug).
+	beforeH := CoreFingerprint(before)
+	afterH := CoreFingerprint(after)
+	if beforeH == afterH {
+		t.Fatal("test precondition: HashPathContent must observe filesystem change")
+	}
+
+	// With SkipFingerprint the same mutation must produce a stable hash.
+	beforeSkip := before
+	beforeSkip.CopyFiles = slices.Clone(before.CopyFiles)
+	beforeSkip.CopyFiles[0].SkipFingerprint = true
+	afterSkip := after
+	afterSkip.CopyFiles = slices.Clone(after.CopyFiles)
+	afterSkip.CopyFiles[0].SkipFingerprint = true
+
+	if got1, got2 := CoreFingerprint(beforeSkip), CoreFingerprint(afterSkip); got1 != got2 {
+		t.Errorf("SkipFingerprint did not stabilize CopyFiles hash under churn: before=%s after=%s", got1, got2)
+	}
+}
+
+// TestLogCoreFingerprintDriftSkipsExcludedEntries ensures diagnostic output
+// does not leak skipped entries, which would otherwise confuse operators
+// debugging real drift.
+func TestLogCoreFingerprintDriftSkipsExcludedEntries(t *testing.T) {
+	stored := map[string]string{
+		"CopyFiles": "oldhash",
+	}
+	current := Config{
+		Command: "claude",
+		CopyFiles: []CopyEntry{
+			{RelDst: "real", ContentHash: "h1"},
+			{RelDst: "skipped-churn", Probed: true, ContentHash: "h2", SkipFingerprint: true},
+		},
+	}
+	var buf bytes.Buffer
+	LogCoreFingerprintDrift(&buf, "agent", stored, current)
+	out := buf.String()
+	if !bytes.Contains([]byte(out), []byte("real")) {
+		t.Errorf("expected non-skipped RelDst in drift output, got: %s", out)
+	}
+	if bytes.Contains([]byte(out), []byte("skipped-churn")) {
+		t.Errorf("skipped entry leaked into drift output: %s", out)
+	}
+}

internal/runtime/runtime.go

@@ -230,6 +230,16 @@ type CopyEntry struct {
 	// (transient I/O error), the fingerprint uses a stable sentinel rather
 	// than falling back to path-based hashing.
 	ContentHash string
+	// SkipFingerprint excludes this entry from CoreFingerprint entirely.
+	// Set for probed entries whose contents are produced by pre_start
+	// staging (e.g. files inside the agent worktree populated by
+	// worktree-setup.sh). Fingerprinting such entries would conflate
+	// "config changed" with "pre_start not done yet" and cause false
+	// config-drift drains. See issue #682. The entry is still staged to
+	// K8s pods and retained in CopyFiles for container providers — the
+	// entry is excluded from identity hashing. Only meaningful when
+	// Probed is true; config-derived entries must still drive drain.
+	SkipFingerprint bool
 }
 
 // HashPathContent returns a hex-encoded SHA-256 of the content at path.