test: isolate and clean temporary test roots - #1099
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Codex review: needs maintainer review before merge. Reviewed August 12, 2026, 7:32 PM ET / 23:32 UTC. ClawSweeper reviewWhat this changesThis PR runs named Node test targets inside unique temporary roots, cleans fixtures and stale roots safely, and routes selected package and Windows CI commands through that shared runner. Merge readinessKeep open for maintainer review. This member-authored PR is not eligible for automated closure; current main does not yet contain its temporary-root lifecycle, and no discrete correctness defect was found in the current branch diff. Priority: P3 Review scores
Verification
How this fits togetherClawSweeper’s Node test runner expands named test targets and starts Node’s test process for local commands and CI. The change supplies that process with an isolated temporary root and removes it after completion, failure, or termination. flowchart LR
A[Package scripts and CI] --> B[Named test target]
B --> C[Shared test runner]
C --> D[Per-run temporary root]
D --> E[Node test process]
E --> F[Cleanup and stale-root reaper]
Decision needed
Why: The patch changes shared test and CI execution behavior; repository policy requires explicit maintainer judgment rather than automated closure for member-authored work. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the isolated runner lifecycle only after a maintainer accepts the shared-test-environment tradeoff and confirms the current-head CI and submitted lifecycle evidence remain applicable. Do we have a high-confidence way to reproduce the issue? Not applicable as a user bug report; the PR body supplies a high-confidence real-run lifecycle verification path for the test harness. Is this the best way to solve the issue? Yes. Centralizing temporary-root isolation in the existing cross-platform named-target runner avoids duplicating cleanup behavior across package scripts and CI. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9a257905e50b. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
4b99634 to
1aa53a6
Compare
|
Local black-box validation completed against commit Environment:
Evidence:
No pre-existing historical temporary directories were deleted during this validation. The PR CI |
…ck finalization (#1109) Three repair lanes rolled up PR status checks independently, and the finalizer's copy had a live bug: it processed every check entry instead of deduplicating to the newest run per check identity, so an older failed run of a re-run check remained a blocker after the newer run succeeded. The finalizer's classifications feed the published repair queue via repair-publish-results, making stale failures veto finalization. All three lanes now share src/repair/status-check-rollup.ts (latest-run selection, ignored-name parsing, casing and timestamp precedence implemented once); lane policy stays in the callers. Proof (docs/proof/status-check-rollup): a red-then-green regression test, and a real-payload production-boundary trace -- the identical real 26-entry check payload from PR #1099 classified by both implementations: pre-change code reports two stale CANCELLED blockers, this head reports zero, and the real report-only finalizer confirms needs_checks=0. Verified in a Docker-backed Crabbox local-container with committed provenance. Automation-risk disposition: the newest run of a check identity is its current state (the rule the router and post-flight lanes already applied); blast radius is finalizer classifications in the published repair queue; rollback is a revert restoring per-entry processing.
Summary
Repeated test runs can leave fixture directories behind in the shared system temporary directory. Some fixtures intentionally contain non-regular files such as FIFOs, which can also cause content-inspecting host cleanup tools to block indefinitely.
This change keeps the FIFO safety coverage while tightening the test lifecycle:
The tracker follows the established OpenClaw auto-cleanup temp-directory pattern, adapted locally so ClawSweeper does not depend on another repository's test helpers.
Review follow-up
Validated at head
a602f2ba67.target-validationis now a canonicalrun-node-tests.mjstarget. Validation fixtures created through the supported narrow-test command inherit the run root, so a SIGKILL, crash, or reboot leaves aclawsweeper-test-run-*root that the existing heartbeat/TTL reaper recognizes. The rawnode --test ... target-validation.test.tscommand is no longer used as the recommended evidence path.test/codex-process.test.tstwice; the named target remains the single source of file selection.Local black-box validation
Linux, Node
v24.15.0. Corepack resolved the repository pin to pnpm11.10.0.Runner and lifecycle behavior
Result: 10 passed, 0 failed. This covers named-target selection plus successful FIFO cleanup, assertion-failure cleanup, stale-root TTL cleanup, and SIGTERM cleanup through the real CLI runner.
Incident-equivalent FIFO fixture through the canonical target
Result: 1 passed, 0 failed; FIFO behavior completed in 19.75 ms. Matching counts were unchanged:
Repeated full target validation through the canonical target
Executed twice:
Results:
clawsweeper-test-run-*:0 -> 0 -> 0clawsweeper-validation-*:0 -> 0 -> 0Repository checks
corepack pnpm run check: static checks, formatting, builds, lint, and changed coverage passed; the final full-coverage run failed only in threetest/review-blob-hydration.test.tspartial-clone hydration cases, which returnedhydrated: false/blobs: 0in this environment. The same failures reproduce when that file is run directly outside the new runner; this PR does not modify that test or hydration code. The current-head hostedpnpm checkpassed: https://github.com/openclaw/clawsweeper/actions/runs/31464912192/job/93695706262.origin/mainafter commit: clean; TruffleHog cleanScope boundary
This change affects only test fixture and test-runner lifecycle. It does not change OpenClaw Bay state, queues, sweep workflows, publication behavior, or dashboard contracts. It does not downgrade pnpm and does not delete host-level pnpm, npm, or Docker caches.