Skip to content

fix(apply): remove coverage-proof scratch files from the uploaded proof tree - #963

Merged
steipete merged 1 commit into
mainfrom
fix/proof-scratch-pollution
Jul 30, 2026
Merged

fix(apply): remove coverage-proof scratch files from the uploaded proof tree#963
steipete merged 1 commit into
mainfrom
fix/proof-scratch-pollution

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Problem

Every apply run today failed at "Validate downloaded close coverage proof tree" with:

Unexpected coverage proof filename: 95982-111523.model.json

runPrCloseCoverageProofModel writes its scratch files (N-M.prompt.md prompt dump, N-M.model.json model output) into runtime.workDir — but that directory doubles as the uploaded proof artifact tree (.artifacts/apply-proof/pr-close-coverage-proof/*), and the downstream validator only admits N-M.proof.json plus manifest.json. Any successful model-backed proof therefore poisoned the artifact and hard-failed the entire apply lane (0 successful applies in the last 24h).

Fix

Successful proof runs now unlink both scratch files immediately after the model output validates, on both success paths (exit 0, and nonzero exit with valid output). Failed runs keep their scratch files for debugging — those runs never reach upload.

Proof

  • New regression test drives the real runner against a fake CODEX_BIN and asserts the workDir is empty of scratch files afterward; verified red on pre-fix code, green on the fix.
  • Full test/pr-close-coverage-proof.test.ts suite: 26/26 pass.
  • Autoreview (codex, xhigh): clean, no actionable findings.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jul 30, 2026
@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 30, 2026, 11:33 AM ET / 15:33 UTC.

ClawSweeper review

What this changes

The PR deletes temporary prompt and model-output files after successful close-coverage proof runs so only validator-approved proof artifacts are uploaded.

Merge readiness

Blocked until real behavior proof from a real setup is added - 4 items remain

Keep this PR open for a small policy cleanup and real after-fix proof. The implementation appears narrowly correct, but the branch edits release-owned CHANGELOG.md and the supplied validation is a fake-CODEX_BIN regression test rather than proof from a real apply/proof run.

Priority: P2
Reviewed head: 0a25ba46ac370467ccfc2a636e2250df1029dfff

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and covered by a regression test, but merge confidence is capped by mock-only runtime proof and the release-owned changelog edit.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The supplied regression test uses a fake CODEX_BIN; add redacted terminal output from a real isolated proof/apply run showing successful validation and the final uploaded-tree filenames before merge. 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 real behavior proof before merge: The supplied regression test uses a fake CODEX_BIN; add redacted terminal output from a real isolated proof/apply run showing successful validation and the final uploaded-tree filenames before merge. 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 4 items Successful-run cleanup: The PR introduces named prompt and model-output paths, validates the model output, then removes both temporary files only on successful result paths; failed validation still throws before cleanup, preserving diagnostics.
Focused regression coverage: The new test invokes the runner with a fake Codex executable and verifies that the proof work directory has no scratch files after a successful result.
Release-note policy: Repository policy says OpenClaw CHANGELOG entries are release-owned and normal contributor PRs should preserve release context in the PR body or commit message instead.
Findings 1 actionable finding [P3] Remove the release-owned changelog entry
Security None None.

How this fits together

The apply lane evaluates close candidates by running a model-backed coverage proof and uploading its proof tree for downstream validation. The proof runner writes temporary inputs and model output before the validator accepts the final proof artifact and allows the apply flow to continue.

flowchart LR
  A[Apply candidate] --> B[Close coverage proof runner]
  B --> C[Prompt and model scratch files]
  B --> D[Validated proof result]
  C --> E[Proof artifact tree]
  D --> E
  E --> F[Filename validator]
  F --> G[Apply decision]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The supplied regression test uses a fake CODEX_BIN; add redacted terminal output from a real isolated proof/apply run showing successful validation and the final uploaded-tree filenames before merge. 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.
  • Remove the release-owned changelog entry (P3) - AGENTS.md reserves OpenClaw CHANGELOG.md for release ownership and directs normal PRs to keep release context in the PR body or commit message. This PR already explains the user-visible behavior in its body, so retain that context there and drop this unrelated release-note edit.
  • Resolve merge risk (P1) - The supplied proof is mock-based, so it does not yet demonstrate that a real model-backed proof/upload path leaves a validator-acceptable artifact tree.
  • Resolve merge risk (P1) - The release-owned CHANGELOG.md edit would create normal-PR release-note churn contrary to repository policy.

Findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:12
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 3 files affected; 90 additions, 4 deletions One production runner, one focused regression suite, and one release-note file make the functional change easy to isolate before merge.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Keep the cleanup narrowly in the proof runner and its regression test, remove the release-owned changelog entry, and attach redacted output from an isolated real proof/apply run showing that the uploaded tree contains only accepted proof files and its manifest.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep the cleanup narrowly in the proof runner and its regression test, remove the release-owned changelog entry, and attach redacted output from an isolated real proof/apply run showing that the uploaded tree contains only accepted proof files and its manifest.

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

Yes at source level: the supplied regression test recreates a successful proof run whose temporary files share the uploaded work directory. The stated red-to-green result was not independently observed here and uses a fake Codex executable rather than the live model-backed path.

Is this the best way to solve the issue?

Yes for the functional bug: cleaning scratch files only after validated successful output preserves failed-run diagnostics while preventing invalid artifact uploads. Remove the release-owned changelog edit and add real-path proof before merge.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:12
    AGENTS.md reserves OpenClaw CHANGELOG.md for release ownership and directs normal PRs to keep release context in the PR body or commit message. This PR already explains the user-visible behavior in its body, so retain that context there and drop this unrelated release-note edit.
    Confidence: 0.98

Overall correctness: patch is correct
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: This repairs an apply-lane failure that blocks normal automated close processing, with a bounded operational blast radius.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The supplied regression test uses a fake CODEX_BIN; add redacted terminal output from a real isolated proof/apply run showing successful validation and the final uploaded-tree filenames before merge. 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.

Label justifications:

  • P2: This repairs an apply-lane failure that blocks normal automated close processing, with a bounded operational blast radius.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The supplied regression test uses a fake CODEX_BIN; add redacted terminal output from a real isolated proof/apply run showing successful validation and the final uploaded-tree filenames before merge. 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:

  • Successful-run cleanup: The PR introduces named prompt and model-output paths, validates the model output, then removes both temporary files only on successful result paths; failed validation still throws before cleanup, preserving diagnostics. (src/pr-close-coverage-proof.ts:276, 0a25ba46ac37)
  • Focused regression coverage: The new test invokes the runner with a fake Codex executable and verifies that the proof work directory has no scratch files after a successful result. (test/pr-close-coverage-proof.test.ts:425, 0a25ba46ac37)
  • Release-note policy: Repository policy says OpenClaw CHANGELOG entries are release-owned and normal contributor PRs should preserve release context in the PR body or commit message instead. (AGENTS.md:251, 56d32bee74e4)
  • Current proof limitation: The PR body reports a focused suite and an intentional fake-CODEX_BIN regression test, but does not include after-fix output from a real apply or model-backed proof run. (0a25ba46ac37)

Likely related people:

  • steipete: The PR description identifies the apply-lane failure mode and proposes the focused proof-artifact lifecycle repair. (role: operational reporter and recent area contributor; confidence: low; commits: 0a25ba46ac37; files: src/pr-close-coverage-proof.ts, test/pr-close-coverage-proof.test.ts)
  • clawsweeper[bot]: The only supplied commit for this focused runner and test change is authored by this account; current-main ownership history was not available in the review evidence. (role: recent patch author; confidence: low; commits: 0a25ba46ac37; files: src/pr-close-coverage-proof.ts, test/pr-close-coverage-proof.test.ts)

Rank-up moves

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

  • Remove the CHANGELOG.md hunk and retain the release context in the PR body.
  • Add redacted after-fix terminal or live output from a real isolated proof/apply run, including the accepted artifact-tree listing; a PR-body update should receive a fresh review automatically, or a maintainer can request one with @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.

@steipete
steipete merged commit f86bf91 into main Jul 30, 2026
16 checks passed
@steipete
steipete deleted the fix/proof-scratch-pollution branch July 30, 2026 15:36
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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant