fix: successful runs fail after lease cleanup

[excelsier]

What Problem This Solves

Fixes an issue where a successful crabbox run could return exit code 7 when lease teardown killed an in-flight workspace-owner renewal after the command and lease cleanup had already succeeded.

Why This Change Was Made

Provider teardown and the renewal loop previously raced. Cleanup now uses an explicit provider-neutral release lifecycle:

• Apple-container and Azure explicitly opt into quiescing normal renewal, preserving any failure observed against the live lease, and synchronously renewing ownership for a 10-minute release grace period before deletion;
• coordinator-backed leases always use the fence because their release endpoint is destructive (delete=true) for every coordinator-supported provider;
• retained stop/pause releases use a bounded short fence, preserving serialization through provider shutdown without leaving the destructive ten-minute fence; reachable keep releases retain the historical post-release remote owner cleanup;
• reachable/static releases keep the historical post-release remote owner cleanup;
• external adapters default to destructive grace fencing; weaker reachable or retained modes require an explicit trusted capability that is bound into routing/scope and cannot be enabled by repository config;
• provider cleanup plans bind a whole-release timeout and a fence of at least that timeout plus 30 seconds; Hostinger covers its two-minute stop wait, GitHub Codespaces covers its complete API sequence, and NVIDIA Brev covers its twelve-minute delete poll plus pre-delete work;
• short-fence plans are rejected before teardown if their normalized fence can outlive the explicit-reuse acquisition window; the current five-minute short fence fits inside the five-minute-thirty-second wait budget, so immediate reuse waits through expiry instead of timing out;
• reachable/static providers preserve the historical post-release remote owner RELEASE; retained and destructive providers preserve their installed fence on either success or an ambiguous release failure;
• only after the required preparation succeeds may provider teardown begin, so teardown cannot kill an in-flight renewal and another client cannot recover a destructive workspace during retries.

If pre-release renewal fails, cleanup is skipped and the run fails closed. Unknown providers default to the destructive fence. GitHub Codespaces preflights the effective delete disposition once: dirty/unpushed work is retained behind the short fence, while a clean or uncertain delete becomes delete-or-fail so a later state change cannot silently turn a ten-minute delete fence into a successful retained stop.

User Impact

Apple-container and Azure one-shot runs no longer report a teardown-manufactured false failure after completing successfully. Genuine pre-release renewal failures remain fatal, destructive cleanup retains exclusive workspace ownership, and other providers retain their prior release behavior.

Evidence

• Reproduced twice on Azure one-shot runs: command exit 0, timing status succeeded, leaseStopped=true, provider inventory empty, followed by remote workspace owner renewal failed closed: signal: killed and outer exit 7.
• Added a blocked-renewal regression proving pre-release failures remain fatal.
• Added a takeover regression proving a competing owner cannot recover after the original short TTL while the release-grace fence is active.
• Added lifecycle regressions proving Namespace retained-stop uses the bounded short fence, Apple-container/Azure use destructive grace, takeover remains blocked, and pre-release renewal failures remain fatal.
• Added a coordinator-adapter regression that exercises the real release endpoint, verifies delete=true, and proves the grace fence survives destructive release; provider-spec coverage proves static SSH and Namespace are coordinator-disabled.
• Added concrete production-provider disposition coverage for destructive, retained-stop, and reachable releases, including Coder config/lease precedence.
• Kept provider-specific disposition in provider adapters and added separate, non-launderable provenance for the external cleanup capability.
• Added retained-stop concurrency regressions proving reacquire is blocked before and during provider shutdown, succeeds after the bounded short fence, and remains fenced on shutdown failure.
• Added Codespaces regressions for dirty-to-clean drift, clean-to-dirty drift, uncertain preflight, early release errors, config drift, and plan-cache cleanup.
• Added Hostinger and NVIDIA Brev duration contracts, plus a destructive-release-failure regression through the production cleanup helper.
• Added whole-operation timeout cancellation, failed owner-preparation plan finalization, oversized short-plan rejection, immediate reuse through the maximum short fence, and Vast keep owner-release regressions.
• Added Coder config-vs-persisted-label and Firecracker persisted-state-record regressions, so cleanup planning now uses the exact authority each provider release path executes; uncertain Firecracker state fails closed to grace.
• Independent adversarial review of exact head 7e3ad05d82c33bd9ae164c4b36ad8d9a66c81310 found no actionable P0/P1/P2 issues; the prior adversarial pass explicitly verified the original six lifecycle finding classes, and the final focused pass verified Coder/Firecracker release-authority parity.
• Focused lifecycle/provider tests and focused race tests passed on exact head 7e3ad05d82c33bd9ae164c4b36ad8d9a66c81310 across core, GitHub Codespaces, Hostinger, NVIDIA Brev, Vast, Coder, and Firecracker.
• Full go test ./... passed on the same exact head.
• Exact-head real workload proof at 7e3ad05d82c33bd9ae164c4b36ad8d9a66c81310: the built binary ran Bookeper's full cold Apple-container gate for 14m54.446s, returned command and outer launcher exit 0, reported runStatus=succeeded, totalMs=907451, endToEndMs=919790, and leaseStopped=true for lease cbx_5e9bf67f8b0c.
• A second exact-head real workload proof used Bookeper's optimized but complete canonical gate at repository head ed1d2ceff2616a7d773911aaf493ecae14975ec2: command 510.534s, Crabbox total 523.151s, end-to-end 535.503s, outer launcher 543s, exit 0, runStatus=succeeded, and leaseStopped=true for lease cbx_a9f9def2ca62. The run exceeded the normal renewal interval, retrieved and hash-verified the complete 8.56 MB replay-failed backend phase log before teardown, and left no lease/container/VM residue.
• The exact-head workload completed after the normal renewal interval and exercised keep=false destructive teardown. Post-run patched and installed provider inventories were empty; coordinator active leases were zero; direct Apple inventory contained only the persistent BuildKit helper; Azure provider GC and leak alarm reported zero VMs/orphans.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 12, 2026, 10:13 AM ET / 14:13 UTC.

ClawSweeper review

What this changes

The PR adds provider-selected workspace-owner cleanup plans so lease teardown cannot turn a successful Crabbox run into a late exit-code-7 failure.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked until stronger real behavior proof is added - 4 items remain

This PR remains necessary: current main and v0.41.5 still expose the teardown race. The patch has strong earlier-head proof, but the current head changed afterward and needs refreshed exact-head real-workload evidence before merge.

Priority: P1
Reviewed head: 0165ca20b8c7d8124c969ea9d090ebffd2473d3f

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The patch has substantial coverage and earlier real proof, but the final force-pushed head lacks matching real-behavior evidence.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: The PR provides convincing real workload output for 7e3ad05, but its current head is 0165ca2; add redacted exact-head output before merge, then update the PR body for re-review.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The PR provides convincing real workload output for 7e3ad05, but its current head is 0165ca2; add redacted exact-head output before merge, then update the PR body for re-review.
Evidence reviewed	5 items	Current main retains the failure path: After successful cleanup, current main stops the owner loop through CloseAfterLeaseRelease; that method returns any in-flight renewal failure, which can turn a completed run into an error. Affected behavior is shipped: Release v0.41.5 contains the same post-cleanup CloseAfterLeaseRelease path and owner-error return, so this is not main-only behavior. Workspace lifecycle provenance: The workspace-owner lifecycle dates to the reused-workspace serialization change and is included in released tags v0.41.3 and v0.41.4.
Findings	None	None.
Security	None	None.

How this fits together

Crabbox executes workloads on leased remote workspaces and maintains a remote owner marker while a run is active. Cleanup releases the provider lease and then removes or preserves the owner marker according to whether the target remains reachable.

flowchart LR
  A[Completed workload] --> B[Workspace owner marker]
  B --> C[Provider cleanup plan]
  C --> D{Release mode}
  D --> E[Release reachable owner]
  D --> F[Retained short fence]
  D --> G[Destructive grace fence]
  E --> H[Lease cleanup result]
  F --> H
  G --> H

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The PR provides convincing real workload output for 7e3ad05, but its current head is 0165ca2; add redacted exact-head output before merge, then update the PR body for re-review.
• Resolve merge risk (P1) - The real-workload proof is for an earlier head, not current head 0165ca2, which has 41 changed files and 2,127 additions.
• Resolve merge risk (P1) - Provider-specific release modes affect destructive cleanup and retained-workspace reuse; exact-head workload proof is needed alongside the extensive unit coverage.
• Complete next step (P2) - The remaining blocker is contributor-supplied exact-head real behavior proof, which repair automation cannot establish on the contributor’s provider setup.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test growth	production +711, tests +1,403, docs +13	The broad provider lifecycle change is accompanied by substantially larger focused regression coverage.
Changed surface	41 files, +2,127 / -133 lines	The patch spans core cleanup sequencing and many provider adapters, making exact-head behavior proof especially valuable.

Merge-risk options

Maintainer options:

1. Refresh current-head workload proof (recommended)
   Run a real destructive one-shot with the exact current binary past the renewal interval and publish redacted exit, status, lease-stop, and residue evidence before merge.
2. Accept the stale-head evidence
   Maintainers may merge based on the earlier-head workload proof while explicitly accepting that the final force-pushed changes lack equivalent live validation.

Technical review

Best possible solution:

Keep cleanup selection provider-neutral in core, derive each mode from the adapter’s actual release action, and merge after a redacted real workload proves current-head teardown succeeds beyond the renewal interval.

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

Yes—current source provides a high-confidence path: provider cleanup completes before the deferred owner close, and an in-flight renewal error is then returned as a run failure. The supplied Azure reproduction matches that path.

Is this the best way to solve the issue?

Yes—the provider-selected cleanup-plan boundary is the narrow maintainable solution because each adapter knows whether its release leaves the workspace reachable.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR provides convincing real workload output for 7e3ad05, but its current head is 0165ca2; add redacted exact-head output before merge, then update the PR body for re-review.
• remove proof: sufficient: Current real behavior proof status is insufficient, not sufficient.
• remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
• remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

• P1: A successful remote workload can currently report failure during lease teardown.
• merge-risk: 🚨 compatibility: The patch changes owner-fence and reuse behavior for retained, reachable, and destructive providers.
• merge-risk: 🚨 availability: Incorrect cleanup sequencing can fail completed runs or temporarily prevent workspace reuse.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR provides convincing real workload output for 7e3ad05, but its current head is 0165ca2; add redacted exact-head output before merge, then update the PR body for re-review.

Evidence

What I checked:

• Current main retains the failure path: After successful cleanup, current main stops the owner loop through CloseAfterLeaseRelease; that method returns any in-flight renewal failure, which can turn a completed run into an error. (internal/cli/run.go:574, f120203ec83b)
• Affected behavior is shipped: Release v0.41.5 contains the same post-cleanup CloseAfterLeaseRelease path and owner-error return, so this is not main-only behavior. (internal/cli/run.go:574, 1611d9dfc3d9)
• Workspace lifecycle provenance: The workspace-owner lifecycle dates to the reused-workspace serialization change and is included in released tags v0.41.3 and v0.41.4. (internal/cli/workspace_owner.go:318, 784092eeae1d)
• Proof does not cover the current head: The supplied workload proof names 7e3ad05, while the current PR head is 0165ca2 after a later force-push; the current head needs its own after-fix workload result. (0165ca20b8c7)
• Final-head source unavailable locally: The checkout is on main and reading the PR head requires a promisor fetch; network resolution failed, so no changed-line finding is asserted without source access.

Likely related people:

• Peter Steinberger: Current source lines in the cleanup and workspace-owner paths blame to the v0.41.5 release commit; he also has the largest history contribution across these files. (role: current-line author and recent area contributor; confidence: high; commits: 1611d9dfc3d9, f120203ec83b; files: internal/cli/run.go, internal/cli/workspace_owner.go)
• Vincent Koc: The reused-workspace serialization change introduced the central workspace-owner lifecycle surface. (role: workspace lifecycle contributor; confidence: high; commits: 784092eeae1d; files: internal/cli/run.go, internal/cli/workspace_owner.go)

Rank-up moves

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

• Run the destructive Apple-container or Azure workload with the exact current head past the renewal interval.
• Publish redacted command and launcher exits, successful status, lease-stop state, and post-run inventory output.
• Update the PR body to trigger a fresh ClawSweeper review; if it does not, ask a maintainer to comment @clawsweeper re-review.

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.

History

Review history (12 earlier review cycles; latest 8 shown)

• reviewed 2026-08-11T22:58:00.524Z sha 05e658f :: needs changes before merge. :: [P1] Preserve reuse after retained stop releases
• reviewed 2026-08-11T23:59:49.790Z sha 3980254 :: needs changes before merge. :: [P1] Fence coordinator-owned destructive releases
• reviewed 2026-08-12T00:13:39.108Z sha 2d1d94c :: needs changes before merge. :: [P1] Fence destructive direct-provider releases | [P1] Release retained owners before stopping their target
• reviewed 2026-08-12T00:18:51.513Z sha 2d1d94c :: needs changes before merge. :: [P1] Fence destructive direct-provider releases | [P1] Release retained Namespace ownership before shutdown
• reviewed 2026-08-12T01:21:05.466Z sha 853e73e :: found issues before merge. :: [P1] Handle Codespaces delete-to-stop fallback before fencing | [P1] Do not assume external release leaves SSH reachable | [P2] Keep provider release classification out of core
• reviewed 2026-08-12T03:18:24.688Z sha 7e3ad05 :: needs maintainer review before merge. :: none
• reviewed 2026-08-12T03:43:24.497Z sha 7e3ad05 :: needs changes before merge. :: [P3] Align the Vast keep lifecycle fixture with production
• reviewed 2026-08-12T04:41:43.535Z sha 7e3ad05 :: needs changes before merge. :: [P3] Align the Vast keep lifecycle fixture with production

[excelsier]

Addressed the ordering finding at e23bde5: renewal now quiesces before provider release begins, so a pre-release failure remains fatal and teardown cannot manufacture a late renewal failure. Focused race coverage is green. I am adding the requested live production-boundary proof before asking for re-review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e23bde562b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[excelsier]

Live after-fix proof on exact head e23bde562b02a0678840060c2df88f1d5df84ef1:

• real Apple-container one-shot ran sleep 15 (past renewal interval), then printed patched-boundary-ok
• command exit 0; outer CLI exit 0
• runStatus=succeeded, commandMs=15110, totalMs=17808, endToEndMs=30270
• leaseStopped=true
• post-run provider list empty; only Apple container persistent BuildKit helper remained
• no remote workspace owner renewal failed closed / outer exit 7

The revised implementation no longer suppresses errors. It prevents the teardown race by quiescing renewal before provider release and preserves any error observed on the live lease. @clawsweeper re-review

[excelsier]

Addressed the retry-window P1 at cfcee8d036c70e528e94b1d0cd7ce60a00abc2ae. Cleanup now quiesces ordinary renewal, synchronously extends the workspace-owner fence to 10 minutes (above the coordinator ~320s retry budget), and only then begins provider deletion. The 30s preparation bound remains below the normal 45s TTL; a failed extension skips cleanup and fails closed. Added takeover-after-original-TTL and pre-release-failure regressions. Focused, focused-race, and go test ./... proof are green. I am running the final exact-head live Apple-container proof now. @clawsweeper re-review

[excelsier]

Final exact-head live proof on cfcee8d036c70e528e94b1d0cd7ce60a00abc2ae: built the CLI from that head and ran a real Apple-container one-shot for 15s (past the normal 10s renewal interval). It printed final-release-fence-ok; command and outer CLI exit were 0; timing reported runStatus=succeeded, commandMs=15102, totalMs=17066, endToEndMs=29494, and leaseStopped=true for lease cbx_164f17b06006. Post-run patched provider list []; installed coordinator list []; direct container inventory contains only Apple’s persistent BuildKit helper; Bookeper Azure/provider leak alarm is clean at 0 leases / 0 Azure VMs. No late renewal rc7 and no competing-cleanup gap.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfcee8d036

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[excelsier]

Resolved the persistent-target P1 on exact head 05e658f688723000ddec92ea0c7491f979a0f12c. A provider-neutral reachability policy now keeps normal renewal through provider release for persistent targets, then performs remote owner RELEASE; destructive providers retain the 10-minute fail-closed grace fence. Actual cleanup-composition tests prove immediate persistent-target reacquire succeeds and destructive-target takeover remains blocked. After rebasing onto current upstream main, focused race tests and go test ./... passed. Exact-head Apple one-shot also exited 0 with runStatus=succeeded, leaseStopped=true, and zero residue. @clawsweeper re-review

[excelsier]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05e658f688

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[excelsier]

Exact head 7e3ad05d82c33bd9ae164c4b36ad8d9a66c81310 aligns Coder and Firecracker cleanup planning with the exact release authority each provider executes. Focused, race, full Go, and independent exact-head review are green; unresolved threads are zero. @clawsweeper re-review

[excelsier]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 7e3ad05d82

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[excelsier]

Maintainer-ready exact-head update: Codex reviewed 7e3ad05d82 clean, unresolved threads are zero, and the PR body now includes a fully hydrated real workload run using the exact built head. Bookeper full cold Apple gate ran 14m54.446, command and outer launcher exited 0, runStatus=succeeded, leaseStopped=true, and all provider/coordinator/Azure/GC inventories were clean. This also supplies the hydrated exact-head lifecycle proof requested by ClawSweeper. Fork CI/Connector/Release/Docs workflows remain action_required pending repository approval.

[excelsier]

Additional exact-head lifecycle proof is now in the PR body: the optimized-but-complete Bookeper canonical gate ran 9m03 outer / 8m30 command on the exact 7e3ad05d82 Crabbox binary, exited 0, retrieved and hash-verified the replay-failed 8.56 MB backend phase log before teardown, reported leaseStopped=true, and left zero leases/containers/Azure VMs/GC candidates. This is a second real workload beyond the earlier 14m54 run and exercises the normal renewal interval plus destructive keep=false cleanup.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.