Repository navigation
fix(OMN-19542): the PR-body stamp guard reads heredocs and create-then-edit commands instead of refusing them - #2341
Conversation
…n-edit commands instead of refusing them The guard split commands with shlex, which has no notion of a here-document and treats a newline as whitespace. Of its 22 logged refusal rows, 15 were commands it could not tokenise (an apostrophe in a quoted heredoc body) and 3 read a body file the same command had not written yet; only 4 were real dropped-line refusals (ledger:3319). - Split with the shared shell tokenizer; shell_words.tokenize gains keep_redirects so the guard can see which file each command writes. - Walk the command in order with a small file model: a body file written by a heredoc, printf, echo, cat, tee, cp, or the live body fetched with a read is judged on that text, never on the stale disk file. --body-file - and --body "$(cat <<'EOF' ...)" resolve from the heredoc. - A body only known after the command runs (interpreter or sed -i rewrite, expanding heredoc with a substitution, unresolvable write target) still refuses, now naming the writer and the workaround: prepare the file in one Bash call, edit in the next. - Fixes a fail-open: an edit on any line after the first was never seen, because shlex never emitted the newline separator. Also: `do`/`then` are keywords, not programs; relative paths resolve against the session cwd; gh api -f body=@x is literal; the wrapper pre-filter now passes --input. The stamp-drop rule is unchanged. Regression set: 39 tests in tests/hooks/test_pr_body_stamp_guard_parse_omn_19019.py; tokenizer tests in tests/hooks/test_shell_words_redirects.py were drafted by onex delegate run 18a8b05c-dd56-4f0c-8330-f91dadf03aa9 (one wrong assertion corrected). Onex-Lane: stamp-guard-parse-fix Onex-Session: d4d20bbca21e42cfb74215bec0aa6367
|
No OCC evidence companion was minted for this PR. product PR is a draft; companion suppressed (F-17) Matched in the state: To clear this: Mark the PR ready for review. The ready_for_review trigger re-fires the born path and the companion mints then. Reported by |
|
No OCC evidence companion was minted for this PR. the PR body contains 'WIP' at line 3, which the F-17 hold-marker rule reads as a do-not-merge/WIP hold; companion suppressed Matched in the body line 3: To clear this: The hold is honoured exactly as written. Reword or remove 'WIP' in the PR body — note that a CONDITIONAL hold ('do not merge until CI is green') is indistinguishable from an unconditional one to this rule — then re-run the OCC Companion Effect Publisher workflow_dispatch for this PR. Reported by |
…iclaude#2341 (#11247) * evidence(OMN-19542): author OCC companion for OmniNode-ai/omniclaude#2341 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 0252fd1cd68485341fd6fb5467ae4311b0113694. * evidence(OMN-19542): self-bind OCC#11247 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
✅ Hostile Reviewer — PASSEDBlocking findings (critical/error): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
OMN-19542: the PR-body stamp guard reads heredocs and create-then-edit commands
Ticket: OMN-19542 (child of OMN-19019, the stamp-guard slice; FP-B there). Guard of record: OMN-18335. Authority: the operator's question about this guard in session omni-home-83 (~07:00Z 2026-09-25), recorded with lane occ-stamp-rebind's tally in the rolling work ledger (TERMINAL row 3319, FRICTION row 3317). The WIP-freeze RULING (OMN-17427) exempts it as a lander fix.
Why
The guard split each command with
shlex, which has no notion of a here-document and treats a newline as plain whitespace. Its 22 refusal rows since 2026-09-23T18:07Z (ledger row 666 onward, 64 folded repeats) break down as follows (tally: ledger row 3319):What changes
shell_words.tokenizegains akeep_redirectsoption that emitsRedirecttokens, so the guard knows which file each command writes. The default output is unchanged, so the worktree and shared-tree guards are unaffected (315 of their tests pass).printf,echo,cat,teeorcp, or filled with the live body bygh pr view --json body --jq .bodyorgh api .../pulls/N --jq .body, is judged on that text. The disk file as it stood at hook time is never used.--body-file -resolves from a heredoc,<or a pipe.--body "$(cat <<'EOF' ...)"resolves too.sed -i, an expanding heredoc with a command substitution, or a write to a path that cannot be resolved. These are still refused. The message now names the writer, and says to prepare the file in one Bash call and run the edit in the next. A command that cannot be split into words at all is refused only when its text namesghfollowed bypr editorapi.shlexnever emits a newline separator, so the old parser could not see an edit on any line after the first. For example,cd /tmpfollowed on the next line bygh pr edit 1 --body gonewas admitted. A regression test now pins this.doandthenare keywords, not programs.cdin the command. Before, they resolved against HOME, where the wrapper runs.gh api -f body=@xis literal, and only-Freads a file.--input, which before never reached the decision core.The stamp-drop rule itself is unchanged.
RED first. Against the unchanged guard, the new regression file had 32 failed and 6 passed.
Green. 105 passed across
tests/hooks/test_pr_body_stamp_guard_parse_omn_19019.py(39),tests/hooks/test_pr_body_stamp_guard.py,tests/hooks/test_pr_body_stamp_guard_rebind_omn_18853.py(the omniclaude#2340 suite, unchanged) andtests/hooks/test_shell_words_redirects.py. The worktree-add and shared-tree git guard suites also pass: 315 tests. The hooks baseline suite passes (25).mypy --strictandruffare clean on the changed modules, and pre-commit passes on the changed files.Replay of the refusing commands. 90 distinct refused commands were harvested from the refusing sessions' transcripts: 48 untokenisable, 23 unreadable and 19 dropped. They were replayed through both guards with a stub live reader.
printflines.Calibration. Over 1,294 transcript Bash commands since 2026-09-22 that pass the wrapper's pre-filter, there were 0 crashes. 46 commands admitted before are refused now. In all 46, the old parser saw no edit at all: the edit sat on a line after a heredoc or a
cd. 37 of them rewrite the body file with Python orsedbefore sending it, so the old guard, had it seen them, would have judged the pre-rewrite file.Live read-only probe, 09:43Z. The worktree's hook wrapper was fed four payloads against omnibase_core#1762, with the live description read by the guard. The commands themselves were never run.
All four got the expected exit code. The probe's refusal row (ledger 3477) is annotated by a CORRECTION row.
Delegation. The tokenizer tests were drafted by
onex delegate --task-type test, run18a8b05c-dd56-4f0c-8330-f91dadf03aa9(Qwen3.8-27B, about 21 s, rc 0). Result: 8 of 9 tests were correct as drafted. One assertion miscounted the words (it said 2; the right count is 3) and was corrected in review.Lab
Not runtime-affecting. This is a session tool-seam hook that runs in no deployed runtime, the same as OMN-18335. The live probe above is the readback.
Deploy note
Hooks load from the omniclaude canonical clone's working tree, not from the plugin cache. After merge, that clone has to be fast-forwarded to
origin/dev, and a new session started, before the fix is live.Residuals, stated
uv run pytest ...; gh pr edit ... --body-file f). The refusal names the workaround. This is deliberate: an interpreter can write any file.bash -c '<edit>'andevalare still not parsed as nested commands. That is unchanged from before.Evidence-Ticket: OMN-19542
Evidence-Source: OCC#11247