Skip to content

fix: rebind selected workspace before db command startup#3230

Merged
julianknutsen merged 5 commits into
mainfrom
fix/rebind-command-context
Apr 16, 2026
Merged

fix: rebind selected workspace before db command startup#3230
julianknutsen merged 5 commits into
mainfrom
fix/rebind-command-context

Conversation

@julianknutsen

Copy link
Copy Markdown
Collaborator

Summary

  • rebind db-targeted commands to the selected workspace before migration, store open, and dolt/server setup
  • keep no-store commands consistent with explicit --db and selector env precedence
  • add regression coverage for target config/env rebinding, no-db command behavior, dolt target selection, init guard mode detection, and external BEADS_DIR config isolation

Root cause

bd resolved 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=1
  • make test
  • golangci-lint run ./cmd/bd/... ./internal/config/...

Notes

  • I ran this through repeated review/fix passes locally and addressed the blocker/major findings that surfaced during review.

@julianknutsen

Copy link
Copy Markdown
Collaborator Author

Tracking down some gnarly rogue dbs in gascity and I think this may be one of the culprits.

@maphew

maphew commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator

Reviewed — the rebinding fix looks good and the Linux CI is solid. One blocker: Test (macos-latest) fails on TestSelectedDoltBeadsDirUsesReboundBEADSDir because macOS t.TempDir() returns a /var/... path that's a symlink to /private/var/.... selectedDoltBeadsDir()FindBeadsDirutils.CanonicalizePath resolves the symlink, so the string equality fails only on macOS.

Sibling test TestLoadSelectionEnvironmentUsesAmbientEnvFileForBEADSDB in the same file (around L177) already handles this by wrapping both sides in utils.CanonicalizePath. Same fix here:

// cmd/bd/doctor_context_test.go:202
if got := selectedDoltBeadsDir(); utils.CanonicalizePath(got) != utils.CanonicalizePath(targetBeadsDir) {
    t.Fatalf("selectedDoltBeadsDir() = %q, want %q", got, targetBeadsDir)
}

utils is already imported. Since maintainer edits aren't enabled on this branch, could you push the one-liner? Happy to merge once macOS CI is green.

The internal/config/config.go addition is minimal and the main.go refactor has proportionate test coverage — the only real concern is the test portability fix above.

@julianknutsen julianknutsen force-pushed the fix/rebind-command-context branch from b662a61 to 9a16b2d Compare April 16, 2026 23:09
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.03297% with 30 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
cmd/bd/main.go 64.63% 21 Missing and 8 partials ⚠️
cmd/bd/context_cmd.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@julianknutsen julianknutsen merged commit 3c67e86 into main Apr 16, 2026
38 checks passed
@maphew maphew deleted the fix/rebind-command-context branch April 17, 2026 21:36
mrmaxsteel added a commit to mrmaxsteel/beads that referenced this pull request Apr 22, 2026
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.
mzlee pushed a commit to mzlee/beads that referenced this pull request Apr 22, 2026
…#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
maphew pushed a commit that referenced this pull request Apr 24, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants