Skip to content

fix: stop failing live proof before finite commands finish - #1278

Merged
steipete merged 4 commits into
mainfrom
fix/live-proof-process-completion
Aug 28, 2026
Merged

fix: stop failing live proof before finite commands finish#1278
steipete merged 4 commits into
mainfrom
fix/live-proof-process-completion

Conversation

@steipete

@steipete steipete commented Aug 28, 2026

Copy link
Copy Markdown
Contributor
Additional instructions

This PR uses a branch in the same repository, which maintainers with repository write access can edit. GitHub reports maintainerCanModify: false; no fork permission setting was changed.

What Problem This Solves

Fixes an issue where ClawSweeper could report a failed live verification while a healthy finite command was still running. The observed OpenClaw migration probe declared terminalCompletion: exit_zero, but its output assertion stopped after 30 seconds, before the test file and reporter finished.

The original premise needs an important qualification: the normal Vitest reporter did emit both slow test names at file completion. The complete Linux log records 49,727 ms for the memory-test file, with batch/deep cases taking 24,499/24,869 ms. The focused run passed 125 tests with two existing skips. The defect was premature observation, not proof that those names never appear or that the migration failed.

This PR does not edit, review for landing, or change the limits of the OpenClaw migration PR.

Why This Change Was Made

Reuse the existing terminal completion owner rather than adding another timeout or supervisor. Expectations for finite commands now wait for authoritative exit zero and sealed, controller-observed output within the existing overall proof budget. This includes finite setup commands before a final long-running command; multiple expectations reuse the completed capture. The final ready_while_running command retains its existing readiness window, stability hold, and liveness checks.

The planner receives the existing effective terminal execution limit and the trusted fact that terminal standard I/O is a PTY, and must inspect the exact command/wrapper/reporter contract. It prefers final summaries for finite tests, distinguishes late slow-test-name reporting from progress, and reports a concrete harness limitation when a meaningful scenario cannot fit. A successful command never waives a missing assertion: the proof remains unverified with an assertion-mismatch diagnostic, not an inferred product defect.

Real-wrapper validation also exposed a small supervisor bug on macOS: inherited /dev/tty descriptors became unusable when Node started a detached child session. Child standard I/O now opens the already-bound concrete PTY path. PID/TTY/lease/nonce validation, capture, and cleanup ownership remain unchanged. The test process-discovery helper filters before capture so unrelated long host command lines cannot overflow its buffer.

No new configuration, dependency, schema, public verification format, heap allowance, or execution/test timeout. OpenClaw's original 256 MiB heap and 180-second case limits are untouched.

The final git diff --numstat origin/main...HEAD separates a small runtime change from its regression coverage: production TypeScript is 30 additions / 2 deletions across two files; tests are 200 additions / 34 deletions across five files. The production planner prompt is behavior-bearing text, counted separately at 19 additions / 3 deletions. Documentation and changelog account for 34 additions / 6 deletions. The larger test share exercises parsed plans through the driver/verifier and detached-child PTY behavior.

User Impact

Healthy finite commands can finish and provide their real result instead of failing an unrelated 30-second progress-text wait. Missing observations, nonzero exits, signals, expired budgets, and failed cleanup remain nonpassing. Detached Node subprocesses retain usable terminal descriptors on macOS.

OpenClaw Bay Impact

Bay is unaffected. The verification format, publication identity, and observer-only lifecycle surfaces are unchanged; this corrects existing execution semantics and planning guidance.

Documentation Impact

Updated the active docs/live-proof.md reference and production review prompt with completion-aware/reporting-aware planning, the existing execution limit, and concrete PTY descriptor semantics. The reference retains its review/publication-maintainer ownership and source-of-truth/update triggers. The unreleased ClawSweeper changelog entry now describes strict observation semantics rather than the retired missing-marker success behavior.

Evidence

Current mechanical refresh targets d5780abb16ffebe2bea30da382f4a0d4354f95a6, rebased onto 777f3211cd5beaf2dce3b3ecca08cd5d3a0818e8. The sole manual resolution was in docs/live-proof.md, preserving upstream pnpm 11 structured-version guidance alongside finite-completion, reporter, and PTY guidance. The original three commit messages are unchanged; their range-diff shows only integration context changes. A separate test-only integration commit aligns the upstream negative-case diagnostic and removes a redundant timing assertion; no runtime edits were made. This PR's production TypeScript files are byte-identical to ba73c412, including driver SHA-256 2b8656cfdd824b1c82b27aa92be229e1e4ae4223aed3aa66202e941313c05cfb. The full upstream range also brings the already-merged report parser/renderer fix from PR 1277; those two production files exactly match main. No runtime, schema, dependency, budget, heap, or Bay contract was changed by the refresh.

On the final tree, Node 24.20.0 and pinned pnpm 11.10.0 passed pnpm run build:all, pnpm run check:static (including format/docs checks), pnpm run lint, and diff checks. All 253 combined executor/context/prompt-policy/package-version/report cases passed, with zero failures or skips, in 40.318 seconds. The explicit-spec real detached-child PTY command below passed on committed head d5780abb16ffebe2bea30da382f4a0d4354f95a6: 1 test, 0 failures, 0 skips, in 9.768 seconds (selected case: 9.419 seconds). Fresh isolated Codex autoreviews of the uncommitted test correction and then the committed branch against origin/main both passed with no actionable P0 findings; mandatory reviewer isolation, guard, and outgoing-pack credential scanning were retained.

The first mechanically rebased run exposed two test integration issues (251/253 passed; a 215-case retry still failed the timing assertion). The package-version negative case correctly rejected the absent literal but expected the retired diagnostic. It now checks the canonical proof-plan assertion mismatch diagnostic while retaining failed status, unsatisfied expectation, actual pnpm display, child-failure checks, and all negative cases. The generic completion test redundantly required an exact sleep 6, although the production hold subtracts real elapsed recording time. Only that duplicate hold variable/assertion was removed; completed status, respawn, and Usage remain. The stronger frozen-clock test still verifies authoritative completion before the exact six-second hold, recorder finalization, and cleanup ordering. No tolerance, timeout, clock, or production behavior changed. The earlier Node 26.7.0 PTY proof also passed (8.501 seconds); those initial failed test runs are preserved as history, not counted as passing gates.

The previous hosted review/proof passed five selected cases and published its review for the prior head. The current-main refresh addresses its sole rank-up request; previous CI and hosted review are historical continuity, not approvals for this changed head/body. See current PR Checks for new-head status. No new CI or hosted-review success is claimed.

  • Before the fix, the delayed-summary regression failed at the 30-second output wait for all three forms: entry-only, setup plus run, and a finite command before a running server. The same parser/report-plan → driver → verification boundary passes after the repair.
  • Earlier publication-head focused planner/executor/prompt run: 170 passed, 0 failed, 0 skipped, in 6.740 seconds with Node 26.7.0 and the existing pnpm 11.10.0 pin. Earlier real PTY/scratch-environment sibling coverage exercised 22 cases; a stale diagnostic assertion was corrected and rerun together with the detached-child regression, both passing. Those real PTY runs preceded publication and were not rerun during this bounded publication pass. Nonzero exits after early success-looking output, missing markers, deadline expiry, stale panes, PTY output ordering, lease cleanup, and readiness liveness remain covered.
  • pnpm run build:all, pnpm run check:static (including documentation and formatting), pnpm run lint, and git diff --check origin/main...HEAD passed on that earlier publication head. An initial corepack pnpm run build:all invocation stopped before compilation because nested pnpm resolved a conflicting global version; retrying with direct pnpm, which resolves the repository's existing 11.10.0 pin, passed without source, configuration, or dependency changes.
  • Fresh isolated precommit and committed-range Codex autoreviews found no actionable P0 defects. Review did not execute tests; the executions above are separate evidence.
  • Real OpenClaw wrapper replay was repeated on the clean published ClawSweeper head cb1fbbd0fa2856ad73555b29b15b6a95c9f233ff: delayed default-summary proof passed in 43.371 s, the deliberately missing fast-test-name assertion was correctly rejected despite 11 passing tests, and explicit-verbose proof passed in 6.320 s. No heap, test timeout, execution budget, or cleanup limit changed.
  • The first hosted full check completed 3,951 tests: 3,942 passed, eight existing skips, and one obsolete prompt-example regex failed. The test-only follow-up removes that illustrative wording assertion, retaining schema/safety checks and the actual planning/execution regressions; both affected prompt suites then passed 72 tests. The corrected full CI run then passed: 3,943 passed, eight existing skips, zero failures. The previous-head CI run passed 3,950 tests with eight existing skips and zero failures on the synthetic merge of a2f044f819 and ba73c412; it covers the trusted-PTY-context correction and is historical continuity, not CI for the refreshed head.
  • The prior full local pnpm run check did not pass: an untouched action-ledger opposing-parent-edge concurrency test stalled and the owned run was stopped. An independent helper-repair lane reproduced the same local stall. That same unchanged case passed in the hosted full check in 397 ms. The local hang remains separately owned; no ledger code, timeout, or assertion was changed here.

Previous publication validation targeted head ba73c4126a46a5d91d009ebaeda85a4ff61c7919 on base af8f89fe69e1e35009591a640c55d12a4a083cf9. That earlier rebase was conflict-free and preserves the separate invalid-live-proof-artifact refresh on main. Follow-ups after the cb1fbbd real-wrapper replay remove one obsolete example assertion and add the trusted PTY planning fact with documentation/tests; production driver bytes still match the proof hash below. The PTY fact regression was run red before its producer fix; all 215 focused executor/context/policy tests, builds, static checks, and lint then passed, as did fresh precommit review. The explicit-spec real-PTY command below passed on that previous head. Exact-head CI, fresh committed review, and the current ClawSweeper disposition control landing; no stale review or failed check is treated as a pass.

Current focused command:

node --test --test-reporter=spec test/live-proof.test.ts test/review-prompt-context.test.ts test/review-prompt-policy.test.ts test/live-proof-package-version.test.ts test/live-proof-report.test.ts

Earlier real PTY sibling command (not rerun in the publication pass):

node --test test/live-proof-review-environment.test.ts

Real Behavior Proof

Narrow hosted verification target

The ClawSweeper code reviews found no code or security defects and judged the supplied process proof sufficient. The first automated probe nevertheless selected the entire multi-minute suite. The next hosted probe selected the correct real-PTY case, which passed in 2.991 seconds with a successful command exit, but mixed a TAP-specific expectation with Node's normal spec-reporter output. That formatting mismatch is not a product or test failure.

This exposed one missing producer fact: the trusted planner context now explicitly supplies terminalStdio: "pty", alongside the existing execution budget. Reporter defaults can differ from redirected logs, and wrappers can pipe their own child streams. The extended context regression first failed because this fact was absent and also rejects PR-authored claims that standard I/O is a pipe.

For a bounded hosted run, this exact command selects one real production-driver, detached-child PTY case, rather than every setup/environment fixture. It previously passed on head ba73c4126a46a5d91d009ebaeda85a4ff61c7919; the selected case took 11.291 seconds locally, after the build. The spec reporter is explicit. Use exactly these format-independent observations: terminal proof preserves inherited PTY descriptors in a detached Node child and pass 1, with terminalCompletion: exit_zero. Cosmetic reporter prefixes are not part of the assertions. No fake marker, repeat invocation, timeout increase, or proof override is needed.

pnpm run build:all && node --test --test-reporter=spec --test-name-pattern='terminal proof preserves inherited PTY descriptors in a detached Node child' test/live-proof-review-environment.test.ts

The same selected real-PTY command was rerun successfully on current head d5780abb16ffebe2bea30da382f4a0d4354f95a6 with Node 24.20.0, taking 9.768 seconds overall. The recorded successful output contains the selected test name, pass 1, fail 0, and skipped 0; its decorative prefix is deliberately excluded from the expected substrings.

The test launches a real detached Node child through the production driveTerminal and mediaProofCommandRunner, verifies character-device/TTY stdin, stdout, and stderr plus clean child exit, observes real output, and checks process/capture cleanup. The delayed-output change itself additionally has the real 35-second before/after and real OpenClaw-wrapper evidence below. This addresses the review's request for after-fix terminal proof without asking the old outer driver to finish a multi-minute suite within its 30-second observation window.

Completed terminal and wrapper proof

Claim and surface: the production terminal driver waits for a finite command's real completion and output, without turning successful exit alone into proof. Controlled local macOS execution used private tmux sockets, an isolated HOME/TMPDIR for the OpenClaw replay, no provider/channel credentials, and no recording or external media upload.

Before/after fixture: a harmless finite process emitted startup output, remained quiet for 35 seconds, then emitted its final summary and exited zero. The pre-fix production driver failed its 30-second expectation; the repaired driver passed in 40.804 seconds including supervision/cleanup, using the unchanged 90-second proof budget. The fixture bytes were identical before/after (SHA-256 bd0f6fec467eae871c1bf18a50ecf879f7399b50f7f7e1bca0cf930c6a7507f7).

Real OpenClaw wrapper replay: plans were parsed and executed through the production terminal driver/verifier from the clean published ClawSweeper head cb1fbbd0fa2856ad73555b29b15b6a95c9f233ff, against OpenClaw 9458d8b18105a9aa18ef334d50c6974c83f55acd, Node 24.20.0, tmux 3.7c, and Vitest 4.1.11. Each command ran the real repository wrapper with its original test behavior:

OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/commands/doctor-format.test.ts --reporter=default
Scenario Observed result
Default reporter, with an explicit 35-second startup-delay fixture before the wrapper 11 tests passed; final Test Files, Tests, and [test] passed assertions satisfied after process completion; whole proof 43.371 s
Default reporter, intentionally requesting a fast test's unprinted name 11 tests passed, but the proof correctly failed as a proof-plan assertion mismatch; exit zero did not waive it
Explicit --reporter=verbose, requesting the emitted name plus final wrapper summary 11 tests passed; both observations satisfied; whole proof 6.320 s

Representative captured default-reporter result:

Test Files  1 passed (1)
     Tests  11 passed (11)
[test] passed 1 Vitest shard in 3.59s

Additional integration evidence: a separate helper-repair lane replayed actual Codex-generated fast and 34-second delayed plans against this exact driver source (SHA-256 2b8656cfdd824b1c82b27aa92be229e1e4ae4223aed3aa66202e941313c05cfb). Both passed seven controls: generated success, duplicate invocation rejection, intentional rerun, exit 7, zero exit with missing output, finite deadline, and readiness timeout. That integration covers the driver, not this PR's updated planner prompt/runtime context; those have separate focused coverage. The helper changes are not included here.

Limits: this is real local process/PTY and wrapper proof, not a new migration, authenticated-provider, browser, media, or production-deployment test. Initial concurrent macOS PTY attempts hit the existing cleanup cap; serial replays passed without raising it. Raw host logs and agent transcripts are not published. Public execution evidence is the sanitized observations above; the existing terminal verification artifact schema is unchanged.

@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

@steipete
steipete marked this pull request as ready for review August 28, 2026 06:47
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 28, 2026
@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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

ClawSweeper review

What this changes

The PR makes finite terminal proof commands wait for their configured completion budget before evaluating output assertions, and preserves usable PTY standard I/O for detached child processes.

Regression provenance

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

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep open: this is a necessary, narrowly scoped repair to terminal live-proof execution, and the reviewed introduced changes have no concrete correctness or security blocker.

Priority: P2
Reviewed head: d5780abb16ffebe2bea30da382f4a0d4354f95a6

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) PR readiness rating was derived from proof quality, review findings, security review, and reviewer confidence.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): ClawSweeper live verification passed on the reviewed terminal surface.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): ClawSweeper live verification passed on the reviewed terminal surface.
Evidence reviewed 5 items Introduced completion handling: The branch changes terminal expectation evaluation so commands that must exit successfully are awaited through the existing terminal deadline; absent expected output still fails with an assertion-mismatch diagnostic.
Regression coverage: Focused coverage exercises entry-only, setup-plus-run, and finite-before-ready plans with a summary emitted after 35 seconds, as well as a real detached Node child retaining all three PTY descriptors.
Clean introduced range: The verified PR range contains 10 focused files and passes whitespace validation; its runtime changes are confined to the live-proof driver and trusted review context.
Findings None None.
Security None None.

Live Verification

Command: pnpm run build:all && node --test --test-reporter=spec --test-name-pattern='terminal proof preserves inherited PTY descriptors in a detached Node child' test/live-proof-review-environment.test.ts

Result: PASS (completed)

$ pnpm run build && pnpm run build:repair && pnpm run build:dashboard
$ tsc -p tsconfig.json
$ tsc -p tsconfig.repair.json
$ tsc -p tsconfig.dashboard.json
✔ terminal proof preserves inherited PTY descriptors in a detached Node child (2872.570605ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 3102.160497





































Assertions:

  • PASS expect_output: terminal proof preserves inherited PTY descriptors in a detached Node child
  • PASS expect_output: pass 1

How this fits together

ClawSweeper's live-proof driver runs a reviewed repository command in a controlled terminal and turns its output, exit state, and cleanup result into a verification record. The driver feeds the review/publication workflow, while the prompt context tells the planner what terminal behavior and budget it may rely on.

flowchart LR
A[Reviewed proof plan] --> B[Terminal proof driver]
B --> C{Completion contract}
C -->|Finite command| D[Exit status and sealed output]
C -->|Long-running command| E[Readiness and liveness]
D --> F[Verification result]
E --> F
D --> G[Cleanup watchdog]
E --> G
Loading

Before merge

  • Resolve merge risk (P1) - This changes shared proof-execution timing and terminal I/O; a regression could affect automated verification jobs, so the still-running exact-head validation should complete before landing.
  • Complete next step (P2) - No narrow repair is identified; this PR needs normal current-head validation completion and maintainer review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Runtime and regression coverage production +30/-2, tests +200/-34 The small executor/context change is accompanied by targeted completion, assertion, and detached-PTY coverage.
Changed surface 10 files affected The diff includes the runtime behavior, planner guidance, documentation, and focused matching tests.

Merge-risk options

Maintainer options:

  1. Finish exact-head automation validation (recommended)
    Let the current-head build, static checks, and test gate complete before merge because this patch changes the terminal verification executor.
  2. Pause if executor validation regresses
    Keep the PR open for a focused correction if current-head validation exposes a completion, cleanup, or PTY portability failure.

Technical review

Best possible solution:

Retain the existing overall proof budget while using authoritative finite-command completion and sealed output for exit-zero plans, with readiness/liveness behavior unchanged for long-running plans.

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

Yes—the source and supplied proof describe a deterministic terminal-driver scenario where a finite command emits its required summary after the former 30-second observation window; this review did not execute it.

Is this the best way to solve the issue?

Yes—the branch reuses the existing completion and budget owner instead of introducing a second timeout, while retaining failure for nonzero exits and missing output.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 777f3211cd5b.

Labels

Label justifications:

  • P2: This is a bounded reliability repair in ClawSweeper's proof workflow without evidence of current data loss, security bypass, or core unavailability.
  • merge-risk: 🚨 automation: The introduced driver logic controls how automated live verification waits for commands, captures output, and cleans up terminal processes.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): ClawSweeper live verification passed on the reviewed terminal surface.
  • proof: sufficient: Contributor real behavior proof is sufficient. ClawSweeper live verification passed on the reviewed terminal surface.

Evidence

What I checked:

  • Introduced completion handling: The branch changes terminal expectation evaluation so commands that must exit successfully are awaited through the existing terminal deadline; absent expected output still fails with an assertion-mismatch diagnostic. (src/live-proof/drivers.ts:1077, d5780abb16ff)
  • Regression coverage: Focused coverage exercises entry-only, setup-plus-run, and finite-before-ready plans with a summary emitted after 35 seconds, as well as a real detached Node child retaining all three PTY descriptors. (test/live-proof.test.ts:1183, d5780abb16ff)
  • Clean introduced range: The verified PR range contains 10 focused files and passes whitespace validation; its runtime changes are confined to the live-proof driver and trusted review context. (src/live-proof/drivers.ts:723, d5780abb16ff)
  • Feature history: History shows the live-proof driver was introduced and repeatedly maintained in the recent live-proof series; this branch's finite-completion and PTY-context commits continue that same surface. (src/live-proof/drivers.ts:723, 3b00eca97551)
  • Supplied real behavior proof: The PR body supplies an exact-head detached-child terminal run and a controlled before/after finite-command scenario using the production driver; the available excerpts identify the unchanged proof budget and observed post-fix completion. Omitted body ranges were not treated as negative evidence. (test/live-proof-review-environment.test.ts:462, d5780abb16ff)

Likely related people:

  • steipete: Recent history attributes the original live-proof feature and the finite-completion/PTY-context work to this contributor. (role: live-proof feature owner and recent contributor; confidence: high; commits: 03c1f452ec49, 3b00eca97551, 29b1b6abff81; files: src/live-proof/drivers.ts, src/clawsweeper-review-runtime.ts)
  • Vincent Koc: The recent authoritative-terminal-results change is in the same driver history and is relevant to completion-status ownership. (role: adjacent terminal-result contributor; confidence: medium; commits: d103c0cf5c98; files: src/live-proof/drivers.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 (5 earlier review cycles)
  • reviewed 2026-08-28T06:51:18.640Z sha cb1fbbd :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-28T07:08:13.796Z sha ee0cdf6 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-28T07:25:27.719Z sha ee0cdf6 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-28T07:51:44.210Z sha ba73c41 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-28T08:34:12.513Z sha ba73c41 :: needs maintainer review before merge. :: none

@steipete

steipete commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Final head: ba73c4126a46a5d91d009ebaeda85a4ff61c7919. The latest follow-up exposes the host-owned terminalStdio: "pty" fact to planning and covers it with a failing-before/passing-after poisoned-context regression. All 215 focused tests, builds, static checks, and lint pass. The production terminal driver is unchanged from the completed delayed-command and real-wrapper proof in the main PR body.

The bounded real-PTY command passed on this head. For this demonstration, use this exact plan; its observations deliberately exclude cosmetic reporter prefixes:

{
  "status": "recommended",
  "surface": "terminal",
  "terminalCompletion": "exit_zero",
  "reason": "The selected regression executes the real terminal driver and verifies detached-child PTY descriptors and cleanup.",
  "payoff": {"kind": "static_text", "justification": "The observed test result is a concise terminal summary."},
  "entry": "pnpm run build:all && node --test --test-reporter=spec --test-name-pattern='terminal proof preserves inherited PTY descriptors in a detached Node child' test/live-proof-review-environment.test.ts",
  "steps": [
    {"action": "expect_output", "text": "terminal proof preserves inherited PTY descriptors in a detached Node child"},
    {"action": "expect_output", "text": "pass 1"}
  ]
}

This retains both a real observed outcome and successful terminal completion; no timeout, heap limit, missing-output check, or proof gate is relaxed. The main body includes the full before/after evidence and final-head CI.

@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@clawsweeper clawsweeper Bot added merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 28, 2026
@steipete
steipete force-pushed the fix/live-proof-process-completion branch from ba73c41 to d5780ab Compare August 28, 2026 08:54
@steipete

Copy link
Copy Markdown
Contributor Author

Land-ready verification for d5780abb16ffebe2bea30da382f4a0d4354f95a6.

The current-head ClawSweeper review reports no correctness/security findings, sufficient real-behavior proof, and live_verification=passed. Its two remaining items ask for current-head validation to finish and a maintainer landing decision. Both are addressed: validation is green, and the requested maintainer landing is proceeding without a proof override or admin bypass.

  • Final CI: full pnpm check passed 3,981 tests, eight existing skips, zero failures. Windows launcher and sparse repair smoke passed; both CodeQL analyses are green.
  • CI's checkout log proves it tested merge dff35d6dd7e1e9434cb4933dd222e876b32c5de6, with current main 777f3211cd5beaf2dce3b3ecca08cd5d3a0818e8 and this exact PR head as parents. The refreshed GitHub merge tree matches a conflict-free local git merge-tree; current main's own CI is green, GitHub reports CLEAN/MERGEABLE, and the reviewed PR body is unchanged.
  • Final hosted review/proof ran the real detached-child PTY case successfully (one pass, zero failures/skips; selected case 2.873 seconds). Both the selected test-name and pass 1 observations were satisfied after exit-zero completion.
  • Local refreshed build, static/format/docs checks, lint, 253 focused cases, and the real macOS PTY case passed. Fresh isolated/scanned precommit and committed-range Codex autoreviews reported no actionable P0 findings.

The hosted command was:

pnpm run build:all && node --test --test-reporter=spec --test-name-pattern='terminal proof preserves inherited PTY descriptors in a detached Node child' test/live-proof-review-environment.test.ts

The combined local boundary/sibling command was:

node --test --test-reporter=spec test/live-proof.test.ts test/review-prompt-context.test.ts test/review-prompt-policy.test.ts test/live-proof-package-version.test.ts test/live-proof-report.test.ts

The main body retains the real 35-second before/after fixture and actual OpenClaw-wrapper positive/negative/verbose replays. The original slow-test names arrived at file completion; the old 30-second observation stopped too early. This fix reuses the existing completion/budget owner. Missing output after exit zero remains nonpassing; readiness/liveness, cleanup fencing, heap limits, test timeouts, and the proof budget are unchanged.

Integration with the newer pnpm-output fix required a documentation-only conflict resolution plus two test corrections: align the new negative case's diagnostic and remove a redundant wall-clock-dependent sleep assertion while retaining the stronger deterministic timing/cleanup test. The terminal driver remains byte-identical to the completed process proof (SHA-256 2b8656cfdd824b1c82b27aa92be229e1e4ae4223aed3aa66202e941313c05cfb). Final production TypeScript is +30/-2 (net +28), tests/test support +200/-34; the added runtime lines serve completion selection and trusted planner facts rather than another timeout or lifecycle path.

The unrelated local action-ledger concurrency hang remains separately owned and honestly disclosed; its unchanged case passed hosted Linux CI. Publication recovered through the normal queue—no workflow disabling, manual label, credential substitution, or gate override was used. Bay is unaffected. This PR does not modify or merge the OpenClaw migration PR.

@steipete
steipete merged commit d56daf0 into main Aug 28, 2026
10 of 11 checks passed
@steipete
steipete deleted the fix/live-proof-process-completion branch August 28, 2026 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant