fix(live-proof): unblock Crabbox setup and terminal cleanup - #1272
Conversation
Select Crabbox's npm worker setup through its existing repository profile instead of running the owner fallback's pnpm install at a manifest-less root. Retain bounded stderr, stdout, and spawn-error evidence so an informational download notice cannot hide the actual setup failure. Limit Darwin lease discovery to descriptors/fileports and query the bound TTY directly, avoiding unrelated host scans without changing cleanup identity checks, sweeps, or budgets. Replace an unrelated mixed-clock apply fixture assertion with direct sleep observation and exact yield outcomes. Proven with real trusted npm/PTY fixtures, negative controls, the full local gate (3,865 passed, zero failed, nine platform skips), and independent Codex review. No target Crabbox code, tests, changelog, permissions, or gates change. Context: openclaw/crabbox#1557 (comment)
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| writeFileSync(clockHookPath, `Date.now = () => ${Date.now()};\n`, "utf8"); | ||
| writeFileSync( | ||
| clockHookPath, | ||
| `Date.now = () => ${Date.now()};\n${applySleepObserverPreload(sleepTracePath)}`, |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 27, 2026, 9:53 PM ET / August 28, 2026, 01:53 UTC. ClawSweeper reviewWhat this changesThe PR adds a Crabbox-specific nested npm setup profile, preserves both stdout and stderr in live-proof failures, and narrows terminal-cleanup process discovery. Merge readinessKeep this PR open: current main still lacks the Crabbox-specific setup profile, and the introduced repair is a focused, evidence-backed fix with no actionable correctness finding. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherClawSweeper materializes an exact PR head into an isolated checkout, uses its repository profile to install dependencies, then executes and publishes a bounded live-proof result. Its terminal driver also cleans up the proof process tree after execution. flowchart LR
A[Review artifact] --> B[Repository profile]
B --> C[Cold target checkout]
C --> D[Target-native setup]
D --> E[Live proof command]
E --> F[Verification receipt]
E --> G[Terminal cleanup watchdog]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Retain the explicit Crabbox profile and focused regression coverage, then land the repair after the exact-head check completes. Do we have a high-confidence way to reproduce the issue? Yes—source establishes that current main routes Crabbox through the generic pnpm fallback, while the PR’s exact-head fixture exercises the required nested npm setup and verification path. Is this the best way to solve the issue? Yes—the explicit repository profile is the narrowest maintainable fix because it preserves the generic fallback while declaring Crabbox’s known nested package layout. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 78540e0458bf. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Landing verification complete for The current ClawSweeper review matches this head and the unchanged PR body. It reports no actionable findings or Rank-up moves, with Local validation used Node 24.20.0, pinned pnpm 11.10.0, npm 11.19.0, Bash 5.3.15, and tmux 3.7c: pnpm build:allpnpm run checknode --test test/apply-runtime-budget.test.tsThe final full gate passed 3,865 tests, zero failures, nine platform skips, plus its separate 12-test changed-coverage stage. The apply-budget file passed 11 tests. Earlier setup/cleanup/fixture failures and the negative-control driver's over-specific prediction remain documented in the PR body; no budgets, coverage thresholds, or gates were relaxed. Fresh isolated Codex reviews passed both before commit and on the committed branch at the configured P0 threshold. ClawSweeper's own fresh live verifier additionally executed the following command successfully, observing both the real nested npm child's success and its intentional exit-7 rejection: pnpm run build:node && node --test --test-name-pattern='Crabbox profile bootstraps a trusted synthetic Go-root/nested npm fixture through the review child' test/live-proof-review-environment.test.tsThe repair stays in repository-profile selection, bounded diagnostics, and cleanup-query ownership. Runtime TypeScript is +16/-8; configuration +20; tests +418/-74; documentation +29/-5. Source tree and tested blobs were verified unchanged through commit and publication. Limits remain explicit: trusted synthetic target proof, not another run of Crabbox's actual Node/PostgreSQL or managed-provider services; no fresh recording claim; environment filtering is not kernel isolation. No original Crabbox code, test, changelog, review comment, permissions, or gates were changed. OpenClaw Bay is unaffected. @steipete explicitly approved landing exact head |
2 participants
Related: openclaw/crabbox#1557 (comment) — verification stopped before the requested Node runtime test executed.
Additional instructions
MUST: Keep Allow edits from maintainers enabled for this PR so maintainers can help update the branch when needed.
What Problem This Solves
Fixes an issue where maintainers reviewing Crabbox would receive a failed live-verification result before the requested worker test ran. The published reason showed only Corepack's download notice, hiding the actionable setup error.
The exact receipt came from https://github.com/openclaw/clawsweeper/actions/runs/33059981352, ClawSweeper source
13f77495c4620d3a783eb0b055f9a18723027a7e, reviewing Crabbox2477578b9cf3e6767bd6cb9bd0f0fdd06067c386. All four files in its artifact inventory were verified against recorded byte lengths and SHA-256 hashes. The requested command wasnpm test --prefix worker -- test/node-runtime.test.ts; the receipt marks both assertions asnot_run.Why This Change Was Made
The bootstrap correctly materializes the reviewed head into a scratch target checkout and uses a private HOME/cache. But Crabbox had no explicit repository profile, so it inherited an owner-level
pnpm install --frozen-lockfileat the target root. That root has no package manifest; Crabbox's npm manifest and lockfile live underworker/. Node 24 was appropriate. The repair selects npm andnpm ci --prefix workerthrough the existing repository-profile boundary, rather than adding package discovery, changing the target, or bypassing Corepack prompts.The existing setup normalizer still adds
--ignore-scripts. Close rules, validation commands, changed-gate settings, plan gates, environment filtering, and execution/publication contracts remain unchanged. Separately, the shared failure-detail producer now reserves bounded space for stderr, stdout, and any spawn error instead of selecting the first nonempty stream. Flattening that summary preserves both streams in one-line verification reasons.The fresh current-main proof exposed a connected terminal cleanup blocker on a busy Darwin host. Stock
lsofrepeatedly scanned mapped images and working directories, and TTY discovery scanned all host processes. The watchdog was alive and the target had exited zero, but discovery consumed the existing cleanup window. Darwin now uses documentedlsof -Xdescriptor/fileport discovery; TTY enumeration selects the already-bound terminal withps -t. Duplicate lease descriptors on Darwin, exact identity checks, pre-signal revalidation, watchdog receipts, sweeps, and budgets are unchanged. Linux's inherited-fd contract is unchanged.One independent test-only correction replaces a parent wall-clock assertion while its CLI clock was frozen. The fixture now directly observes forbidden sleeps and verifies exact pre/post-close reports, command counts, archives, and cursors. A deterministic shared-budget case checks elapsed time, the report-flush reserve, equality, and scope restoration. Production apply behavior and fixture budgets are unchanged.
User Impact
Crabbox verification can reach the declared worker command after target-native dependency setup. When setup or another proof command fails, informational stderr no longer suppresses a useful stdout cause. This PR does not change Crabbox runtime behavior, tests, permissions, or release notes.
The related release-owned changelog finding was a separate wrong-target policy injection. It is already fixed by #1261. No target changelog entry is removed here, and the old Crabbox public review is not rerun or rewritten.
OpenClaw Bay Impact
None. No lifecycle, queue, public schema, status, route, or observer contract changes. The existing verification schema and publication path remain in use.
Documentation Impact
Updated the active
docs/live-proof.mdanddocs/target-repositories.mdreferences. They document target-root setup CWD, explicit nested-package commands, repository-specific toolchain ownership, bounded multi-stream failure summaries, and descriptor/TTY-scoped cleanup discovery. ClawSweeper maintainers own these references; the source of truth remains repository profiles and the live-proof executor, diagnostic producer, and terminal driver. Revisit them when those contracts change.Evidence
Validated commit:
5954ad0dcfd09209269d80f9494395ad627960cbValidated base:
6230a9d7c8b4bb103a6872de63add3f7e3b77701.Native macOS validation used Node 24.20.0, repository-pinned pnpm 11.10.0, npm 11.19.0, Bash 5.3.15, and tmux 3.7c. The generated terminal wrapper/watchdog use the system
/bin/bash; selecting modern PATH Bash only supplies the unrelated workflow test prerequisite.pnpm run checkon the final frozen patchnode --test test/apply-runtime-budget.test.tspnpm build:all, lint, formatting, docs/limits checks,git diff --checkFinal full-check coverage: 82.22% lines, 74.70% branches, 87.68% functions. The pre-commit diff SHA-256 remained
ab871792539af5268ab227749ee74be20b4001c8b591b6682206a019b3bf4c69throughout full validation and review. Source, assertions, suite concurrency, and coverage thresholds were not changed during the run.The original pre-fix regression run had eight passes and six failures: the profile selected pnpm instead of npm, and browser/terminal setup failures lost stdout evidence behind stderr. The six failures include the parent test's failure accounting; they are not six independent defects.
Earlier full-check attempts are retained as history, not represented as passing proof: macOS Bash 3.2 lacked
mapfile; the same workflow test passed with installed Bash 5. Two old deadline fixtures later failed when a shared budget expired before an assumed subprocess phase. The current base includes the phase-based fixture repair from #1270. This PR neither changes those tests nor increases budgets or changes assertions to accommodate them.The first refreshed full check then had 3,847 passes, 16 failures, and 10 platform skips: 15 terminal cleanup failures plus the parent-clock apply assertion. These were diagnosed and repaired as described above; the failures are not discarded as retries.
Runtime TypeScript: +16/-8 (net +8), comprising bounded multi-stream diagnostics and three cleanup-query substitutions plus their invariant comments. Configuration: +20, for the explicit existing-contract Crabbox profile. Tests: +418/-74. Docs: +29/-5. No new dependency, configuration option, or production abstraction is introduced.
Real Behavior Proof
Claim and exercised surface: the real
executeReviewLiveProofsowner materializes the exact reviewed commit, starts its normal child, resolves the authoritative Crabbox profile, runs official npm against the nested worker package, and produces a verification result without weakening install-script, environment, or exit-status controls. The diagnostic producer also retains the actionable stdout from an actual cold Corepack failure.Environment and scenario: native macOS, Node 24, repository-pinned pnpm 11.10.0, npm, tmux, and modern Bash. The executed target is trusted synthetic source: a Go-style root without
package.json, aworker/package.jsonand frozen lock, an offline local dependency, pre/postinstall hooks that would leave a marker, and root/worker verification scripts. No Crabbox source, credentials, provider, PostgreSQL server, or cloud resource is executed or allocated by this proof. The source fixture is dirtied after its commit to ensure the child uses the recorded head, not current filesystem contents.Command: after building, execute the real-child scenario and its negative exit-status leg:
node --test --test-name-pattern='Crabbox profile bootstraps a trusted synthetic Go-root/nested npm fixture through the review child' test/live-proof-review-environment.test.tsInside the child the configured setup is normalized to
npm ci --ignore-scripts --prefix worker. Its entry executesnode verify.cjs root && npm test --prefix worker -- pass; a second independent exact-head child usesfailand exits 7.Observed result and normalized trace:
Both legs verify the recorded head, root and worker CWDs, installed local dependency, absent install-hook marker, stripped credential-name sentinels, private HOME/cache, unchanged lockfile, and untouched dirty source checkout. The negative leg prints its completion marker and then exits 7; verification must still fail, so a marker cannot mask a failed command.
Cold bootstrap reproduction: an empty root with only a nested worker manifest was run with Corepack 0.35.0, pnpm 11.24.0, piped stdin, and private HOME/cache. It exited 1 rather than waiting for confirmation:
Replaying those captured real streams through the repaired producer retains both the notice and manifest error within the existing 1,000-character cap. Corepack's actual interactive confirmation requires
stdin.isTTY && !CI; the notice alone is not proof of a prompt blockage. Historical raw stdout was discarded, so the exact historical stdout cannot be recovered.Cleanup before/after: an identical real printf/
exit_zeroplan failed before with an armed watchdog, held target exit 0, legitimate wrapper/sleep lease holders, and no completed cleanup receipt. Observedlsofcalls took 1.93–2.39 seconds; whole-hostpsscanned roughly 2,300 processes in 1.31 seconds versus 14–30 ms for bound-TTY selection. The complete repair produced the exactcontroller|ok|0receipt and the original pane's death. Total drive time changed from 19.831 seconds (failed) to 16.622 seconds (completed); these are total drive times, not cleanup-budget measurements. An intermediate descriptor-only change was insufficient; both discovery costs were addressed. All 137 terminal tests passed afterward, including duplicated/closed/unrelated descriptors, resistant descendants, pane death, and both npm-fixture outcomes.Apply fixture controls: the complete apply-budget file passed 11 tests. Deliberately inserting a sleep before either close guard was rejected immediately by the armed sleep observer. Ignoring elapsed time, dropping the flush reserve, and permitting the equality boundary each failed the corresponding deterministic assertion. A sixth control omitting pre-close admission was rejected by the report-shape assertion after another freshness guard intervened; the scratch driver's prediction of a close-command assertion was over-specific and failed. That attempt is retained as an observed rejection, not a clean driver run; neither source nor test assertions were changed to accommodate it. No real GitHub close occurred: the existing controlled transport fixtures were used.
Limits: this proves ClawSweeper's bootstrap, diagnostic, and native macOS cleanup boundaries, not a rerun of Crabbox's Node/PostgreSQL or managed-provider proof. Those successful target-specific runs remain documented in the original Crabbox PR. The synthetic child is unsandboxed with the existing sanitized scratch environment; environment filtering is not kernel isolation. This change does not expand source-trust permissions or claim to solve that separate containment boundary. No media or agent transcript is attached.