Putting cross-rig routing on hold for an architecture discussion

[maphew]

Putting #2954 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.

Originally posted by @steveyegge in #2954 (comment)

[miridius]

Cross-Rig Routing Architecture Analysis

Analysis of the four concerns raised, with proposed solutions.

Current State

Cross-rig routing code is present in main (internal/routing/routes.go, ~450 lines) despite the d762920 removal. Key functions: LoadRoutes, ResolveBeadsDirForID, ResolveBeadsDirForRig. Integration via cmd/bd/routed.go and cmd/bd/create.go (--rig flag). Still missing: move.go, refile.go, and all routing tests.

---

Concern 1: routes.jsonl has no schema or versioning

Current: JSONL format, no version field, malformed lines silently skipped, missing fields silently ignored.

Proposal: Add a header line: {"_version": 1, "_schema": "routes"}. LoadRoutes checks first non-comment line for version. Unknown versions return error. Add bd doctor validation for required fields, prefix format, path safety.

---

Concern 2: No path-traversal validation

Current: Route paths joined directly with townRoot via filepath.Join(townRoot, route.Path, ".beads"). No validation that resolved path stays within town root. filepath.Join("/town", "../../etc") = /etc.

Precedent: cmd/bd/doctor/fix/common.go has IsWithinWorkspace validation.

Proposal: Add ValidateRoutePath(townRoot, routePath) that checks filepath.Clean(resolved) has townRoot prefix. Call in LoadRoutes or ResolveBeadsDirForID. Add bd doctor check.

---

Concern 3: No circular-route detection

Analysis: True circular prefix routing isn't possible in current design (all rigs share town-level routes.jsonl, and resolution doesn't recurse into target's routes). The real risk is redirect chains: beads.ResolveRedirect follows .beads redirects with no max-depth.

Proposal: Add max-depth (3 hops) to ResolveRedirect. Add bd doctor check for redirect cycles and duplicate prefixes.

---

Concern 4: BEADS_DOLT_SERVER_DATABASE env var is implicit

Current: os.Setenv("BEADS_DOLT_SERVER_DATABASE", ...) in two places (routes.go:187, routes.go:276). Never cleaned up. Process-global, not concurrency-safe, affects all subsequent GetDoltDatabase() calls.

Options (escalating):

• A (minimal): Save/restore env var around routing calls
• B (better): Pass database name through function params to store constructor
• C (best): Add WithDatabase(name) option to config, eliminating env mutation entirely (as hilmes review recommended)

---

Additional Gaps

• Missing tests: ~1200 lines of routing tests deleted in d762920, not restored
• No route priority: First-match wins with no longest-prefix matching
• No bd doctor integration: Should validate schema, paths, duplicates, cycles

Recommended Phasing

1. Phase 1 (prerequisite): Path-traversal validation + env var cleanup (safety)
2. Phase 2: Schema versioning + bd doctor checks (operational safety)
3. Phase 3: Restore test coverage (quality gate)
4. Phase 4: Re-enable cross-rig commands with full validation

Routing code can stay in codebase, but --rig flag and cross-rig resolution should remain gated until Phases 1-3 are complete.

🤖 Generated with Claude Code