fix: rebind selected workspace before db command startup#3230
Conversation
|
Tracking down some gnarly rogue dbs in gascity and I think this may be one of the culprits. |
|
Reviewed — the rebinding fix looks good and the Linux CI is solid. One blocker: Sibling test // cmd/bd/doctor_context_test.go:202
if got := selectedDoltBeadsDir(); utils.CanonicalizePath(got) != utils.CanonicalizePath(targetBeadsDir) {
t.Fatalf("selectedDoltBeadsDir() = %q, want %q", got, targetBeadsDir)
}
The |
b662a61 to
9a16b2d
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Addresses review follow-ups on this PR. Four changes in one commit: 1. cmd/bd/where.go: findOriginalBeadsDir's BEADS_DIR fast-path used to return "" unconditionally when BEADS_DIR was set but the env'd directory had no redirect file. Since gastownhall#3230 rebinds BEADS_DIR to the post-redirect target during command startup, that shortcut silently hid the redirect source from `bd where` output — `bd where` inside a redirecting worktree rendered without its "(via redirect from ...)" footnote. Fall through to the filesystem walk instead; keep the fast-path's positive branch so a user-set BEADS_DIR pointing at a redirecting directory is still returned directly. Regression tests in cmd/bd/where_test.go cover both directions: BEADS_DIR set + no redirect → walk finds the redirect source; BEADS_DIR set + has redirect → env value returned directly. 2. internal/beads/beads_inherited_artifacts_test.go: close the codecov gap in hasBeadsDatabase by exercising the dolt/ and embeddeddolt/ directory branches (previously only *.db was covered). Add a table-driven runWorktreeSeparateDBPreservedTest helper so the three variants share setup. 3. internal/beads/beads_inherited_artifacts_test.go: new test TestFindBeadsDir_WorktreeNoDatabaseAnywhereFallsBackToLocal exercises the lenient-escape-hatch path in FindBeadsDir step 3b: when neither the worktree nor the shared fallback has a database, return the worktree-local .beads/ so a fresh `bd init` can bootstrap. 4. internal/beads/beads.go findDatabaseInTree: apply the same CanonicalizePath treatment as FindBeadsDir so the gitRoot boundary comparison is robust against /var → /private/var and case-insensitive-filesystem mismatches. Not a bug fix (this path requires an actual *.db to match, so inherited metadata can't masquerade as a real project), but consistency with the sibling resolver closes a latent pitfall.
…#3230) * fix: rebind target workspace for db commands * fix: isolate rebinding from caller workspace state * fix: restore no-db target selection semantics * fix: honor no-db target precedence consistently * test: restore db flag state in context rebinding tests
…3425) * fix(worktree): don't prefer inherited .beads/ over the shared fallback `bd worktree create --help` promises that worktrees "automatically share the same beads database as the main repository via git common directory discovery — no manual redirect configuration needed." That promise is broken when the parent repo commits its `.beads/` artifacts (metadata.json, config.yaml, issues.jsonl, etc.) — a common setup because these files are the portable/durable record of the beads project. `git worktree add` checks out those tracked files into the worktree's `.beads/`, but the gitignored `.beads/dolt/` data directory is not checked out. `FindBeadsDir` sees the worktree-local `.beads/` with its inherited metadata.json and returns it, instead of falling back to the shared `.beads/` with the actual database. bd then tries to auto-start a Dolt server against an empty data directory, which fails with: failed to open database: database "<proj>" not found on Dolt server at 127.0.0.1:<port> ...and every subsequent `bd` invocation from inside the worktree errors. Two root causes, both fixed here: 1. `FindBeadsDir` step 2 walks up from CWD with a boundary check at `git.GetRepoRoot()`. On macOS, `os.Getwd()` returns unresolved `/var/folders/...` while `git rev-parse --show-toplevel` returns the canonical `/private/var/folders/...`. The `dir == walkBoundary` comparison silently never matches; the walk overshoots the worktree root, finds the inherited `.beads/` at the worktree root, and returns it — never reaching the worktree-fallback logic in step 3. Fix: canonicalize both `cwd` and `walkBoundary` (and the step 4 `extendedRoot`) before comparison. 2. `FindBeadsDir` step 3b accepted the worktree-local `.beads/` as separate-DB if `hasBeadsProjectFiles` returned true. That check matches on metadata.json alone, so tracked-artifact dirs pass. The shared-DB fallback (3c) was never reached. Fix: add `hasBeadsDatabase` (strict — requires dolt/, embeddeddolt/, or *.db). Step 3b now requires a real database for the separate-DB verdict. If the worktree-local .beads/ lacks a database but the shared fallback has one, fall through to 3c. The lenient check is preserved when no fallback exists with a real database (so `bd init` in a fresh worktree can still bootstrap). Tests: two new regression tests under `internal/beads/` — TestFindBeadsDir_WorktreeWithInheritedArtifacts reproduces the bug (fails on unpatched main) and TestFindBeadsDir_WorktreeSeparateDBPreservesLocal guards against regressing genuine separate-DB mode. * fix(where): fall through BEADS_DIR shortcut when env has no redirect Addresses review follow-ups on this PR. Four changes in one commit: 1. cmd/bd/where.go: findOriginalBeadsDir's BEADS_DIR fast-path used to return "" unconditionally when BEADS_DIR was set but the env'd directory had no redirect file. Since #3230 rebinds BEADS_DIR to the post-redirect target during command startup, that shortcut silently hid the redirect source from `bd where` output — `bd where` inside a redirecting worktree rendered without its "(via redirect from ...)" footnote. Fall through to the filesystem walk instead; keep the fast-path's positive branch so a user-set BEADS_DIR pointing at a redirecting directory is still returned directly. Regression tests in cmd/bd/where_test.go cover both directions: BEADS_DIR set + no redirect → walk finds the redirect source; BEADS_DIR set + has redirect → env value returned directly. 2. internal/beads/beads_inherited_artifacts_test.go: close the codecov gap in hasBeadsDatabase by exercising the dolt/ and embeddeddolt/ directory branches (previously only *.db was covered). Add a table-driven runWorktreeSeparateDBPreservedTest helper so the three variants share setup. 3. internal/beads/beads_inherited_artifacts_test.go: new test TestFindBeadsDir_WorktreeNoDatabaseAnywhereFallsBackToLocal exercises the lenient-escape-hatch path in FindBeadsDir step 3b: when neither the worktree nor the shared fallback has a database, return the worktree-local .beads/ so a fresh `bd init` can bootstrap. 4. internal/beads/beads.go findDatabaseInTree: apply the same CanonicalizePath treatment as FindBeadsDir so the gitRoot boundary comparison is robust against /var → /private/var and case-insensitive-filesystem mismatches. Not a bug fix (this path requires an actual *.db to match, so inherited metadata can't masquerade as a real project), but consistency with the sibling resolver closes a latent pitfall.
Summary
--dband selector env precedenceBEADS_DIRconfig isolationRoot cause
bdresolved the target workspace for DB commands too late. Global config and ambient caller state could leak into commands targeting another workspace, which let dolt server mode, auto-start settings, and config values come from the caller repo instead of the selected repo.Testing
go test ./cmd/bd -count=1make testgolangci-lint run ./cmd/bd/... ./internal/config/...Notes