fix(review): avoid failures from multiline live-proof commands - #1268
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 automation
Contributor
|
Codex review: needs maintainer review before merge. Reviewed August 27, 2026, 8:03 PM ET / August 28, 2026, 00:03 UTC. ClawSweeper reviewWhat this changesThe PR adds single-line schema constraints and review guidance for live-proof commands, plus focused parser and prompt coverage. Merge readinessKeep open: this is a coherent, current fix for a real parser/prompt mismatch, and the updated body supplies after-fix structured-generation and CLI receipts that resolve the prior proof blocker. No introduced correctness or security defect was found. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherClawSweeper asks a model for structured review decisions that may include terminal live-proof plans. The shared prompt and JSON schema guide that output, while the decision parser validates it before the proof runner executes anything. flowchart LR
Prompt[Review prompt] --> Decision[Structured decision]
Schema[Decision schema] --> Decision
Decision --> Parser[Strict decision parser]
Parser --> Plan[Live-proof plan]
Plan --> Runner[Proof runner]
Runner --> Report[Review report]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep the schema, prompt, documentation, and parser boundary aligned so invalid multiline plans are prevented during generation and still rejected at parsing. Do we have a high-confidence way to reproduce the issue? Yes, with high confidence: current-main parser source rejects the reported separators, and the PR body records the prior real Codex heredoc output that reached that failure path. This review did not run a new model inference. Is this the best way to solve the issue? Yes. Constraining the active structured-output schema and prompt while retaining the parser as the final validation boundary is the narrowest maintainable fix. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d103c0cf5c98. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
Contributor
Author
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
Fixes an issue where an otherwise successful ClawSweeper review failed parsing when Codex generated a multiline heredoc in a live-proof command. The incident was observed during a local review of #1261 at head
14214b6f84a7be78785b24d0360080be3c368630: inference exited successfully and returned JSON with no code findings, but the decision parser rejectedliveProofPlan.entryas multiline. This is a separate follow-up, not a change to that PR or its repository-specific release-note policy.Why This Change Was Made
The shared decision schema and review template now express the existing parser contract: no literal CR, LF, U+2028, or U+2029 in
entryor terminalrun.command, including leading/trailing separators; run commands must be nonblank. Complex commands should use an existing script or a properly quoted single line. Ordinary anchored character classes are used because actual structured-output inference rejects regex lookaround.This is a cross-repository output-format contract, so the ownership belongs in the common schema/template rather than repository profiles or an additional maintainer prompt. The parser and terminal driver are unchanged relative to the current base. One-shot entry-plus-assertion plans, setup-plus-one-run plans, intentional repeated commands, cold-checkout prerequisites, authoritative terminal results, containment, and release/merge guards are preserved. Generated JSON is never rewritten to make it pass.
User Impact
Reviewers receive format guidance that prevents the observed heredoc mismatch without relaxing the safety checks that reject invalid generated plans. There is no new execution permission, fallback, retry, dependency, or schema-version change.
OpenClaw Bay Impact
None. This only aligns generation guidance with the existing decision parser; no observer route, lifecycle, publication, or dashboard data contract changes.
Documentation Impact
Updated the active
docs/live-proof.mdrunbook's command-format guidance and the changelog. ReviewedAGENTS.md,CONTRIBUTING.md, the documentation index, and the local-review documentation. Existing execution and release/merge policy remains accurate.Review Finding Disposition
The current review raises one P2 finding, "Provide the required local-container behavior proof," and repeats that request as its merge-risk and Rank-up item. There is no separate implementation finding.
Disposition: not applicable to the recorded proof host. The cited policy requires actual controlled behavior proof generally, then scopes the specified provider to a host:
The actual generation and CLI proof in this PR was performed on macOS, not a Windows host. A direct environment assertion is
{"platform":"darwin","arch":"arm64","node":"v24.20.0"}. The general real-behavior requirement is satisfied by the two actual Codex/CLI runs documented below, not merely by unit tests or CI. Their tested source was verified byte-identical to the committed head. The Windows-specific provider clause does not establish an all-platform Docker requirement; a future Windows-host run should follow that clause as written.The identical Rank-up request is therefore not applied to this macOS proof. This is a host-applicability disposition, not a proof waiver: no policy text, execution boundary, parser rule, merge guard, or
proof: overridelabel was changed. Re-review should evaluate the cited host condition and the existing actual runtime receipts. The code and all proof inputs are unchanged; this body update records the disposition and the now-complete CI result.Evidence
pnpm run checkcompleted with 3,844 passed, 10 skipped, 2 failed (plus 12/12 changed-coverage tests). Both failures are unchanged wall-clock assertions intest/apply-runtime-budget.test.ts: the GitHub retry and malformed-JSON retry deadline cases exceeded their 3-second allowance during the broad run. Both immediately passed in isolation (271 ms and 207 ms). No timing budget or unrelated implementation was changed, and the broad run is not being represented as green. The exact committed head now has green hosted CI, including the completepnpm check, Windows launcher smoke, and sparse repair build smoke; CodeQL also passed. CI reinforces, but does not replace, the actual generation and CLI proof.Focused command:
Real Behavior Proof
Claim and revision. Actual structured generation and the real local review CLI accept single-line live-proof commands while the parser continues rejecting separators. Refreshed after rebasing onto
d103c0cf5c98b0f3d4242bb4202c032856c36a49; the tested source is the exact committed tree at702d7836f8dbca682c08d9e2cdc104a8d1df9278(the pre-commit tree and committed tree are verified byte-identical). Environment: macOS, Node 24.20.0, pnpm 11.10.0; configured authenticated Codex. Existing Bash 5.3.15 was exposed through a task-local PATH for workflow fixtures; no global configuration was changed.Generation control. A real production
runAgentProcesscall used the complete shipped decision schema, the verbatim stock live-proof guidance, and a dependency-free Node fixture. The fixture requires a setupentry, exactly one non-overwriting proofrun, and a stable output assertion; embedded JavaScript must handle an escaped newline. There was no historical PR context or extra single-line workaround. Inference exited 0; the unmodified JSON passed the production parser and both shipped field patterns. Both commands also passed Bash syntax checking. The model executed no tools, and the proposed plan was not run.Raw generated plan excerpt:
{ "status": "recommended", "surface": "terminal", "terminalCompletion": "exit_zero", "entry": "node -e 'const fs = require(\"node:fs\"); fs.writeFileSync(\"input.json\", JSON.stringify({message: \"first\\nsecond\"}), {flag: \"wx\"});'", "steps": [ { "action": "run", "command": "node -e 'const fs = require(\"node:fs\"); const {message} = JSON.parse(fs.readFileSync(\"input.json\", \"utf8\")); if (typeof message !== \"string\" || message.split(\"\\n\").length !== 2) throw new Error(\"Expected two decoded message lines\"); fs.mkdirSync(\"proof-output\"); fs.writeFileSync(\"proof-output/result.txt\", message, {flag: \"wx\"}); console.log(\"PROOF_OUTPUT_CREATED\");'" }, { "action": "expect_output", "text": "PROOF_OUTPUT_CREATED" } ] }Real CLI path. A fresh
review --local-only --allow-closedrun used a new managed checkout of the original PR 1261 head14214b6f84a7be78785b24d0360080be3c368630. It supplied no additional prompt, additional policy, or body override. Equivalent CLI invocation, using a fresh artifact directory and the configured Codex:node dist/clawsweeper.js review --local-only --allow-closed --target-repo openclaw/clawsweeper --item-number 1261 --artifact-dir "$(mktemp -d /tmp/clawsweeper-live-proof.XXXXXX)" --readonly-openclaw --codex-sandbox read-only --codex-reasoning-effort high --codex-timeout-ms 1200000Observed receipt:
The CLI hydrated GitHub context through the configured cache: its metadata reported the historical PR open although an independent PR view showed it merged. The exact target head was verified. The historical PR body quotes its prior additional-prompt workaround; that text was not injected as a maintainer instruction. This is why the separate generation control deliberately contains no historical PR context. This CLI run establishes generation/parsing/report creation at the requested historical head, not current PR lifecycle freshness.
Artifact fingerprints. Full raw outputs and command/provenance receipts are retained locally; these hashes bind the inspected outputs without publishing agent transcripts:
d1e015846fd51478782250e0fb80cdb94ad21e13a5f395922f2dea095fd6e24b0088624a08951b16d74b42ee8a46febf927062efa738c89d964bfeb2773af3243d60d5b07b456b317c8b600c25ce3fb99f05aa0fed75fefe8ae000f2cfb7283c2284556c7bded9af826510b51ab42b8d5d4c6aac489d0103c122c41b4a6af1922d977205a9ec4035cc77d4bc5058417e3a2e599cc3934e6c4ef51c6d3f7ad86b5a229ec4e1a914297759954e11394225b123280f511696d2a5c03c84dda34c0eScope and Limits
The terminal commands generated during this proof were not executed. The exercised behavior is actual structured generation, strict parsing, and real local CLI report creation, not terminal execution, a production deployment, or a statistical reliability guarantee. No live apply/close, repair, release, or unrelated publication was used for proof.