Fix local bucket mount synchronization - #890
Conversation
🦋 Changeset detectedLatest commit: 450d1b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
🐳 Docker Images Published
Usage: FROM cloudflare/sandbox:0.0.0-pr-890-450d1b9fVersion: 📦 Standalone BinaryFor arbitrary Dockerfiles: COPY --from=cloudflare/sandbox:0.0.0-pr-890-450d1b9f /container-server/sandbox /sandbox
ENTRYPOINT ["/sandbox"]Download via GitHub CLI: gh run download 33654241059 -n sandbox-binaryExtract from Docker: docker run --rm cloudflare/sandbox:0.0.0-pr-890-450d1b9f cat /container-server/sandbox > sandbox && chmod +x sandbox |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
db26341 to
6d8d6fe
Compare
Large transfers and runtime replacement could leave local R2 mounts stuck or lose pending changes. Stream transfers, retry serialized updates, and bind mount synchronization to the active runtime.
6d8d6fe to
450d1b9
Compare
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| await this.withEchoSuppression(containerPath, () => | ||
| this.transferR2ObjectToContainer(key, containerPath, generation) | ||
| ); |
There was a problem hiding this comment.
🔴 Pending local edits get overwritten
While a local upload is pending, pollR2ForChanges can replace the newer local file with the previous upload. The delayed upload republishes old bytes, losing the edit.
Prompt for agents
LocalMountSyncManager serializes transfers but pollR2ForChanges does not honor pathIntentVersions. If upload A is active, a second filesystem event records intent B, and a poll queues before B's debounce expires, the poll can observe A in R2 while the snapshot still represents the pre-A object. It then writes A back over local B; B's delayed upload reads A and permanently loses B. Update pollR2ForChanges to avoid applying R2 modifications or deletions for container paths with an active local intent. Preserve retry behavior and define how the skipped R2 state is revisited after the local intent finishes.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const settle = (async () => { | ||
| if (this.uploadTimers.size > 0) { | ||
| await delay(UPLOAD_DEBOUNCE_MS); | ||
| } | ||
| await this.transferQueue; |
There was a problem hiding this comment.
🟡 Stop drops late upload retries
An upload failure during stop can schedule its retry after transferQueue settles. interrupt clears that retry, so unmounting can discard the final change.
Prompt for agents
LocalMountSyncManager.stop checks uploadTimers only once, then awaits transferQueue. An active transfer can reject while stop is waiting; scheduleUploadAttempt handles that rejection outside transferQueue and creates a retry timer. Stop then finishes settling and interrupt clears that timer. Make graceful stop track retry handlers and newly scheduled retry timers until no admitted upload intent remains, subject to stopTimeoutMs. Ensure timeout still aborts stalled streams and releases the runtime hold.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Local bucket mounts could fail on large files, lose updates after transient R2 errors, or remain tied to a replaced container runtime.
Resolves #864 on
next.Validation
NO FINDINGSnpm run check: 23/23 tasks