feat(mount): --push-local-once teardown drain (stop dropping last-moment writeback drafts)#304
Conversation
…eback drafts aren't dropped Local writeback drafts are ingested into the durable outbox by the running daemon's sync cycle (watcher + pushLocal). A draft written after that cycle and just before shutdown — e.g. a one-shot sandbox writing a final fire-and-forget reply right before teardown — is on disk but not yet in the outbox. The teardown cleanup runs --flush-outbox-once (outbox-only, deliberately no local scan, the flush-124 cure), so that draft is never ingested and is silently dropped: the header lands, the threaded reply vanishes. Add PushLocalAndFlushOnce / --push-local-once: one pushLocal pass (scans the on-disk mirror in a fresh process, so it catches drafts the running daemon never ingested) + an outbox flush, then exit. It skips pullRemote/digest/websocket, so it cannot reintroduce the pull-side flush-124 stalls — the only added cost is the local scan, which callers should gate on "pending local writes detected" (keeping --flush-outbox-once for the fast path). Test: TestPushLocalAndFlushOnceIngestsUnsyncedLocalDraft — an unsynced on-disk draft is dropped by FlushOutboxOnce (0 uploads) but ingested + uploaded + drained by PushLocalAndFlushOnce. Full mountsync suite + vet pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
More reviews will be available in 38 minutes and 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Changespush-local-once drain path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
…(unblock CI)
The v0.9.6 release bumped packages/sdk/typescript/package.json optionalDependencies
to @relayfile/mount-*@0.9.6 but did not regenerate the package's standalone
package-lock.json, which still pinned 0.9.5. CI runs `npm ci` against that
lockfile, so it failed EUSAGE ("lock file's @relayfile/mount-*@0.9.5 does not
satisfy 0.9.6") on every PR (SDK Typecheck / contract / E2E / evals), #304
included. Pre-existing and unrelated to the daemon change in this PR (Go Build +
Go Test pass).
Regenerated with `npm install --no-workspaces --package-lock-only` (the package
is an npm workspace member but ships a standalone lockfile, so the workspace
root must be ignored to re-resolve its own optional deps). `npm ci` now validates
clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
CI red here was not this PR — it's pre-existing, repo-wide lockfile drift. Synced the lockfile to 0.9.6 ( Heads up to whoever owns releases: the release tooling should regenerate |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/mountsync/syncer.go`:
- Around line 1648-1655: The PushLocalAndFlushOnce function is missing the
mount-root invariant guard that protects operations on the local mirror. Add a
call to assertMountRootInvariant in PushLocalAndFlushOnce before invoking
s.pushLocal(ctx), following the same pattern used in syncReserved. This ensures
that teardown cannot bypass the clobber/missing-root recovery gate on the new
local-scan path opened by the pushLocal call.
🪄 Autofix (Beta)
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: 598e8d73-7410-48cf-a3fa-18c300a55ee5
⛔ Files ignored due to path filters (1)
packages/sdk/typescript/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
cmd/relayfile-mount/main.gointernal/mountsync/syncer.gointernal/mountsync/syncer_test.go
…orkspace lockfile Two follow-ups on #304: - CodeRabbit (valid): PushLocalAndFlushOnce calls pushLocal (which scans+mutates the local mirror) without the mount-root invariant guard that syncReserved runs first. Add assertMountRootInvariant() at the top so the teardown drain can't operate on a wiped/clobbered mount root (recovery stays gated behind --reset-after-clobber). FlushOutboxOnce keeps skipping it — it's outbox-only. - CI: the prior commit synced packages/sdk/typescript/package-lock.json, but CI runs `npm ci` in that workspace-member dir WITHOUT --no-workspaces, so it validates the ROOT package-lock.json — which still pinned @relayfile/mount-*@0.9.5 vs package.json 0.9.6. Regenerated the root lockfile to 0.9.6; `npm ci` (workspace-aware) now validates clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in 25c7c5f:
|
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. |
Root cause
Local writeback drafts are ingested into the durable outbox by the running daemon's sync cycle (watcher +
pushLocal). A draft written after that cycle and just before shutdown — e.g. a one-shot deploy sandbox writing a final fire-and-forget Slack reply right before teardown — is on disk but not yet in the outbox.The teardown cleanup runs
--flush-outbox-once(FlushOutboxOnce: outbox-only, deliberately no local scan — the flush-124 cure), so that draft is never ingested and is silently dropped. Symptom downstream: the header message lands, the threaded reply vanishes.Fix
Add
PushLocalAndFlushOnce/--push-local-once: onepushLocalpass (scans the on-disk mirror — in the fresh cleanup process, so it catches drafts the running daemon never ingested) followed by an outbox flush, then exit. It skipspullRemote/digest/websocket, so it cannot reintroduce the pull-side flush-124 stalls. The only added cost over--flush-outbox-onceis the local scan, so callers should invoke it only when pending local writes are detected and keep--flush-outbox-oncefor the no-pending-writes fast path (the cloud cleanup shell already does thatfind -newerdetection — wired in the companion cloud PR).Test
TestPushLocalAndFlushOnceIngestsUnsyncedLocalDraft: an unsynced on-disk draft is dropped byFlushOutboxOnce(0 uploads) but ingested + uploaded + drained byPushLocalAndFlushOnce(1 upload, empty outbox). Fullmountsyncsuite +go vetpass.Rollout
New
relayfile-mountbinary → release → cloud bumpsRELAYFILE_MOUNT_VERSION+ cleanup shell invokes--push-local-onceon pending writes (companion PR) → snapshot rebuild. Backward-compatible: feature-detected; absent the flag, behavior is unchanged.🤖 Generated with Claude Code