fix: share one Relayfile workspace mirror across routed repos - #220
Conversation
|
Warning Review limit reached
Next review available in: 58 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughFactory now mounts one registered workspace mirror for all routed repositories. It resolves explicit or persisted mirror roots, reuses the root across mount operations, reports local mount health, refreshes stale mirrors, and updates CLI, babysitter, and orchestration paths. ChangesShared workspace mirror
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config/schema.ts`:
- Around line 237-240: Update the localMountRoot schema validation to trim
input, reject blank values, and require an absolute path before the value
reaches the mount client. Preserve the optional behavior when the setting is
omitted, and ensure the validated value is the trimmed absolute path.
In `@src/mount/relayfile-cloud-mount-client.ts`:
- Around line 391-410: Move the authoritative registered-root retry from the
per-caller flow around _localMountRoot and _runLocalMountOperation into the
shared mount operation so concurrent unresolved callers await the same
fallback-and-retry sequence. Ensure the operation retries only when the
admission error provides a different registered root, while preserving
explicit/configured roots and existing cleanup behavior. Add a concurrent test
covering two unresolved callers receiving the same admission error and both
completing through the registered-root retry.
- Around line 331-333: Update the local mount root resolution in the constructor
to query both this.workspaceId and the Relayfile workspace UUID, matching the
identifier set used by the resolution path around lines 368-370. Apply the same
two-identifier input to config.workspaceMirrorResolver and
resolveRegisteredWorkspaceMirror while preserving the existing precedence and
fallback behavior.
In `@src/mount/workspace-mirror.ts`:
- Around line 40-45: Update the workspace registry lookup loop to collect
distinct normalized local roots for all accepted matching records instead of
returning the first result. Return the resolved root only when exactly one root
matches; otherwise return undefined, consistent with readMountStateDirectory().
Add coverage for two matching records with different roots.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b83909ad-3a48-415c-a311-371f60673d79
📒 Files selected for processing (14)
src/cli/fleet.test.tssrc/cli/fleet.tssrc/config/schema.tssrc/mount/local-mount-preflight.test.tssrc/mount/local-mount-preflight.tssrc/mount/relayfile-binary.test.tssrc/mount/relayfile-binary.tssrc/mount/relayfile-cloud-mount-client.test.tssrc/mount/relayfile-cloud-mount-client.tssrc/mount/workspace-mirror.test.tssrc/mount/workspace-mirror.tssrc/orchestrator/factory.tssrc/ports/index.tssrc/ports/mount.ts
|
Follow-up after CodeRabbit findings: implemented all four valid fixes in 5f5cb2b (absolute trimmed localMountRoot validation; both workspace aliases; shared concurrent admission fallback; ambiguous registry roots rejected).\n\nFinal verification:\n- tsc -p tsconfig.build.json && tsc-alias -p tsconfig.build.json\n- 6 focused test files passed, 195 tests passed\n- final rebuilt scratch daemon preflight: mounted=1 failed=0 routedRepos=16\n- final real 16-route run: successfulRoutes=16 failedRoutes=0 uniqueMirrorTargets=1 sdkMountStartsDuringRun=1\n- final stale status red-check: localMountDegraded=true, reason last reconcile 5m ago.\n\nNo re-home was requested. |
|
Follow-up for the event-transport requirement (commit
That distinguishes a quiet but registered feed from no listener, and a registered listener with a stale/dead mount feed. Listener status is registration state, not a claim that a websocket has delivered a recent event. Actual output from the rebuilt Focused verification after the update: The scratch workspace has no GitHub integration, so this patch does not claim an end-to-end provider webhook delivery test; it proves mount/liveness and listener-registration reporting. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Review follow-up (valid non-default-cadence finding) in The stale threshold is now three intervals from the registered mirror’s The added regression puts a live 2-minute mount beyond the default 90-second cutoff and verifies it remains healthy; another verifies invalid |
|
CI on |
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Post-review hardening in
All five Cubic review threads were answered and resolved. Final focused verification after rebuilding: Actual rebuilt-binary scratch red-check remains explicit after this commit: The scratch mount is still the only mount touched; no re-home was requested. |
|
Final remote verification for |
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
2 participants
Fixes #218
Fixes #219
Root cause confirmed
Factory derived a Relayfile mirror from every routed checkout, although Relayfile admits one local directory per workspace. That made the first mount succeed and every later route request a rejected re-home. The stale-repair path repeated that same checkout-derived target, so it could not heal the registered mirror. The diagnosis matches the live incident.
Change
localMountRoot, Relayfile's local registration, or (for older installs) the authoritative "already mirrored at ..." admission response. The fallback retries only that registered root; it never passes--rehome.factory statusexposelocalMountDegraded, its reason, and the mirror root.Local proof (built Factory 0.1.57; private scratch workspace and mirror only)
The scratch daemon used the real rebuilt Relayfile client/mirror and 16 configured routed repos. Its empty scratch workspace has no GitHub integration, so the daemon harness bypassed only that unrelated readiness gate; the mount and Factory start path were otherwise production code.
A direct 16-route run through the same built mount client recorded one SDK mount launch and one target:
Red-check: after backdating that scratch mirror by five minutes, the built CLI surfaced degradation:
Red-check/heal: the stale refresh was invoked with an unrelated routed checkout, and the output shows it targeted the registered scratch root and succeeded:
Verification: