fix(apply): remove coverage-proof scratch files from the uploaded proof tree - #963
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 30, 2026, 11:33 AM ET / 15:33 UTC. ClawSweeper reviewWhat this changesThe 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 Priority: P2 Review scores
Verification
How this fits togetherThe 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]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest 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:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 56d32bee74e4. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Problem
Every apply run today failed at "Validate downloaded close coverage proof tree" with:
runPrCloseCoverageProofModelwrites its scratch files (N-M.prompt.mdprompt dump,N-M.model.jsonmodel output) intoruntime.workDir— but that directory doubles as the uploaded proof artifact tree (.artifacts/apply-proof/pr-close-coverage-proof/*), and the downstream validator only admitsN-M.proof.jsonplusmanifest.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
CODEX_BINand asserts the workDir is empty of scratch files afterward; verified red on pre-fix code, green on the fix.test/pr-close-coverage-proof.test.tssuite: 26/26 pass.