fix: cross-rig prefix routing for issue resolution via routes.jsonl

[outdoorsea]

Summary

Three fixes for bd doctor achieving zero-warning state and cross-rig resolution:

1. Cross-rig prefix routing — When bd runs from a redirected .beads directory, issue IDs with prefixes mapped to other rigs via routes.jsonl were not resolved. Adds resolveViaPrefixRouting to the resolution chain: local store → prefix routing (NEW) → contributor auto-routing.

2. Dolt Format auto-fix — bd doctor --fix had no automatic fix for the "missing .bd-dolt-ok marker" warning. Adds a DoltFormat fix that seeds the marker file.

3. bd vc commit includes config table — bd vc commit called Commit() which skips the config table (GH#2455). An explicit user commit should include everything visible in bd vc status. Switches to CommitPending which includes config.

Test plan

• bd show hr-8wn.1 resolves correctly from redirected context
• bd show hq-i91 still resolves locally (no regression)
• BD_DEBUG_ROUTING=1 shows prefix routing path
• bd doctor --fix --yes now fixes Dolt Format warning
• bd config set + bd vc commit clears config from dolt_status
• bd doctor returns 0 warnings after all fixes
• Existing tests pass
• Builds cleanly with both CGO_ENABLED=0 and CGO_ENABLED=1

🤖 Generated with Claude Code

[outdoorsea]

All 31 CI checks are now green. The last commit fixes the TestEmbeddedVC/commit_json failure — CommitPending correctly returns committed: false when bd create already auto-committed, so the test now accepts both outcomes (matching the pattern in commit_with_message).

This PR is the missing piece for cross-rig convoy tracking in gastown. The prefix routing it adds to resolveAndGetIssueWithRouting means bd dep add hq-cv-X hr-8wn.1 will:

1. Resolve hq-cv-X locally (HQ database)
2. Resolve hr-8wn.1 via prefix routing → herald rig database
3. DoltStore.AddDependency already handles IsCrossPrefix (skips target existence validation)

Without this, all convoy tracking writes silently fail for cross-rig deps — bd dep add returns exit 0 but doesn't persist. This caused 5 herald convoys to fire "Convoy landed" immediately with 0 tracked issues.

Ready for review and merge.

[nl0]

This fixes #2967. The resolveViaPrefixRouting function restores the routes.jsonl-based cross-rig lookup that was lost in merge commit 73c66ad3 — local → prefix routing → contributor auto-routing is the correct resolution chain.

Verified that resolveAndGetIssueWithRouting and getIssueWithRouting both include the new step, which covers all affected commands (bd show, bd close, bd update, bd dep, etc.).

[hilmes]

Review: fix: cross-rig prefix routing for issue resolution via routes.jsonl

Verdict: Three distinct fixes bundled together. The prefix routing is the significant one — functional but has a concurrency hazard. Dolt Format fix and vc commit fix are clean.

5 files, +229 −21. 5 commits. Three independent changes: cross-rig prefix routing (routed.go), Dolt Format auto-fix (dolt_format.go, doctor_fix.go), and vc commit config inclusion (vc.go, vc_embedded_test.go).

---

Fix 1: Cross-rig prefix routing via routes.jsonl

What it does: Adds a new resolution layer between local store lookup and contributor auto-routing. When a bead ID like hr-8wn.1 isn't found locally, extracts the prefix (hr-), looks it up in routes.jsonl, opens the target rig's database, and resolves there.

Architecture:

• extractBeadPrefix → splits on first hyphen (correct for bead ID format)
• loadPrefixRoutes → reads JSONL with comment/blank-line support
• resolveViaPrefixRouting → finds matching route → derives town root → follows redirect → reads target metadata → opens read-only store against target database

Correctness: Functional with caveats.

The resolution chain (local → prefix → auto-routing) is wired into both resolveAndGetIssueWithRouting and getIssueWithRouting, maintaining symmetry. The isNotFoundErr guard ensures prefix routing only fires on genuine not-found errors. The . path skip prevents infinite loops when a prefix routes to the current database.

🔴 Environment variable mutation is not concurrency-safe

origDB := os.Getenv("BEADS_DOLT_SERVER_DATABASE")
_ = os.Setenv("BEADS_DOLT_SERVER_DATABASE", targetDB)
targetStore, err := newReadOnlyStoreFromConfig(ctx, targetBeadsDir)
if origDB != "" {
    _ = os.Setenv("BEADS_DOLT_SERVER_DATABASE", origDB)
} else {
    _ = os.Unsetenv("BEADS_DOLT_SERVER_DATABASE")
}

This is a process-global mutation. If any concurrent goroutine calls GetDoltDatabase() during this window, it reads the wrong database name. In the CLI's current single-threaded command flow this may be safe today, but it's a latent bug waiting for any parallelism (background sync, concurrent commands, test parallelism). The env var hack works around configfile.GetDoltDatabase() reading the env — a cleaner approach would be to pass the database name through to the store constructor directly, but that would require a larger refactor.

Recommendation: At minimum, add a comment warning that this is not goroutine-safe. Ideally, add a WithDatabase(name string) option to newReadOnlyStoreFromConfig or the dolt Config struct to avoid env mutation entirely.

🟡 No tests for prefix routing

The prefix routing is the most complex new code path (~100 lines of logic including file I/O, path resolution, store lifecycle management, env mutation) and has zero test coverage. extractBeadPrefix and loadPrefixRoutes are both easily unit-testable. The integration path is harder but table-driven tests with a mock routes.jsonl would catch regressions.

🟡 loadPrefixRoutes silently swallows malformed lines

if err := json.Unmarshal([]byte(line), &route); err != nil {
    continue
}

Invalid JSON lines are silently skipped. This is defensible for robustness, but a debug log (gated on BD_DEBUG_ROUTING or debug.Logf) would help diagnose misconfigured routes.jsonl files.

🟡 townRoot derivation assumes specific directory structure

townRoot := filepath.Dir(currentBeadsDir)

This assumes currentBeadsDir is always <town_root>/.beads. If the beads directory is elsewhere (symlinked, or BEADS_DIR pointing to a non-standard location), townRoot will be wrong and rigDir will resolve incorrectly. The code should document this assumption.

Minor: Prefix collision

extractBeadPrefix takes the first hyphen: hr-8wn.1 → hr-. If two rigs have prefixes h- and hr-, the ID hr-8wn.1 matches hr- (first hyphen at index 2), which is correct. But h-r-8wn.1 would match h-, not hr-. This is fine as long as prefixes are always the text before the first hyphen — just worth documenting that routes.jsonl prefixes must match this convention.

---

Fix 2: Dolt Format auto-fix ✅

Clean and minimal. The DoltFormat function:

1. Follows redirects to find the actual beads dir
2. Checks IsPreV56DoltDir (guards against overwriting in non-applicable cases)
3. Writes the .bd-dolt-ok marker file

The guard is correct — IsPreV56DoltDir returns true only when .dolt/ exists but .bd-dolt-ok doesn't. The fix is idempotent. File permissions (0600) are appropriate.

The doctor_fix.go wiring is a two-line case statement — no risk.

---

Fix 3: bd vc commit includes config table

Correct behavioral change. Commit() was designed to skip the config table during auto-commits (GH#2455) to prevent sweeping stale issue_prefix changes. But an explicit bd vc commit is a user-initiated action — the user ran bd vc status, saw pending config changes, and expects bd vc commit to commit them.

The switch from Commit() to CommitPending():

• Uses CommitWithConfig internally (includes all tables)
• Returns (bool, error) — handles "nothing to commit" gracefully
• Generates its own descriptive commit message via buildBatchCommitMessage

🟡 User's -m message is silently dropped

The old code passed vcCommitMessage to Commit(). The new code calls CommitPending() which calls buildBatchCommitMessage() internally. The user's bd vc commit -m "my message" flag is ignored for the actual Dolt commit message. The PR comment acknowledges this ("Note: CommitPending generates its own descriptive commit message rather than using vcCommitMessage"), but this is a behavioral regression for users who relied on custom commit messages. Worth a follow-up to thread the message through CommitPending, or at minimum document the change.

Test update is correct. The commit_json test now accepts both committed=true and committed=false since bd create auto-commits and CommitPending correctly reports "nothing to commit" when there's nothing pending.

---

Summary

Fix	Verdict	Key Concern
Prefix routing	🟡 Functional, ship with caveats	Env var mutation not goroutine-safe; no tests
Dolt Format	✅ Clean	None
vc commit config	🟡 Correct intent, subtle regression	User's -m message silently dropped

Structural note: These three fixes are independent. Consider splitting into separate PRs for cleaner git history and easier revert if any one fix causes issues.

[steveyegge]

Putting this on hold for an architecture discussion. Cross-rig routing was deliberately removed in d762920, and reintroducing it (even at narrower scope) needs consensus before merging.

Specific concerns:

• routes.jsonl has no schema or versioning
• No path-traversal validation on route targets
• No circular-route detection
• The env var override (BEADS_DOLT_SERVER_DATABASE) is implicit and easy to misuse

The implementation quality is good -- this is not a code quality issue, it is a design direction question. Let us discuss in an issue before proceeding.

[nl0]

Putting this on hold for an architecture discussion. Cross-rig routing was deliberately removed in d762920

but doesn't this break gastown, since it relies on routes.jsonl to dispatch to rig-level beads?

and reintroducing it (even at narrower scope) needs consensus before merging

yeah this makes sense, i'm just sad bc removing it turned my gastown into a pumpkin after a routine update

[maphew]

Reviewed. The three changes are cohesive — routing foundation, doctor migration marker, and vc commit completeness belong together. One rebase note: routed.go will need these imports added (missing from the PR but required by the new functions): bufio, encoding/json, os, path/filepath, github.com/steveyegge/beads/internal/beads, github.com/steveyegge/beads/internal/debug. Routing logic is sound: prefix parsed from bead ID → routes.jsonl lookup → dolt_database read → read-only store opened. All routing/doctor tests pass. ✓

[maphew]

@nl0 I'm belaying my approval as I don't understand the situation or concerns @steveyegge raised well enough. I created issue #3055 for the discussion