diff --git a/cmd/gc/bd_env.go b/cmd/gc/bd_env.go index 681ab244eb..8e8c14954b 100644 --- a/cmd/gc/bd_env.go +++ b/cmd/gc/bd_env.go @@ -37,6 +37,7 @@ func bdStoreForCity(dir, cityPath string) *beads.BdStore { if err != nil { cfg = nil } + reapStaleBdExportJSONL(dir) return beads.NewBdStoreWithPrefix(dir, bdCommandRunnerForCity(cityPath), issuePrefixForScope(dir, cityPath, cfg)) } @@ -53,9 +54,84 @@ func bdStoreForRig(rigDir, cityPath string, cfg *config.City, knownPrefix ...str } } } + reapStaleBdExportJSONL(rigDir) return beads.NewBdStoreWithPrefix(rigDir, bdCommandRunnerForRig(cityPath, cfg, rigDir), prefix) } +// reapStaleBdExportJSONL removes .beads/issues.jsonl best-effort when the +// scope is gc-managed. The file is a stale export from when bd's auto-export +// was on (the upstream default); keeping it on disk causes bd 1.x to detect +// a "fresh clone" / "empty database" on the next write and stall bd create / +// gc mail send for the full 2m subprocess timeout while it re-imports the +// JSONL (sa-41j3kp). +// +// Cleanup conditions (any of which proves the scope is gc-managed and the +// JSONL is therefore stale): +// +// - config.yaml explicitly sets export.auto:false (PR 1965 canonical state) +// - config.yaml's gc.endpoint_origin is one of the managed origins +// +// Best-effort: any error is ignored because the env-var BD_EXPORT_AUTO=false +// in bdRuntimeEnv is a second line of defense, and a concurrent reader of the +// file (e.g., a bd-aware viewer) shouldn't fail the caller's operation. Reads +// use os.Stat/os.Remove (not fsys.OSFS) so the helper stays callable from +// store constructors that don't carry an fs seam. +func reapStaleBdExportJSONL(scopeRoot string) { + scopeRoot = strings.TrimSpace(scopeRoot) + if scopeRoot == "" { + return + } + jsonlPath := filepath.Join(scopeRoot, ".beads", "issues.jsonl") + if _, err := os.Stat(jsonlPath); err != nil { + // Fast path: no file → nothing to do. This is the steady state + // once the cleanup has run once, so the rest of the helper is + // only reached during the one-shot transition. + return + } + if !scopeIsGCManaged(scopeRoot) { + // Unmanaged scope: leave the file alone. Removing it under those + // conditions could race with a legitimate auto-exporter (e.g., a + // rig that opted out of managed canonicalization). + return + } + _ = os.Remove(jsonlPath) +} + +// scopeIsGCManaged reports whether a scope's .beads/config.yaml proves the +// scope is gc-managed under the canonical (non-explicit) shape. Either of +// two signals counts as proof: +// - export.auto is explicitly false (PR 1965 wrote it; the user did not +// opt back into auto-export afterward) +// - gc.endpoint_origin is one of the canonical managed origins (the scope +// was initialized by gc, even if the export.auto key has not yet been +// normalized into the config on disk) +// +// Either signal alone is sufficient: the first covers post-normalization +// state, the second covers the transitional state where samtown-style +// long-lived cities still have a pre-PR-1965 config but were always +// gc-managed. +// +// EndpointOriginExplicit is intentionally excluded: per PR 1965, that is +// the deliberate opt-out path for rigs that want to keep JSONL-based +// sharing, so issues.jsonl there is load-bearing, not stale. +func scopeIsGCManaged(scopeRoot string) bool { + configPath := filepath.Join(scopeRoot, ".beads", "config.yaml") + if autoExport, ok, err := contract.ReadExportAuto(fsys.OSFS{}, configPath); err == nil && ok && !autoExport { + return true + } + state, ok, err := contract.ReadConfigState(fsys.OSFS{}, configPath) + if err != nil || !ok { + return false + } + switch state.EndpointOrigin { + case contract.EndpointOriginManagedCity, + contract.EndpointOriginCityCanonical, + contract.EndpointOriginInheritedCity: + return true + } + return false +} + func controlBdStoreForCity(dir, cityPath string, cfg *config.City) *beads.BdStore { return beads.NewBdStoreWithPrefix(dir, controlBdCommandRunnerForCity(cityPath), issuePrefixForScope(dir, cityPath, cfg)) } @@ -548,6 +624,16 @@ func bdRuntimeEnv(cityPath string) map[string]string { // dolt.auto-start:false config (beads resolveAutoStart priority bug) and // starts rogue servers from the agent's cwd with the wrong data_dir. env["BEADS_DOLT_AUTO_START"] = "0" + // Suppress bd's auto-export of issues.jsonl on every write. The canonical + // config also persists export.auto:false (see internal/beads/contract/files.go), + // but the env var is the bulletproof per-invocation guard: it covers fresh + // scopes whose config has not yet been canonicalized, and it short-circuits + // the export → next-write-auto-import stall cycle (sa-41j3kp) even when an + // out-of-band caller has left a stale .beads/issues.jsonl on disk. Without + // this, bd's "auto-importing N bytes ... into empty database" path can + // stall bd create / gc mail send for the full 2m subprocess timeout on + // large datasets. + env["BD_EXPORT_AUTO"] = "false" if !cityUsesBdStoreContract(cityPath) { return env } diff --git a/cmd/gc/bd_env_test.go b/cmd/gc/bd_env_test.go index ba4286568e..1076651ecf 100644 --- a/cmd/gc/bd_env_test.go +++ b/cmd/gc/bd_env_test.go @@ -3064,3 +3064,158 @@ func TestControlBdCommandRunnerDefaultsBeadsActorToControllerWhenUnset(t *testin t.Fatalf("BD_EXPORT_AUTO = %q, want false", got) } } + +// TestBdRuntimeEnvDisablesAutoExport pins the env-var defense against +// bd's auto-export-on-write trap (sa-41j3kp): every gc-initiated bd call +// must set BD_EXPORT_AUTO=false so that even if .beads/config.yaml has not +// yet been canonicalized with export.auto:false (older cities pre-PR-1965, +// fresh inits, rigs that have not reached normalization), bd's auto-export +// of issues.jsonl on every write is suppressed. Without this, the next bd +// write re-creates the 15MB JSONL, and the bd write after that stalls for +// the full 2m subprocess timeout while bd re-imports the file. +func TestBdRuntimeEnvDisablesAutoExport(t *testing.T) { + t.Setenv("GC_BEADS", "bd") + t.Setenv("GC_DOLT", "skip") + + cityPath := t.TempDir() + env := bdRuntimeEnv(cityPath) + + if got := env["BD_EXPORT_AUTO"]; got != "false" { + t.Fatalf("BD_EXPORT_AUTO = %q, want false (auto-export-on-write suppression for sa-41j3kp)", got) + } +} + +// TestScopeIsGCManagedRecognizesExplicitAutoOff verifies that a config with +// export.auto:false is recognized as gc-managed even when gc.endpoint_origin +// is absent. This is the steady-state signal post-PR-1965. +func TestScopeIsGCManagedRecognizesExplicitAutoOff(t *testing.T) { + scope := t.TempDir() + beadsDir := filepath.Join(scope, ".beads") + if err := os.MkdirAll(beadsDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(beadsDir, "config.yaml"), + []byte("issue_prefix: zz\nexport.auto: false\n"), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + if !scopeIsGCManaged(scope) { + t.Fatalf("scopeIsGCManaged = false, want true for explicit export.auto:false") + } +} + +// TestScopeIsGCManagedRecognizesManagedOrigin verifies that a long-lived +// city whose config still pre-dates PR 1965 (export.auto absent) is still +// recognized as gc-managed because gc.endpoint_origin proves it. This is +// the transitional signal — without it the jsonl reaper would refuse to +// clean up samtown-style cities until they hit a canonicalization event. +func TestScopeIsGCManagedRecognizesManagedOrigin(t *testing.T) { + scope := t.TempDir() + beadsDir := filepath.Join(scope, ".beads") + if err := os.MkdirAll(beadsDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(beadsDir, "config.yaml"), + []byte("issue_prefix: zz\ngc.endpoint_origin: managed_city\n"), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + if !scopeIsGCManaged(scope) { + t.Fatalf("scopeIsGCManaged = false, want true for gc.endpoint_origin: managed_city") + } +} + +// TestScopeIsGCManagedDoesNotClaimExplicitOptOut verifies the carve-out +// for rigs that deliberately keep JSONL-based sharing. Per PR 1965 docs, +// gc.endpoint_origin: explicit is the supported opt-out path; issues.jsonl +// there is load-bearing, not stale, so scopeIsGCManaged must return false. +func TestScopeIsGCManagedDoesNotClaimExplicitOptOut(t *testing.T) { + scope := t.TempDir() + beadsDir := filepath.Join(scope, ".beads") + if err := os.MkdirAll(beadsDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(beadsDir, "config.yaml"), + []byte("issue_prefix: zz\ngc.endpoint_origin: explicit\n"), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + if scopeIsGCManaged(scope) { + t.Fatalf("scopeIsGCManaged = true, want false for explicit opt-out") + } +} + +// TestReapStaleBdExportJSONLRemovesFileOnManagedScope verifies the +// transitional cleanup that breaks the sa-41j3kp loop on samtown-style +// long-lived cities. issues.jsonl from a pre-PR-1965 auto-export is +// removed on the next bd-store construction, so the next bd create does +// not stall on auto-import. +func TestReapStaleBdExportJSONLRemovesFileOnManagedScope(t *testing.T) { + scope := t.TempDir() + beadsDir := filepath.Join(scope, ".beads") + if err := os.MkdirAll(beadsDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + if err := os.WriteFile(jsonlPath, []byte(`{"_type":"issue","id":"zz-1"}`+"\n"), 0o644); err != nil { + t.Fatalf("write jsonl: %v", err) + } + if err := os.WriteFile(filepath.Join(beadsDir, "config.yaml"), + []byte("issue_prefix: zz\ngc.endpoint_origin: managed_city\n"), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + reapStaleBdExportJSONL(scope) + + if _, err := os.Stat(jsonlPath); !os.IsNotExist(err) { + t.Fatalf("jsonl present after reap; stat err = %v, want IsNotExist", err) + } +} + +// TestReapStaleBdExportJSONLLeavesFileOnExplicitOptOut verifies the +// opt-out path: rigs with gc.endpoint_origin: explicit keep issues.jsonl +// because they're deliberately using JSONL-based sharing. +func TestReapStaleBdExportJSONLLeavesFileOnExplicitOptOut(t *testing.T) { + scope := t.TempDir() + beadsDir := filepath.Join(scope, ".beads") + if err := os.MkdirAll(beadsDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + if err := os.WriteFile(jsonlPath, []byte(`{"_type":"issue","id":"zz-1"}`+"\n"), 0o644); err != nil { + t.Fatalf("write jsonl: %v", err) + } + if err := os.WriteFile(filepath.Join(beadsDir, "config.yaml"), + []byte("issue_prefix: zz\ngc.endpoint_origin: explicit\n"), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + reapStaleBdExportJSONL(scope) + + if _, err := os.Stat(jsonlPath); err != nil { + t.Fatalf("jsonl removed on explicit opt-out; stat err = %v, want nil", err) + } +} + +// TestReapStaleBdExportJSONLLeavesFileOnUnmanagedScope verifies that an +// unmanaged scope (no config.yaml, or config without any gc/managed signal) +// keeps issues.jsonl intact. The reaper must not be aggressive on scopes +// that gc does not own. +func TestReapStaleBdExportJSONLLeavesFileOnUnmanagedScope(t *testing.T) { + scope := t.TempDir() + beadsDir := filepath.Join(scope, ".beads") + if err := os.MkdirAll(beadsDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + if err := os.WriteFile(jsonlPath, []byte(`{"_type":"issue","id":"zz-1"}`+"\n"), 0o644); err != nil { + t.Fatalf("write jsonl: %v", err) + } + // No config.yaml at all — definitely not gc-managed. + + reapStaleBdExportJSONL(scope) + + if _, err := os.Stat(jsonlPath); err != nil { + t.Fatalf("jsonl removed on unmanaged scope; stat err = %v, want nil", err) + } +} diff --git a/cmd/gc/beads_provider_lifecycle.go b/cmd/gc/beads_provider_lifecycle.go index 519c9fe068..13db5fadfa 100644 --- a/cmd/gc/beads_provider_lifecycle.go +++ b/cmd/gc/beads_provider_lifecycle.go @@ -325,8 +325,36 @@ func ensureCanonicalScopeConfigState(fs fsys.FS, dir string, state contract.Conf if err := ensureBeadsDir(fs, beadsDir); err != nil { return err } - _, err := contract.EnsureCanonicalConfig(fs, filepath.Join(beadsDir, "config.yaml"), state) - return err + changed, err := contract.EnsureCanonicalConfig(fs, filepath.Join(beadsDir, "config.yaml"), state) + if err != nil { + return err + } + if changed { + // PR 1965 made export.auto:false canonical, but a pre-existing + // .beads/issues.jsonl from before this normalization still triggers + // bd's auto-import-on-write trap (sa-41j3kp) — bd sees the file, + // detects a "stale DB", and stalls bd create for the full 2m + // subprocess timeout while it re-imports the JSONL. The file is a + // stale export from when auto-export was on; with the canonical + // config now suppressing auto-export, nothing will refresh it, so + // keeping it on disk is pure liability. Remove best-effort. + removeStaleBdExportJSONL(fs, beadsDir) + } + return nil +} + +// removeStaleBdExportJSONL removes .beads/issues.jsonl if present. Called after +// EnsureCanonicalConfig writes export.auto:false, since the file is a stale +// export that bd's auto-import path would otherwise re-load on every write, +// stalling bd create for the full subprocess timeout on large datasets. +// Best-effort: any error is non-fatal because the env-var BD_EXPORT_AUTO=false +// path (bdRuntimeEnv) is a second line of defense for gc-initiated calls. +func removeStaleBdExportJSONL(fs fsys.FS, beadsDir string) { + path := filepath.Join(beadsDir, "issues.jsonl") + if _, err := fs.Stat(path); err != nil { + return + } + _ = fs.Remove(path) } func seedDeferredManagedBeads(cityPath, dir, prefix, doltDatabase string) { diff --git a/internal/beads/contract/files.go b/internal/beads/contract/files.go index 4bd1efc780..329f1f75ca 100644 --- a/internal/beads/contract/files.go +++ b/internal/beads/contract/files.go @@ -118,6 +118,30 @@ func ReadAutoStartDisabled(fs fsys.FS, path string) (bool, error) { return false, nil } +// ReadExportAuto returns the configured value of export.auto along with a +// presence indicator. ok=false means the key is absent from the config (the +// upstream bd default is true). Used by gc to gate cleanup of stale +// .beads/issues.jsonl exports: when export.auto is explicitly false, the +// JSONL is a stale artifact that bd's auto-import-on-write path (sa-41j3kp) +// would otherwise reload on every write, stalling bd create for minutes on +// large datasets. +func ReadExportAuto(fs fsys.FS, path string) (value bool, ok bool, err error) { + doc, err := readConfigDoc(fs, path) + if err != nil { + if os.IsNotExist(err) { + return false, false, nil + } + if raw, scanOK := scanConfigLineValue(fs, path, "export.auto:"); scanOK { + return raw == "true", true, nil + } + return false, false, err + } + if raw, present := configStringValue(mappingRoot(doc), "export.auto"); present { + return raw == "true", true, nil + } + return false, false, nil +} + // ReadEndpointStatus reads gc.endpoint_status when present. func ReadEndpointStatus(fs fsys.FS, path string) (EndpointStatus, bool, error) { doc, err := readConfigDoc(fs, path) diff --git a/internal/beads/contract/files_test.go b/internal/beads/contract/files_test.go index 6df3fe4a36..4b751ef1cd 100644 --- a/internal/beads/contract/files_test.go +++ b/internal/beads/contract/files_test.go @@ -571,6 +571,73 @@ func TestReadAutoStartDisabledFallsBackToLineScanOnMalformedYAML(t *testing.T) { } } +func TestReadExportAuto(t *testing.T) { + tests := []struct { + name string + yaml string + wantValue bool + wantOK bool + }{ + { + name: "explicit false", + yaml: "issue_prefix: zz\nexport.auto: false\n", + wantValue: false, + wantOK: true, + }, + { + name: "explicit true", + yaml: "issue_prefix: zz\nexport.auto: true\n", + wantValue: true, + wantOK: true, + }, + { + name: "absent", + yaml: "issue_prefix: zz\n", + wantValue: false, + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs := fsys.OSFS{} + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + if err := fs.WriteFile(path, []byte(tt.yaml), 0o644); err != nil { + t.Fatal(err) + } + + gotValue, gotOK, err := ReadExportAuto(fs, path) + if err != nil { + t.Fatalf("ReadExportAuto() error = %v", err) + } + if gotOK != tt.wantOK { + t.Errorf("ReadExportAuto() ok = %v, want %v", gotOK, tt.wantOK) + } + if gotValue != tt.wantValue { + t.Errorf("ReadExportAuto() value = %v, want %v", gotValue, tt.wantValue) + } + }) + } +} + +func TestReadExportAutoOnMissingFileReturnsAbsent(t *testing.T) { + fs := fsys.OSFS{} + dir := t.TempDir() + path := filepath.Join(dir, "missing.yaml") + + gotValue, gotOK, err := ReadExportAuto(fs, path) + if err != nil { + t.Fatalf("ReadExportAuto() error = %v, want nil for missing file", err) + } + if gotOK { + t.Errorf("ReadExportAuto() ok = true, want false for missing file") + } + if gotValue { + t.Errorf("ReadExportAuto() value = true, want false for missing file") + } +} + func TestEnsureCanonicalMetadataPreservesUnknownKeysAndScrubsDeprecatedOnes(t *testing.T) { fs := fsys.OSFS{} dir := t.TempDir()