Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions cmd/gc/cmd_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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,
})
}
}
Expand All @@ -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).
Expand Down
16 changes: 13 additions & 3 deletions cmd/gc/cmd_start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down
9 changes: 8 additions & 1 deletion cmd/gc/config_hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions cmd/gc/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion cmd/gc/template_resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
118 changes: 116 additions & 2 deletions cmd/gc/template_resolve_workdir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
}

Expand Down
12 changes: 12 additions & 0 deletions internal/runtime/fingerprint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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
}
}
Expand Down
Loading
Loading