fix: report bounded ASCII Box cleanup progress

[steipete]

Summary

Fix #1730: show periodic progress while ASCII Box cleanup waits on native commands, and preserve the deletion phase, exact accepted operation, and last validated status when a status lookup times out.

The reporter uses a cleanup-scoped cadence, so both a stalled command and repeated fast polls produce output. Each command joins its reporter before returning, keeping it out of the guarded remote-cleanup callback's writer. Remaining time comes from the actual command context; no new whole-Stop deadline is claimed.

Centralize native status/config execution through the existing prepared-command path and cap capture at 8 MiB per stream. Runner/capture errors remain errors even if their output looks like valid empty inventory or a completed operation. Deletion authority, claim bindings, retry rules, native mutation sequencing, and completion/absence gates remain intact. Documentation and Unreleased notes credit @shunkakinoki for the report.

Verification

• Red regression: a pending → blocked → stalled native deletion-status lookup returned only context deadline exceeded on previous source.
• Provider race suite passed after the fix, including held-command progress, fast-poll cadence, reporter join, partial-capture rejection, accepted-operation retention, and canceled inventory confirmation with both successful and failed responses.
• Existing real-subprocess output-limit, inherited-pipe, and controller-owned process-group tests passed under the race detector.
• Vet, docs-link checks, diff checks, independent semantic review, and managed review passed. The independent review caught two diagnostic gaps (snapshot phase labeling and cancellation precedence during failed inventory); both were corrected and retested.

Built live pending-retry proof and exact-head hosted CI are pending. The task's earlier ASCII Box is natively absent but its exact deletion operation is still blocked, so completed cleanup has not been proved. This draft does not claim successful destruction or permission to remove the retained claim.

Source-bound native pending-retry result

Candidate 966a19453879c019d947ebd3af1daf18db64957a, built CLI SHA-256 d36ac7c8a587c0bda8877060043321e811df3f3609504639432bf6c9bf7ec150, was run against the same task-owned retained claim using the official native ASCII CLI. It returned the expected exit 2 in 3.425 seconds:

ascii-box cleanup phase=deletion-operation elapsed=0s remaining=30s; waiting for native CLI
ascii-box cleanup phase=deletion-operation operation=bdop_53e2ade7b3f34fdc83113cd1cbc6a4d0 last_observed_status=blocked; retaining claim

The claim and lease trust file remained byte-identical. This is live proof of pending-retry diagnostics and conservative retention, not a claim that the provider's blocked deletion completed. Deterministic tests separately cover periodic output during a held call and across fast polling. The native operation remains the external cleanup hold; no new Box was allocated for this retry.

[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 September 4, 2026, 1:06 AM ET / 05:06 UTC.

ClawSweeper review

What this changes

The branch adds periodic stderr progress and clearer retained-claim diagnostics to ASCII Box cleanup, caps native CLI output capture, and adds regression coverage.

Merge readiness

⛔ Blocked before merge - 4 items remain

Keep open. The provider changes are narrowly scoped and preserve the deletion-completion safeguards, but the earlier changelog finding remains and the supplied 3.425-second native trace cannot demonstrate the new ten-second periodic-reporting behavior.

Priority: P2
Reviewed head: 966a19453879c019d947ebd3af1daf18db64957a

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The implementation and focused regressions are solid, but real evidence has not yet shown the periodic native-wait behavior it introduces.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: The changed production owner is the ASCII Box native-command path: cleanup contexts reach the reporter in the backend, and prepared native commands start it before calling the real CLI. The supplied official-CLI trace shows the after-fix initial deletion-operation diagnostic and retained claim, but its 3.425-second run cannot show the roughly ten-second periodic behavior; controlled runner tests are supplemental only. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The changed production owner is the ASCII Box native-command path: cleanup contexts reach the reporter in the backend, and prepared native commands start it before calling the real CLI. The supplied official-CLI trace shows the after-fix initial deletion-operation diagnostic and retained claim, but its 3.425-second run cannot show the roughly ten-second periodic behavior; controlled runner tests are supplemental only. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	6 items	Cleanup reporter implementation: The introduced reporter emits an initial phase update, then uses a shared ten-second ticker and joins its goroutine before the native command returns. Retained-claim safety path: Deletion waits retain the accepted operation while reporting the last validated status, and command capture errors are checked before decoded inventory or operation data can authorize cleanup. Real-proof gap: The PR body supplies a source-bound official-CLI trace with the expected retained-claim diagnostic, but its stated 3.425-second duration is shorter than the implemented roughly ten-second reporting interval and therefore does not observe a periodic follow-up report.
Findings	1 actionable finding	[P3] Leave the Unreleased entry to maintainers
Security	None	None.

How this fits together

ASCII Box is Crabbox’s provider adapter for allocating and releasing remote boxes through the provider’s native CLI. Cleanup uses retained lease claims and deletion-status checks to either finalize a verified deletion or safely retain the claim for retry.

flowchart LR
  A[Crabbox stop or rollback] --> B[ASCII Box cleanup]
  B --> C[Native ASCII CLI]
  C --> D[Deletion and inventory checks]
  D --> E[Retain or finalize claim]
  B --> F[Progress on stderr]
  F --> G[Operator diagnostics]

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The changed production owner is the ASCII Box native-command path: cleanup contexts reach the reporter in the backend, and prepared native commands start it before calling the real CLI. The supplied official-CLI trace shows the after-fix initial deletion-operation diagnostic and retained claim, but its 3.425-second run cannot show the roughly ten-second periodic behavior; controlled runner tests are supplemental only. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Leave the Unreleased entry to maintainers (P3) - Repository policy assigns CHANGELOG.md updates to maintainers for contributor PRs. Remove this branch-owned entry; maintainers can add the release note when the change lands.
• Resolve merge risk (P1) - The supplied live trace proves an initial diagnostic and claim retention, but not a periodic report during a native call that remains blocked past the ten-second cadence.
• Complete next step (P2) - Remove the contributor-authored CHANGELOG entry and add redacted native evidence showing at least two periodic progress reports during a blocked cleanup command; update the PR body for re-review.

Findings

• [P3] Leave the Unreleased entry to maintainers — CHANGELOG.md:5

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Scope	8 files; Go production +109 net, tests +156 net, docs +10	The production growth is confined to the ASCII Box adapter and is accompanied by more regression coverage than net production code.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Retain the bounded, fail-safe cleanup design, remove the contributor-authored changelog entry, and add a redacted native wait trace that visibly crosses the reporting interval before maintainers add the release note.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Retain the bounded, fail-safe cleanup design, remove the contributor-authored changelog entry, and add a redacted native wait trace that visibly crosses the reporting interval before maintainers add the release note.

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

Yes. An accepted ASCII Box deletion whose native status call remains blocked for more than ten seconds exercises the reporter through the normal cleanup path; the supplied trace confirms the real retained-claim path, though this review did not execute it.

Is this the best way to solve the issue?

Yes for the bounded-diagnostics portion of the linked report: the reporter sits at the native command boundary and preserves existing deletion authority and claim retention. The linked report’s separate fast-reconciliation request remains outside this PR’s scope.

Full review comments:

• [P3] Leave the Unreleased entry to maintainers — CHANGELOG.md:5
  Repository policy assigns CHANGELOG.md updates to maintainers for contributor PRs. Remove this branch-owned entry; maintainers can add the release note when the change lands.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• remove rating: 🧂 unranked krab: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

• P2: This improves cleanup observability and retained-claim diagnostics for one provider without evidence of a broader service outage.
• 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 changed production owner is the ASCII Box native-command path: cleanup contexts reach the reporter in the backend, and prepared native commands start it before calling the real CLI. The supplied official-CLI trace shows the after-fix initial deletion-operation diagnostic and retained claim, but its 3.425-second run cannot show the roughly ten-second periodic behavior; controlled runner tests are supplemental only. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Cleanup reporter implementation: The introduced reporter emits an initial phase update, then uses a shared ten-second ticker and joins its goroutine before the native command returns. (internal/providers/asciibox/cleanup_progress.go:51, 3288f47f6831)
• Retained-claim safety path: Deletion waits retain the accepted operation while reporting the last validated status, and command capture errors are checked before decoded inventory or operation data can authorize cleanup. (internal/providers/asciibox/client.go:410, 966a19453879)
• Real-proof gap: The PR body supplies a source-bound official-CLI trace with the expected retained-claim diagnostic, but its stated 3.425-second duration is shorter than the implemented roughly ten-second reporting interval and therefore does not observe a periodic follow-up report. (966a19453879)
• Contributor changelog policy: Repository policy assigns Unreleased changelog edits to maintainers and asks contributor PR authors to leave them for maintainer handling. (AGENTS.md:41, 966a19453879)
• Current-main necessity check: The fetched main tree has no cleanup-progress implementation; the verified main-to-head diff contains the new reporter and related provider changes, so the central diagnostics work is not already implemented on main. (internal/providers/asciibox/cleanup_progress.go, 53cc84ba13a1)
• Area history: Recent history connects Shun Kakinoki to the deletion-operation retry work and Peter Steinberger to longer-running ASCII Box lifecycle work. (internal/providers/asciibox/client.go, 67ccaeff1bc6)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• shunkakinoki: 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.
• Add a redacted native cleanup trace that shows at least two progress reports during a blocked call and the retained-claim outcome, then update the PR body for 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 (1 earlier review cycle)

• reviewed 2026-09-04T05:00:46.098Z sha 966a194 :: needs real behavior proof before merge. :: [P3] Leave the Unreleased entry to maintainers

[shunkakinoki]

Additional source-bound proof for exact head 966a19453879c019d947ebd3af1daf18db64957a:

• Built with repository-pinned Go 1.26.5; CLI SHA-256: 7026d7708af305127cadf94034d241850726b1f465832272f7bb6a550da31c11.
• go test ./internal/providers/asciibox passed.
• An unmodified retry against the retained real ASCII Box claim used the official native CLI, emitted the initial deletion-operation diagnostic, returned the native blocked status in about five seconds, and retained the claim.
• To cross the ten-second reporter cadence without allocating another Box, a temporary wrapper outside the repository delayed 22 seconds and then exec-ed the official /etc/profiles/per-user/shunkakinoki/bin/box with the original arguments. The production cleanup path emitted:

ascii-box cleanup phase=deletion-operation elapsed=0s remaining=30s; waiting for native CLI
ascii-box cleanup phase=deletion-operation elapsed=10s remaining=20s; waiting for native CLI
ascii-box cleanup phase=deletion-operation elapsed=26s remaining=30s; waiting for native CLI
ascii-box cleanup phase=deletion-operation elapsed=36s remaining=20s; waiting for native CLI
ascii-box cleanup phase=deletion-operation operation=<redacted> last_observed_status=blocked; retaining claim

The delay was controlled and is not presented as provider latency. It proves the exact candidate reporter remains live across multiple native-command waits, rejoins before return, then preserves the official CLI result and conservative claim retention. No Box was allocated for this proof.

The remaining branch actions are maintainer-owned: remove the contributor-authored Unreleased entry and reconcile the current merge conflict. The separate fast absent-Box reconciliation requested by #1730 remains outside this PR.

@clawsweeper re-review