Skip to content

fix: rebind selected workspace before db command startup#3230

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

fix: rebind selected workspace before db command startup#3230
julianknutsen merged 5 commits intomainfrom
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
Copy link
Copy Markdown
Collaborator

maphew commented Apr 13, 2026

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