fix(live-proof): keep review module caches writable

[vincentkoc]

What Problem This Solves

Review live-proof jobs can finish successfully and then fail while removing their private HOME because Go creates read-only module-cache directories by default. The cleanup error masks the successful review, requeues the item, and repeats the review work.

Why This Change Was Made

The private live-proof child environment now pins GOMODCACHE under its scratch profile and composes inherited flags with [environment.GOFLAGS, "-modcacherw"].filter(Boolean).join(" "). Appending preserves existing flags and makes the final value authoritative when an inherited environment contains -modcacherw=false.

The environment composition lives in one focused producer helper so the exact inherited and replacement values can be tested without adding cleanup behavior.

This fixes the state at its producer. It does not add permission recovery, filesystem traversal, cleanup retries, ignored cleanup failures, or process/worktree lifecycle changes.

User Impact

Successful Go-backed review proofs no longer create read-only module-cache directories under the private scratch profile, so ordinary recursive cleanup can complete without turning a successful review into a failed and requeued run.

OpenClaw Bay Impact

Bay is unaffected. Queue state, durable records, publication payloads, comments, dashboard contracts, and cleanup ownership are unchanged.

Documentation Impact

Reviewed the live-proof documentation and contributor workflow. No documentation update is needed because commands, output, flags, and operator workflow are unchanged.

Evidence

Exact head: e5c34566436448ad33dba6e0499f173ec8e39e55

• Signed commit based directly on current origin/main.
• node_modules/.bin/tsc -p tsconfig.json
• env -u TMUX -u TMUX_PANE TMUX_TMPDIR=<isolated> node --test test/live-proof-review-environment.test.ts - 2 passed
• Same isolated environment with test/live-proof.test.ts, test/review-blob-hydration.test.ts, and test/live-proof-review-environment.test.ts - 62 passed
• Targeted oxfmt, type-aware oxlint, and git diff --check
• Real Go module-cache proof: default mode 0555; inherited -trimpath -modcacherw=false followed by -modcacherw produced mode 0755; recursive removal succeeded
• Committed codex review --base origin/main - no introduced defect

The linked worktree intentionally shares node_modules, so repository-level pnpm wrappers attempt to purge that symlink. Their focused exact-head constituents ran directly. Hosted CI provides the clean-install full-check proof.

Real Behavior Proof

• Claim: inherited Go flags remain intact while the private review profile owns writable module-cache directories.
• Exercised surface: executeReviewLiveProofs child-environment construction and the real static-plan execution path.
• Scenario: the parent environment supplies GOFLAGS=-trimpath -modcacherw=false and a shared GOMODCACHE.
• Command and environment: focused Node live-proof execution on macOS plus real go mod download runs against separate default and appended-flag caches.
• Observed result: the environment producer returns GOFLAGS=-trimpath -modcacherw=false -modcacherw, replaces the shared module-cache path with its scratch-local cache, and the live-proof integration test completes. The real Go cache changes from mode 0555 by default to 0755 with the appended flag and is recursively removable.
• Artifact or trace: the focused test prints the successful sanitized live-proof receipt; the redacted Go probe records default=555 writable=755 and writable_cleanup=removed.
• Dependency contract: Go applies repeated boolean flags in order, so the appended value wins.
• Limits: local proof ran on macOS with an isolated tmux socket; hosted CI covers the clean Linux runner.

Prior Review Findings

This replaces the previous broad lifecycle rewrite entirely. The reviewed lstat/chmod race, detached-descendant concern, permission-recovery traversal, and cleanup retry behavior are absent from this head. The narrow producer fix leaves the existing cleanup and process ownership model untouched.

A pre-commit review also reproduced the separate existing-server tmux environment limitation. That is not accepted into this PR: fixing it requires the previously rejected process/socket lifecycle expansion, while the affected exact-review workflow runs the proof in a fresh hosted job. This PR deliberately keeps the reviewed producer-only scope.

Diff

• Production: +6/-0
• Tests: +18/-1

The production delta creates one canonical, directly testable Go-environment producer and applies it at the existing child boundary.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 4:36 AM ET / 08:36 UTC.

ClawSweeper review

What this changes

The PR gives each review live-proof child a scratch-local Go module cache and appends Go’s writable-module-cache flag while retaining inherited Go flags.

Regression provenance

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

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

No blocking correctness or security defect was found. This bounded fix has credible after-fix terminal evidence and focused coverage, but remains open for normal maintainer review because it is member-authored.

Priority: P1
Reviewed head: e5c34566436448ad33dba6e0499f173ec8e39e55

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A small, direct reliability fix with meaningful after-fix terminal evidence and no actionable review finding.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body supplies after-fix terminal evidence from a real Go module-cache probe, including cache modes and successful recursive cleanup.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body supplies after-fix terminal evidence from a real Go module-cache probe, including cache modes and successful recursive cleanup.
Evidence reviewed	5 items	Focused producer change: The exact head derives GOFLAGS and GOMODCACHE from the sanitized child environment, replacing any inherited module-cache location with a path under the private profile. Production boundary consumption: The produced Go environment is spread into the same environment passed to the live-proof child process, before HOME and other profile-local caches are set. Regression coverage: Focused coverage verifies preservation of an inherited flag, trailing -modcacherw precedence, and replacement of an inherited GOMODCACHE path.
Findings	None	None.
Security	None	None.

Live Verification

Command: pnpm run build && node --test test/live-proof-review-environment.test.ts

Result: PASS (completed)

pnpm run build && node --test test/live-proof-review-environment.test.ts
runner@runnervm76f27:/tmp/clawsweeper-live-proof-1220-PQSteX/target$ pnpm run build && node --test test/live-proof-review-environment.test.ts
$ tsc -p tsconfig.json
pnpm run build && node --test test/live-proof-review-environment.test.ts
[live-proof] sanitized environment assertion passed: credentials=0
[live-proof] item=42 head=de474d402a4f4c9f06b7fc9c67d0b2d55e83896d execution=unsandboxed credentials=0
✔ review live proof composes inherited Go environment settings (0.788611ms)
✔ review live proof runs an unsandboxed static plan with a sanitized child environment (1959.255235ms)
ℹ tests 2
ℹ suites 0
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 2058.234422
runner@runnervm76f27:/tmp/clawsweeper-live-proof-1220-PQSteX/target$ pnpm run build && node --test test/live-proof-review-environment.test.ts
$ tsc -p tsconfig.json

Assertions:

• PASS expect_output: review live proof composes inherited Go environment settings

How this fits together

Live-proof execution runs a reviewed repository command in a private scratch profile. The child environment determines where Go writes its module cache before the scratch profile is removed after proof execution.

flowchart LR
A[Review job] --> B[Live-proof plan]
B --> C[Private scratch profile]
C --> D[Child environment]
D --> E[Go module cache]
E --> F[Proof command]
F --> G[Scratch cleanup]

Loading

Before merge

• Complete next step (P2) - No repair-lane action is needed because this PR already contains the narrow implementation and has no actionable finding.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	2 files affected; production +6/-0, tests +18/-1	The change is a narrowly scoped environment producer with focused regression coverage.

Technical review

Best possible solution:

Land the constrained child-environment fix after the remaining required checks finish, keeping Go cache handling localized to the review live-proof boundary.

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

Yes—current source gives a focused path: run a Go-backed live proof with its scratch profile as HOME, then remove that profile after module-cache population; the PR body also records an after-fix real-Go probe.

Is this the best way to solve the issue?

Yes—the change repairs the cache state at the child-environment producer rather than broadening cleanup permissions, retries, or process lifecycle behavior.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix terminal evidence from a real Go module-cache probe, including cache modes and successful recursive cleanup.

Label justifications:

• P1: The fix targets production review jobs that can requeue and repeat successful work when scratch cleanup fails.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies after-fix terminal evidence from a real Go module-cache probe, including cache modes and successful recursive cleanup.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix terminal evidence from a real Go module-cache probe, including cache modes and successful recursive cleanup.

Evidence

What I checked:

• Focused producer change: The exact head derives GOFLAGS and GOMODCACHE from the sanitized child environment, replacing any inherited module-cache location with a path under the private profile. (src/live-proof/review-artifacts.ts:46, e5c345664364)
• Production boundary consumption: The produced Go environment is spread into the same environment passed to the live-proof child process, before HOME and other profile-local caches are set. (src/live-proof/review-artifacts.ts:125, e5c345664364)
• Regression coverage: Focused coverage verifies preservation of an inherited flag, trailing -modcacherw precedence, and replacement of an inherited GOMODCACHE path. (test/live-proof-review-environment.test.ts:15, e5c345664364)
• Feature provenance: Current-main history identifies the merged live-proof implementation as the prior source change for this path. (src/live-proof/review-artifacts.ts:125, 03c1f452ec49)
• After-fix behavior evidence: The PR body documents a real Go module-cache probe where default mode 0555 became writable mode 0755 with the appended flag and recursive cleanup succeeded. (e5c345664364)

Likely related people:

• steipete: Merged history identifies this contributor’s live-proof implementation as the origin of the reviewed execution path. (role: introduced live-proof review execution; confidence: high; commits: 03c1f452ec49; files: src/live-proof/review-artifacts.ts)

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 (2 earlier review cycles)

• reviewed 2026-08-21T15:59:30.960Z sha 0664e36 :: found issues before merge. :: [P1] Eliminate the scratch-tree chmod race
• reviewed 2026-08-25T08:11:11.498Z sha a6d8945 :: needs maintainer review before merge. :: none

[yetval]

Flagging production impact in case it helps prioritize this draft. This scratch cleanup is failing live on main (introduced in #1205) and is currently the single largest source of failed runs.

The rmSync(scratch, ...) in the finally throws EACCES because the scratch dir is handed to the child as HOME/TMPDIR/PNPM_HOME/XDG_CACHE_HOME, and the package caches create read-only directories that force: true cannot remove. Because it is in a finally, it throws after the review has already succeeded, so the job ends as:

##[error]Exact review generation failed: classification=codex_or_content_failure reservation=posted review_outcome=success review_exit=0

The review itself succeeded (review_outcome=success review_exit=0); only the cleanup failed, but the item is then requeued and re-reviewed from scratch. steipete/camsnap#15 has looped on this roughly 184 times in 24 hours (26 of the latest 100 failed runs), each paying for a full review that is never published.

Your recoverScratchDirectoryAccess approach here is the right shape, and the AggregateError also handles the case where a real error from the try body is otherwise masked by the cleanup throw. If a smaller stopgap would help land the fix for the active incident sooner, I have a bounded tolerate-then-recover version with a regression test and before/after proof that I am happy to share or open as an alternative. Whatever works best for you.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[vincentkoc]

@clawsweeper re-review

Please review exact head 04d30dca285b96e87dbc1f76511a261cc7cb1d12. The old broad cleanup rewrite has been replaced entirely by the narrow scratch-local Go module-cache producer fix, with current proof and prior-finding disposition in the PR body.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

[vincentkoc]

@clawsweeper re-review

Please review exact head a6d894595a782e12186cfdfa0ffb9370886ee3d8. The broad cleanup rewrite remains fully removed. This head contains only the scratch-local GOMODCACHE, inherited GOFLAGS plus appended -modcacherw, and the focused environment regression test. The current real-behavior proof and prior-finding disposition are in the PR body.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/32826405025
• Updated: 2026-08-25T08:24:19.496Z

[vincentkoc]

@clawsweeper re-review

Please review exact head e5c34566436448ad33dba6e0499f173ec8e39e55 and the current PR body. The branch remains producer-only: scratch-local GOMODCACHE, inherited GOFLAGS composed with an authoritative trailing -modcacherw, and focused direct coverage. No chmod, cleanup retry, timeout, tmux/socket lifecycle, or process ownership change is included.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/32828352261
• Updated: 2026-08-25T08:46:31.507Z