Merge pull request #702 from gastownhall/hotfix/v0.14.0-copyfiles-fingerprint

hotfix: backport #687 and #695 to v0.14.0

Author: julianknutsen

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

cmd/gc/providers.go

@@ -245,8 +245,17 @@ func displayProviderName(name string) string {
 // Priority: GC_BEADS env var → city.toml [beads].provider → "bd" default.
 // This is the unmodified config value; use beadsProvider() for lifecycle
 // routing which remaps "bd" → exec:.
+//
+// If the ambient GC_BEADS points at the city-managed gc-beads-bd lifecycle
+// wrapper, we normalize it back to "bd". The wrapper exits 2 for data ops
+// (see #647), so inheriting it from a contaminated parent would reintroduce
+// the crash in nested agent sessions. Genuine user exec: overrides are
+// preserved — only the well-known lifecycle wrapper path is stripped.
 func rawBeadsProvider(cityPath string) string {
 	if v := os.Getenv("GC_BEADS"); v != "" {
+		if isLifecycleWrapperPath(v) {
+			return "bd"
+		}
 		return v
 	}
 	// Try to read provider from city.toml.
@@ -257,10 +266,26 @@ func rawBeadsProvider(cityPath string) string {
 	return "bd"
 }
 
+// isLifecycleWrapperPath reports whether v is the city-managed gc-beads-bd
+// lifecycle wrapper (i.e., `exec:<anything>/.gc/system/bin/gc-beads-bd`).
+func isLifecycleWrapperPath(v string) bool {
+	if !strings.HasPrefix(v, "exec:") {
+		return false
+	}
+	return strings.HasSuffix(v, string(filepath.Separator)+citylayout.SystemBinRoot+string(filepath.Separator)+"gc-beads-bd")
+}
+
 // beadsProvider returns the bead store provider name for lifecycle operations.
 // Maps "bd" → "exec:<cityPath>/.gc/system/bin/gc-beads-bd" so all lifecycle operations
 // route through the exec: protocol. Other providers pass through unchanged.
 //
+// This is for lifecycle operations ONLY (start/stop/health/ensure-ready/init).
+// gc-beads-bd exits 2 for data operations (get/list/create/update/close); it
+// is not a full exec-beads protocol implementation. Data-path callers — in
+// particular agent-session environments (see template_resolve.go) — must use
+// rawBeadsProvider() so they route through BdStore directly. See #647 for the
+// crash that surfaced when this invariant was violated.
+//
 // Related env vars:
 //   - GC_DOLT=skip — the gc-beads-bd script checks this and exits 2 for all
 //     operations. Used by testscript and integration tests.

cmd/gc/template_resolve.go

@@ -212,7 +212,9 @@ func resolveTemplate(p *agentBuildParams, cfgAgent *config.Agent, qualifiedName
 	for key, value := range citylayout.CityRuntimeEnvMap(p.cityPath) {
 		agentEnv[key] = value
 	}
-	agentEnv["GC_BEADS"] = beadsProvider(p.cityPath)
+	// Agent-session data ops must bypass the lifecycle wrapper. See
+	// beadsProvider() docs and #647.
+	agentEnv["GC_BEADS"] = rawBeadsProvider(p.cityPath)
 	if exe, err := os.Executable(); err == nil && exe != "" {
 		agentEnv["GC_BIN"] = exe
 	}

cmd/gc/template_resolve_workdir_test.go

@@ -214,6 +214,8 @@ func TestResolveTemplateRigScopedEnvCarriesRigRoots(t *testing.T) {
 }
 
 func TestResolveTemplateUsesCityManagedDoltPort(t *testing.T) {
+	// Hermetic: ambient GC_BEADS would poison the "bd" assertion below.
+	t.Setenv("GC_BEADS", "")
 	cityPath := t.TempDir()
 	writeTemplateResolveCityConfig(t, cityPath, "")
 	stateDir := filepath.Join(cityPath, ".gc", "runtime", "packs", "dolt")
@@ -269,8 +271,120 @@ func TestResolveTemplateUsesCityManagedDoltPort(t *testing.T) {
 	if got := tp.Env["GC_BIN"]; got == "" {
 		t.Fatalf("GC_BIN = %q, want non-empty", got)
 	}
-	if got := tp.Env["GC_BEADS"]; got != "exec:"+filepath.Join(cityPath, ".gc", "system", "bin", "gc-beads-bd") {
-		t.Fatalf("GC_BEADS = %q, want exec gc-beads-bd provider", got)
+	// Agent sessions route data ops to raw bd, not the lifecycle wrapper.
+	// See #647.
+	if got := tp.Env["GC_BEADS"]; got != "bd" {
+		t.Fatalf("GC_BEADS = %q, want %q", got, "bd")
+	}
+}
+
+// Regression for #647: agent-session data ops must not route through the
+// lifecycle-only gc-beads-bd wrapper.
+func TestResolveTemplateRoutesAgentSessionDataOpsToRawBd(t *testing.T) {
+	cases := []struct {
+		name     string
+		provider string
+		want     string
+	}{
+		{name: "default bd", provider: "", want: "bd"},
+		{name: "explicit bd", provider: "bd", want: "bd"},
+		{name: "file passthrough", provider: "file", want: "file"},
+		{name: "custom exec passthrough", provider: "exec:/custom/gc-beads-br", want: "exec:/custom/gc-beads-br"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Hermetic: the city.toml value is authoritative for this test,
+			// regardless of what the developer has exported in their shell.
+			t.Setenv("GC_BEADS", "")
+			cityPath := t.TempDir()
+			writeTemplateResolveCityConfig(t, cityPath, tc.provider)
+
+			params := &agentBuildParams{
+				cityName:   "city",
+				cityPath:   cityPath,
+				workspace:  &config.Workspace{Provider: "test"},
+				providers:  map[string]config.ProviderSpec{"test": {Command: "echo", PromptMode: "none"}},
+				lookPath:   func(string) (string, error) { return "/bin/echo", nil },
+				fs:         fsys.OSFS{},
+				beaconTime: time.Unix(0, 0),
+				beadNames:  make(map[string]string),
+				stderr:     io.Discard,
+			}
+
+			agent := &config.Agent{Name: "worker"}
+			tp, err := resolveTemplate(params, agent, agent.QualifiedName(), nil)
+			if err != nil {
+				t.Fatalf("resolveTemplate: %v", err)
+			}
+
+			if got := tp.Env["GC_BEADS"]; got != tc.want {
+				t.Fatalf("GC_BEADS = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+// Regression for #647: if a parent process leaked GC_BEADS pointing at the
+// city-managed lifecycle wrapper, nested agent sessions would re-inherit it
+// and recreate the exit-2/empty-JSON crash on data ops. The raw provider
+// normalizes that well-known wrapper path back to "bd".
+func TestResolveTemplateRawBeadsProviderStripsLifecycleWrapper(t *testing.T) {
+	cityPath := t.TempDir()
+	writeTemplateResolveCityConfig(t, cityPath, "")
+	contaminated := "exec:" + filepath.Join(cityPath, ".gc", "system", "bin", "gc-beads-bd")
+	t.Setenv("GC_BEADS", contaminated)
+
+	params := &agentBuildParams{
+		cityName:   "city",
+		cityPath:   cityPath,
+		workspace:  &config.Workspace{Provider: "test"},
+		providers:  map[string]config.ProviderSpec{"test": {Command: "echo", PromptMode: "none"}},
+		lookPath:   func(string) (string, error) { return "/bin/echo", nil },
+		fs:         fsys.OSFS{},
+		beaconTime: time.Unix(0, 0),
+		beadNames:  make(map[string]string),
+		stderr:     io.Discard,
+	}
+
+	agent := &config.Agent{Name: "worker"}
+	tp, err := resolveTemplate(params, agent, agent.QualifiedName(), nil)
+	if err != nil {
+		t.Fatalf("resolveTemplate: %v", err)
+	}
+
+	if got := tp.Env["GC_BEADS"]; got != "bd" {
+		t.Fatalf("GC_BEADS = %q, want %q (ambient wrapper must be normalized)", got, "bd")
+	}
+}
+
+// Genuine user exec: overrides must pass through untouched — only the
+// well-known lifecycle wrapper path is stripped.
+func TestResolveTemplateRawBeadsProviderPreservesCustomExec(t *testing.T) {
+	cityPath := t.TempDir()
+	writeTemplateResolveCityConfig(t, cityPath, "")
+	custom := "exec:/usr/local/bin/my-beads-backend"
+	t.Setenv("GC_BEADS", custom)
+
+	params := &agentBuildParams{
+		cityName:   "city",
+		cityPath:   cityPath,
+		workspace:  &config.Workspace{Provider: "test"},
+		providers:  map[string]config.ProviderSpec{"test": {Command: "echo", PromptMode: "none"}},
+		lookPath:   func(string) (string, error) { return "/bin/echo", nil },
+		fs:         fsys.OSFS{},
+		beaconTime: time.Unix(0, 0),
+		beadNames:  make(map[string]string),
+		stderr:     io.Discard,
+	}
+
+	agent := &config.Agent{Name: "worker"}
+	tp, err := resolveTemplate(params, agent, agent.QualifiedName(), nil)
+	if err != nil {
+		t.Fatalf("resolveTemplate: %v", err)
+	}
+
+	if got := tp.Env["GC_BEADS"]; got != custom {
+		t.Fatalf("GC_BEADS = %q, want %q (custom exec: must pass through)", got, custom)
 	}
 }
 

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
 			}
 		}