feat: project one workspace mirror to multiple local views - #411
Conversation
|
Warning Review limit reached
Next review available in: 49 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 (4)
📝 WalkthroughWalkthroughWorkspace records now support registered symlinked subtree views. Workspace commands manage views and report their health. Sync state now records event-listener status, timestamps, heartbeat, connection attempts, and retry data. ChangesWorkspace views and listener health
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkspaceCLI
participant CanonicalMirror
participant Syncer
participant StateJSON
WorkspaceCLI->>CanonicalMirror: validate and create workspace view symlink
WorkspaceCLI->>CanonicalMirror: read view and canonical mirror health
Syncer->>StateJSON: publish listener heartbeat and connection health
WorkspaceCLI->>StateJSON: read persisted listener health
WorkspaceCLI-->>WorkspaceCLI: report view and listener status
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 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: 2
🧹 Nitpick comments (6)
cmd/relayfile-cli/main.go (2)
5637-5647: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe symlink survives a failed record write.
installWorkspaceViewcreates the symlink first. IfupsertWorkspaceDetailsthen fails, the symlink stays on disk but no view record exists.relayfile workspace view removecannot remove it, becauseworkspaceViewByLocalDirfinds no match. Remove the newly created symlink when persistence fails.♻️ Proposed refactor
if err := installWorkspaceView(localDir, target, *replace); err != nil { return err } record.Views = upsertWorkspaceView(record.Views, workspaceViewRecord{ RemotePath: remotePath, LocalDir: localDir, CreatedAt: time.Now().UTC().Format(time.RFC3339), }) if _, err := upsertWorkspaceDetails(record); err != nil { + _ = removeWorkspaceViewLink(localDir, target) return err }🤖 Prompt for 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. In `@cmd/relayfile-cli/main.go` around lines 5637 - 5647, Update the workspace-view creation flow around installWorkspaceView and upsertWorkspaceDetails so that, when persistence fails after the symlink is installed, the newly created symlink is removed before returning the error. Limit cleanup to the newly created view and preserve the existing error propagation.
5670-5676: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
view listreports nothing when the mirror is not configured, and emitsnullin JSON.
buildWorkspaceHealthReportreturns early whenrecord.LocalDiris empty, soreport.Viewsis nil even when views are registered. With--json,writeJSONthen writesnullinstead of an empty array. Build the view list fromrecord.Viewsdirectly, and normalize a nil slice to[]workspaceViewHealth{}before encoding.🤖 Prompt for 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. In `@cmd/relayfile-cli/main.go` around lines 5670 - 5676, Update the view-listing flow around buildWorkspaceHealthReport to derive views directly from record.Views rather than relying on report.Views when record.LocalDir is empty. Before passing the result to writeJSON, normalize a nil slice to an empty []workspaceViewHealth so JSON output is [] and preserve the existing text formatting.internal/mountsync/syncer.go (2)
7353-7368: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify what
listenerHeartbeatAtmeasures.
markSyncSuccessandmarkSyncErrorupdatelistenerHeartbeatAton every reconcile result, including errors. The value therefore tracks reconcile-loop liveness, not websocket listener liveness.canonicalViewStatusincmd/relayfile-cli/main.gouses this timestamp to decidestale, so a websocket that is disconnected but still polled will keep a fresh heartbeat. The separateStatusfield still exposes the disconnect, so this is not a correctness defect. Add a comment on the field to record the intended meaning.🤖 Prompt for 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. In `@internal/mountsync/syncer.go` around lines 7353 - 7368, Add a comment to the listenerHeartbeatAt field clarifying that it records reconcile-loop liveness, updated on both successful and failed reconcile results, rather than websocket listener connectivity. Mention that canonicalViewStatus uses it for staleness checks, while websocket disconnect state is represented separately by Status.
3500-3507: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a distinct status before the first connection attempt.
If
websocketis enabled and no dial has started,wsConnis nil andwsConnectingis false. The status is thenreconnecting, but no connection was ever attempted. Consumers cannot tell "never connected" from "connection lost" without also readingLastConnectedAt.♻️ Optional refinement
health.Mode = "websocket" switch { case s.wsConn != nil: health.Status = "listening" case s.wsConnecting: health.Status = "connecting" + case s.wsLastAttemptAt.IsZero(): + health.Status = "starting" default: health.Status = "reconnecting" }🤖 Prompt for 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. In `@internal/mountsync/syncer.go` around lines 3500 - 3507, Update the health status selection around wsConn and wsConnecting to distinguish the initial websocket-enabled state before any dial has started from the reconnecting state after a failed or lost connection. Use the existing connection-attempt or connection-history indicator to identify whether a connection was ever attempted, assign a distinct “never connected” status for the initial state, and preserve “listening” and “connecting” for their current conditions.cmd/relayfile-cli/main_test.go (2)
6959-7016: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test depends on the default mirror remote root.
The test never writes
.relay/state.jsonintocanonical, yetworkspace view addmust succeed.workspaceViewTargetcallsreadMountRemoteRoot(canonical)and thennormalizeWorkspaceViewRemotePath, which rejects an empty string. The test therefore asserts, implicitly, thatreadMountRemoteRootreturns a non-empty default. Make that dependency explicit, or add a case that writes an explicitremoteRoot.Note also that this test creates a symlink. On Windows, symlink creation requires elevated privileges or developer mode. Add a skip guard if the suite runs on Windows.
🤖 Prompt for 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. In `@cmd/relayfile-cli/main_test.go` around lines 6959 - 7016, Update TestWorkspaceViewsShareCanonicalMirrorWithoutRehome to make the mirror remote-root setup explicit by writing the expected .relay/state.json configuration into canonical, or add a separate explicit-remoteRoot case while preserving the current behavior. Also add a Windows skip guard before symlink creation so the test does not fail where symlinks are unavailable.
7045-7066: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case where
eventListeneris absent.This test always writes an
eventListenerblock.canonicalViewStatusreturnsnot-listeningwhenstate.EventListeneris nil, which is the state file produced by an older daemon. Add a case that omitsEventListenerand asserts the expected view status. This locks the backward-compatibility behavior discussed oncmd/relayfile-cli/main.goLine 5996.🤖 Prompt for 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. In `@cmd/relayfile-cli/main_test.go` around lines 7045 - 7066, The test around writeMirrorStateFile and buildWorkspaceHealthReport only covers a present EventListener; add a separate state-file case that omits EventListener entirely, then assert the resulting view status is “not-listening” while preserving the existing listener-state assertions for the current case.
🤖 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 `@cmd/relayfile-cli/main.go`:
- Around line 5996-6016: Update canonicalViewStatus in
cmd/relayfile-cli/main.go:5996-6016 to remove the nil EventListener else branch,
allowing absent listener state to fall through and return the trimmed
state.Status. Add a cmd/relayfile-cli/main_test.go:7045-7066 case with no
EventListener block that asserts the view status follows state.Status.
- Line 5732: Update the backslash normalization in the surrounding
value-processing logic to replace individual backslash characters with “/”,
using the correct Go string literal rather than the current two-character
pattern. Preserve all other value transformations unchanged.
---
Nitpick comments:
In `@cmd/relayfile-cli/main_test.go`:
- Around line 6959-7016: Update
TestWorkspaceViewsShareCanonicalMirrorWithoutRehome to make the mirror
remote-root setup explicit by writing the expected .relay/state.json
configuration into canonical, or add a separate explicit-remoteRoot case while
preserving the current behavior. Also add a Windows skip guard before symlink
creation so the test does not fail where symlinks are unavailable.
- Around line 7045-7066: The test around writeMirrorStateFile and
buildWorkspaceHealthReport only covers a present EventListener; add a separate
state-file case that omits EventListener entirely, then assert the resulting
view status is “not-listening” while preserving the existing listener-state
assertions for the current case.
In `@cmd/relayfile-cli/main.go`:
- Around line 5637-5647: Update the workspace-view creation flow around
installWorkspaceView and upsertWorkspaceDetails so that, when persistence fails
after the symlink is installed, the newly created symlink is removed before
returning the error. Limit cleanup to the newly created view and preserve the
existing error propagation.
- Around line 5670-5676: Update the view-listing flow around
buildWorkspaceHealthReport to derive views directly from record.Views rather
than relying on report.Views when record.LocalDir is empty. Before passing the
result to writeJSON, normalize a nil slice to an empty []workspaceViewHealth so
JSON output is [] and preserve the existing text formatting.
In `@internal/mountsync/syncer.go`:
- Around line 7353-7368: Add a comment to the listenerHeartbeatAt field
clarifying that it records reconcile-loop liveness, updated on both successful
and failed reconcile results, rather than websocket listener connectivity.
Mention that canonicalViewStatus uses it for staleness checks, while websocket
disconnect state is represented separately by Status.
- Around line 3500-3507: Update the health status selection around wsConn and
wsConnecting to distinguish the initial websocket-enabled state before any dial
has started from the reconnecting state after a failed or lost connection. Use
the existing connection-attempt or connection-history indicator to identify
whether a connection was ever attempted, assign a distinct “never connected”
status for the initial state, and preserve “listening” and “connecting” for
their current conditions.
🪄 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: 09b853eb-7ae5-4f7a-9c7b-a7dba17cf08e
📒 Files selected for processing (3)
cmd/relayfile-cli/main.gocmd/relayfile-cli/main_test.gointernal/mountsync/syncer.go
![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 across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
fe8eae1 to
4b08411
Compare
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
2 participants
Fixes #409.\n\nKeeps one canonical registered mirror and adds explicit symlink-backed subtree views. The canonical daemon remains the sole owner of local state, watcher, event cursor, and writeback outbox. Workspace health reports every view independently and public state exposes event-listener liveness so an idle listener is distinct from a disconnected one.\n\nVerification: go test ./cmd/relayfile-cli; go test ./internal/mountsync -run '^$'.