fix(agents): stop copilot autoreview cleanup crash on Windows by paulcam206 · Pull Request #97901 · openclaw/openclaw

paulcam206 · 2026-06-29T18:36:09Z

What Problem This Solves

Fixes an issue where running the autoreview skill with --engine copilot on Windows could crash with a Python traceback (PermissionError: [WinError 32] The process cannot access the file because it is being used by another process) even though the code review itself completed successfully. The structured review result was discarded and the command exited non-zero for that run.

Why This Change Was Made

run_copilot runs inside a tempfile.TemporaryDirectory(...) context manager whose __exit__ deletes the directory. The spawned copilot -C <tempdir> process (and its MCP subprocesses) keep that directory as their working directory for a brief moment after exit, so on Windows the lingering directory handle makes rmtree raise WinError 32. Because the review had already finished, the cleanup race should never fail the run. Passing ignore_cleanup_errors=True makes the temp-dir cleanup best-effort, matching how the other engines already tolerate post-run temp-file cleanup. This is the only engine that used TemporaryDirectory; no other behavior changes.

User Impact

The autoreview copilot engine no longer fails a completed review because Windows has not released the temporary directory handle yet. No intended behavior change on macOS/Linux.

Evidence

Before: test-review-harness.ps1 -Fixture benign -Engine copilot aborted with the WinError 32 traceback from TemporaryDirectory.__exit__ (shutil.rmtree), exit code 1.
After: the same harness no longer aborts with the WinError 32 cleanup traceback; the separate prose-wrapped JSON parser failure is handled in the companion copilot parser PR.
Local %TEMP%\autoreview-copilot.* leftovers confirmed cleanup can fail without deleting the prompt directory; this change makes that cleanup best-effort instead of turning the finished review into a failure.
Closeout review on this commit (autoreview --mode commit --commit HEAD --engine claude): autoreview clean: no accepted/actionable findings reported, patch is correct (0.9).
ignore_cleanup_errors requires Python 3.10+; the script already targets 3.10+ (verified, runtime 3.14) and declares no older floor.

…r cleanup On Windows the spawned copilot process and its MCP subprocesses keep the TemporaryDirectory as their cwd briefly after exit, holding a directory handle. That makes TemporaryDirectory's rmtree raise PermissionError (WinError 32) on __exit__, aborting the run with a traceback even though the review completed successfully. Pass ignore_cleanup_errors=True so cleanup is best-effort and a post-review cleanup race never fails the run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

clawsweeper · 2026-06-29T18:37:19Z

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 2:47 PM ET / 18:47 UTC.

Summary
The PR makes the autoreview copilot engine's temporary prompt directory cleanup best-effort and adds an explanatory cleanup-race comment.

PR surface: Other +4. Total +4 across 1 file.

Reproducibility: yes. source-backed: on Windows, run .agents\skills\autoreview\scripts\test-review-harness.ps1 -Fixture benign -Engine copilot where the copilot child can briefly hold the temp directory as cwd. I did not rerun the Windows/copilot harness in this Linux review checkout.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Next step before merge

[P2] No repair lane is needed because the PR is already a focused implementation with sufficient proof and no actionable findings.

Security
Cleared: The diff only changes post-subprocess temporary-directory cleanup behavior and does not add dependencies, permissions, network access, secrets handling, or package-resolution changes.

Review details

Best possible solution:

Land the focused cleanup fix after ordinary maintainer and CI gates, while leaving the separate copilot parser behavior to #97902.

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

Yes, source-backed: on Windows, run .agents\skills\autoreview\scripts\test-review-harness.ps1 -Fixture benign -Engine copilot where the copilot child can briefly hold the temp directory as cwd. I did not rerun the Windows/copilot harness in this Linux review checkout.

Is this the best way to solve the issue?

Yes. Passing ignore_cleanup_errors=True is the narrowest maintainable fix because the review result has already been collected and the remaining failure is only post-run temp-directory deletion.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This fixes a real Windows-only autoreview failure with limited blast radius and no core runtime or channel-delivery impact.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides a concrete Windows harness command with before/after terminal outcomes for the cleanup traceback, which is sufficient real behavior proof for this small non-visual CLI fix.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a concrete Windows harness command with before/after terminal outcomes for the cleanup traceback, which is sufficient real behavior proof for this small non-visual CLI fix.

Evidence reviewed

PR surface:

Other +4. Total +4 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	0	0	0	0
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	1	5	1	+4
Total	1	5	1	+4

What I checked:

Current main still has strict cleanup: On current main, run_copilot uses tempfile.TemporaryDirectory(prefix="autoreview-copilot.") around the copilot child process, so cleanup-time PermissionError can still escape after the review subprocess returns. (.agents/skills/autoreview/scripts/autoreview:627, 82dfd8910743)
PR changes only copilot temp-dir cleanup policy: The PR head adds an explanatory comment and passes ignore_cleanup_errors=True without changing copilot invocation, tool allowlist, parsing, or result handling. (.agents/skills/autoreview/scripts/autoreview:631, f3d3afd925b0)
Sibling engine check: Codex and Droid use NamedTemporaryFile paths, Claude does not use a temp cwd, and only the copilot engine currently runs its child with a TemporaryDirectory as cwd. (.agents/skills/autoreview/scripts/autoreview:516, 82dfd8910743)
Python cleanup contract supports the fix: Python 3.12 TemporaryDirectory accepts ignore_cleanup_errors=False by default and threads that flag into cleanup/rmtree as ignore_errors, which is the exact cleanup behavior the PR opts into. (Python 3.12 tempfile.py:1019)
Harness precedent for best-effort temp cleanup: The autoreview harness already treats temporary repo cleanup failures as warnings rather than hard failures, matching the PR's post-review cleanup semantics. (.agents/skills/autoreview/scripts/test-review-harness.py:183, 82dfd8910743)
History and provenance check: git blame ties the current run_copilot block to the current-main import commit, while older history shows Peter Steinberger introduced the copilot engine shape and Vincent Koc recently hardened the Windows harness and autoreview scope policy. (.agents/skills/autoreview/scripts/autoreview:622, 5715744ea0c8)

Likely related people:

steipete: Peter Steinberger's autoreview engine work includes the older run_copilot/TemporaryDirectory implementation shape and adjacent engine support history. (role: feature-history owner; confidence: high; commits: 31a189db0ad3, 322ceb36ce4c, 236edb267df7; files: .agents/skills/autoreview/scripts/autoreview, .agents/skills/autoreview/SKILL.md)
vincentkoc: Vincent Koc recently hardened the autoreview Windows harness and updated the same autoreview skill policy path, including the merged bounded-scope autoreview work. (role: recent area contributor; confidence: high; commits: 35ce10337829, fd806ada649a; files: .agents/skills/autoreview/scripts/autoreview, .agents/skills/autoreview/scripts/test-review-harness.py, .agents/skills/autoreview/SKILL.md)
goutamadwant: Local blame on current main points the run_copilot block at the broad merge/import commit for PR fix(control-ui): apply seamColor bootstrap config #93699, though that PR's stated product area was not autoreview. (role: current-main carrier; confidence: low; commits: 5715744ea0c8; files: .agents/skills/autoreview/scripts/autoreview)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works

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.

…r cleanup (openclaw#97901) On Windows the spawned copilot process and its MCP subprocesses keep the TemporaryDirectory as their cwd briefly after exit, holding a directory handle. That makes TemporaryDirectory's rmtree raise PermissionError (WinError 32) on __exit__, aborting the run with a traceback even though the review completed successfully. Pass ignore_cleanup_errors=True so cleanup is best-effort and a post-review cleanup race never fails the run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

openclaw-barnacle Bot added the size: XS label Jun 29, 2026

paulcam206 mentioned this pull request Jun 29, 2026

fix(agents): recover copilot autoreview JSON wrapped in prose #97902

Closed

paulcam206 mentioned this pull request Jun 29, 2026

fix(agents): recover copilot autoreview JSON wrapped in prose #97908

Closed

vincentkoc self-assigned this Jun 29, 2026

vincentkoc merged commit 99bce5f into openclaw:main Jul 1, 2026
108 of 113 checks passed

clawsweeper Bot mentioned this pull request Jul 1, 2026

fix: honor filePath/path/media fallbacks in outbound reply normalization #63015

Closed

3 tasks

Navigation Menu

Uh oh!

fix(agents): stop copilot autoreview cleanup crash on Windows - #97901

fix(agents): stop copilot autoreview cleanup crash on Windows#97901
vincentkoc merged 1 commit into
openclaw:mainfrom
paulcam206:fix/autoreview-copilot-windows-tempdir-cleanup

paulcam206 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

paulcam206 commented Jun 29, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading