Skip to content

fix: preserve Cloudflare workspaces with shared archive sync - #1814

Merged
steipete merged 2 commits into
mainfrom
codex/cloudflare-archive-owner-c396
Sep 4, 2026
Merged

steipete merged 2 commits into
mainfrom
codex/cloudflare-archive-owner-c396

Conversation

@steipete

@steipete steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

The legacy Cloudflare Containers adapter deleted the live checkout before preparing and uploading its replacement. An upload refusal or invalid archive therefore erased an otherwise reusable workspace. It also checked dirty-delta limits before uploading a full checkout, and could leave partially uploaded archives behind.

Use the existing shared archive owner for preparation, upload, staged extraction, replacement, and exact temporary-path cleanup. Fresh sync prepares and checks its full snapshot before allocating a container. Cloudflare still owns disk admission, HTTP upload with exact Content-Length, and runner execution; no new shared abstraction or upload protocol is introduced. Deletion-disabled sync retains its merge behavior.

This is the cloudflare/cf provider, not Cloudflare Sandbox or Dynamic Workers. Full Run finalization, keep-on-failure during preparation, and claim-revision fencing are separate follow-up work; this change does not claim to fix them.

Verification

Four unchanged-baseline regressions fail for the expected reasons: fresh/reused full-archive limits permit remote work, and rejected/corrupt uploads destroy existing files. Candidate tests execute real Bash/tar/filesystem operations through the transport fixture and verify preservation, replacement, merge semantics, Content-Length, and exact archive/staging cleanup.

Cloudflare/shared Go race tests, focused core archive races, vet, CLI build, docs build, and dead-code scan passed. The POSIX filesystem fixture now gates Windows/Bash availability after independent review caught that portability gap; Windows test compilation also passed. Managed Codex review is clean at the configured P0 threshold, and independent review found no production issue.

Live local Worker/container proof

The built CLI ran through repository-pinned Wrangler 4.127.0, the actual Worker/Durable Object code, and the checked-in Go runner in a real local Linux/amd64 Docker container (on an arm64 Mac). A loopback fault proxy truncated actual uploads to the runner; one returned a synthetic 500 after the partial write, another allowed extraction to discover the corrupt archive. This was real local container execution with synthetic fault injection, not a production Cloudflare deployment.

initial archive sync and file verification: passed
partial upload: exit 1; old file retained; new file absent; archive removed
corrupt extraction: exit 2; old file retained; archive/staging removed
successful replacement: new file present; old file removed
deletion-disabled merge: unrelated existing file retained
command exit 42: preserved
fresh sync and automatic cleanup: passed
explicit stop: passed

All 16 CLI steps completed with expected results. Both exact sandbox IDs reported stopped, their local claims were absent, and no container remained for the unique proof image. Wrangler and the fault proxy stopped, all three loopback listeners were absent, and task-only runtime/config state was removed. The image cache was retained.

CLI SHA256: ac06ed6b9344403686613f810c84d0a11aff47a37b74b311109dd56f39fdf9f9.
Runner image: sha256:458f6dc81d22ee6c350dcb30fd305cde0e499b61bda3a30a07fbdaca89746001.

Initial harness setup corrections concerned CLI config routing, proxy response encoding, required runner bindings, and discovering the installed Docker Buildx plugin inside an isolated configuration. None required a production code workaround, real credential, provider deployment, or changing the user's Docker configuration. Remote temporary cleanup remains best-effort and preserves the primary failure; this is not a durable cleanup journal.

@clawsweeper

clawsweeper Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 4, 2026
@clawsweeper

clawsweeper Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed September 4, 2026, 9:07 AM ET / 13:07 UTC.

ClawSweeper review

What this changes

The branch moves Cloudflare workspace synchronization onto the shared transactional archive flow, preserving existing workspaces when upload or extraction fails and enforcing full-snapshot limits before fresh allocation.

Merge readiness

Needs changes before merge - 2 items remain

Keep this PR open: the transactional archive approach and real-container proof support the Cloudflare workspace fix, but the contributor-authored changelog entry conflicts with repository policy.

Priority: P2
Reviewed head: 231ae20a06544c4fa1aae821822178de8f7578f0

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A well-scoped reliability fix with strong production-path local proof; only a small repository-policy correction remains.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (logs): The changed Cloudflare archive owner is exercised through the built CLI, actual Worker/Durable Object path, and checked-in runner in a local Docker container; the supplied trace records retained old files after injected partial-upload and corrupt-extraction failures, then successful replacement and deletion-disabled merge behavior.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (logs): The changed Cloudflare archive owner is exercised through the built CLI, actual Worker/Durable Object path, and checked-in runner in a local Docker container; the supplied trace records retained old files after injected partial-upload and corrupt-extraction failures, then successful replacement and deletion-disabled merge behavior.
Evidence reviewed 6 items Fresh-run admission: The Cloudflare backend prepares the archive before creating a new container, so full-snapshot preflight failure occurs before allocation.
Transactional workspace flow: The adapter delegates archive lifecycle to the shared flow while retaining Cloudflare-specific disk admission, HTTP upload, and command execution callbacks.
Focused regression coverage: The new fixtures cover fresh and reused archive-size rejection before remote work plus rejected upload, corrupt extraction, replacement, merge, Content-Length, and cleanup behavior.
Findings 1 actionable finding [P3] Remove the contributor-authored changelog entry
Security None None.

How this fits together

Cloudflare is Crabbox’s remote-container provider: the CLI packages a local checkout, sends it to a Durable Object-backed container, then runs the requested command there. The changed sync path determines whether an existing remote workspace is preserved, merged, or atomically replaced before command execution.

flowchart LR
  A[Local checkout] --> B[Archive preparation]
  B --> C[Cloudflare provider]
  C --> D[Remote disk admission]
  D --> E[Upload archive]
  E --> F[Staged extraction]
  F --> G[Workspace replacement or merge]
  G --> H[Remote command execution]
Loading

Before merge

  • Remove the contributor-authored changelog entry (P3) - Repository policy reserves Unreleased changelog edits for maintainers and agents and asks contributor PR authors to leave them out. Remove this line; a maintainer can add the release note when the fix lands.
  • Complete next step (P2) - Remove the contributor-authored CHANGELOG.md entry; maintainers will add the release note when the fix lands.

Findings

  • [P3] Remove the contributor-authored changelog entry — CHANGELOG.md:5
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Implementation and regression delta production +63/-110; tests +180/-50 The provider refactor removes local lifecycle code while adding focused coverage for the preservation and cleanup contract.

Technical review

Best possible solution:

Land the transactional Cloudflare sync flow after removing the contributor-authored Unreleased bullet; retain the shared archive lifecycle and the provider-specific disk and upload hooks.

Do we have a high-confidence way to reproduce the issue?

Yes, source reproducible: the parent implementation removed the live workspace before upload, and the added fixtures exercise rejected uploads and corrupt archives against that failure boundary.

Is this the best way to solve the issue?

Yes, apart from the changelog metadata: using the established shared archive owner removes the destructive ordering while leaving Cloudflare-specific transport and disk checks in the provider adapter.

Full review comments:

  • [P3] Remove the contributor-authored changelog entry — CHANGELOG.md:5
    Repository policy reserves Unreleased changelog edits for maintainers and agents and asks contributor PR authors to leave them out. Remove this line; a maintainer can add the release note when the fix lands.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9f1b19a89ffe.

Labels

Label changes:

  • add P2: This is a bounded reliability fix for reusable Cloudflare workspaces and does not establish an urgent widespread outage.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The changed Cloudflare archive owner is exercised through the built CLI, actual Worker/Durable Object path, and checked-in runner in a local Docker container; the supplied trace records retained old files after injected partial-upload and corrupt-extraction failures, then successful replacement and deletion-disabled merge behavior.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The changed Cloudflare archive owner is exercised through the built CLI, actual Worker/Durable Object path, and checked-in runner in a local Docker container; the supplied trace records retained old files after injected partial-upload and corrupt-extraction failures, then successful replacement and deletion-disabled merge behavior.

Label justifications:

  • P2: This is a bounded reliability fix for reusable Cloudflare workspaces and does not establish an urgent widespread outage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The changed Cloudflare archive owner is exercised through the built CLI, actual Worker/Durable Object path, and checked-in runner in a local Docker container; the supplied trace records retained old files after injected partial-upload and corrupt-extraction failures, then successful replacement and deletion-disabled merge behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The changed Cloudflare archive owner is exercised through the built CLI, actual Worker/Durable Object path, and checked-in runner in a local Docker container; the supplied trace records retained old files after injected partial-upload and corrupt-extraction failures, then successful replacement and deletion-disabled merge behavior.

Evidence

Acceptance criteria:

  • [P1] git diff --check.

What I checked:

  • Fresh-run admission: The Cloudflare backend prepares the archive before creating a new container, so full-snapshot preflight failure occurs before allocation. (internal/providers/cloudflare/backend.go:104, 919219640a49)
  • Transactional workspace flow: The adapter delegates archive lifecycle to the shared flow while retaining Cloudflare-specific disk admission, HTTP upload, and command execution callbacks. (internal/providers/cloudflare/sync.go:32, 919219640a49)
  • Focused regression coverage: The new fixtures cover fresh and reused archive-size rejection before remote work plus rejected upload, corrupt extraction, replacement, merge, Content-Length, and cleanup behavior. (internal/providers/cloudflare/backend_test.go:663, 919219640a49)
  • Real behavior proof: The PR body records a built CLI using the repository-pinned Wrangler Worker, Durable Object, and checked-in runner in a local Docker container; injected partial-upload and corrupt-archive faults retained the old workspace, while successful replacement and merge produced the stated results. (231ae20a0654)
  • Still needed on main: Current main contains only a later configuration-overlay refactor beyond the PR base; the two Cloudflare archive commits remain branch-only, so current main does not already provide this fix. (9f1b19a89ffe)
  • Changelog policy: Repository policy assigns Unreleased changelog edits to maintainers and agents; contributor PR authors should leave them out. (AGENTS.md:41, 231ae20a0654)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Altay: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Remove the contributor-authored CHANGELOG.md entry so maintainers can add it at landing.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete

steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer disposition: the changelog entry is intentional. This is a maintainer/agent-authored fix, not a request for a contributor to edit release notes. This repository requires maintainer/agent Unreleased entries for user-visible fixes as they land, with full PR links. The entry records the preserved-workspace behavior and matches that policy. There are no accepted technical findings in the review; no source change or rerun is needed for this author-role classification.

@steipete
steipete merged commit 5f8f118 into main Sep 4, 2026
28 checks passed
@steipete
steipete deleted the codex/cloudflare-archive-owner-c396 branch September 4, 2026 13:28
@steipete

steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Merged as 5f8f118. The landed tree exactly matches the verified merge preview.

Cloudflare now uses the shared prepared-archive and replacement owner while retaining its own HTTP upload and disk-admission behavior. A failed upload or extraction no longer deletes the previous checkout; fresh allocation is also preceded by sizing the complete snapshot rather than a potentially misleading dirty delta.

Proof includes regression failures on the unchanged baseline; Cloudflare/shared race tests, core archive tests, vet/build/docs and Windows cross-compilation; and a 16-step local Wrangler/Worker/Durable Object plus Linux Go-runner test covering partial upload, corrupt archive, replacement, merge mode, reuse and exit 42. Both native containers and claims were confirmed absent afterward. This was local native-runtime proof with synthetic inputs, not a production Cloudflare deployment. All 24 non-skipped exact-head checks passed without reruns, and Cloudflare/shared races passed again on main. The maintainer changelog entry was retained after resolving the bot's author-role misclassification.

The separate run-finalization and captured-claim lifecycle follow-up remains in progress; this archive change does not claim to fix those boundaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant