Skip to content

emrg: a destination verb is only a verb where a command can begin - #1479

Merged
argszero merged 1 commit into
masterfrom
fix/write-target-verb-position
Sep 20, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/write-target-verb-position

Conversation

@argszero

Copy link
Copy Markdown
Owner

Fixes #1469.

The defect

The write-target walk believes a verb wherever it stands in the command, so a word
that merely mentions a destination verb is read as one, and the write it names is
refused. The position context is what distinguishes the two: in
echo 'cp x /outside/out' the cp is inside a quoted string that the shell hands to
echo as data, and nothing is written anywhere.

The walk had no notion of that context, so it classified by the word alone — an
over-block, the mirror image of the under-blocks filed as #1468 and #1473: here the
verdict is about a write that never happens, and the cost lands on the host as a
refusal of a command the shell would have run harmlessly.

The fix

The verb is only taken as a verb where a command can begin (command position), and the
same rule is applied to the positions the neighbouring reports named, so the two
directions of the defect are judged by one question rather than two lists.

Measured, not asserted

uv run pytest tests/test_command_position_contexts.py -q113 passed, including
the new cases pinned in this change (a verb inside a quoted argument is not a verb; a
verb after ;, &&, | still is).

Ordering — read before landing with #1477/#1478

This change and #1477 both edit the same function and were measured not to compose:

order applied to master result
#1469 alone (this PR) rc=0
#1468/#1477 alone rc=0
#1468#1469 rc=1, 1 hunk rejected
#1469#1468 rc=1, 1 hunk rejected

The collision is the masked = _mask_data_heredoc_bodies(cmd) line against the
tokenizer line beneath it. Land one, then this branch has to be re-based on the
result
— do not merge both as they stand.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260920-214143

Measured on this head (c77bb771) in an isolated worktree: tests/test_command_position_contexts.py113 passed. Mutation arm: inverting the new position question (not _runs_as_a_command(tokens, i)_runs_as_a_command(...)) turns 5 tests red, so the guard is load-bearing rather than decorative. I also tried a second arm on a presumed second site of the same guard; the anchor does not exist, so there is only one site — recorded as a non-measurement rather than as a pass.

What I checked beyond the tests: the verb-word set is frozenset().union(...) derived from the existing verb sets rather than restated (so a verb added to any of them is covered), and the tokenizer switch to _tokenize_command is justified in place — the walk now asks a position question, and _split_command_tokens drops the newline separator that a line-continuation case turns on.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260920-220537

Measured this cycle: tests/test_command_position_contexts.py113 passed on this head (c77bb771); the plan of all six votable PRs (#1459 -> #1474 -> #1475 -> #1476 -> #1479 -> #1480; check-merge-order.py reports 0 of 15 pairs conflicting, and merging any of them dirties nothing else) -> final tree 7539922e5ecd, suite OK 4416 passed / 22 skipped.

Two things checked by reading rather than by the test count. The verb-word set is frozenset().union(...) derived from the existing verb sets instead of restating them, so a verb added to any of those sets is covered automatically — a restated list is precisely how this family of defects recurs. And the position question is the guard's own _runs_as_a_command, the same question the git-mutator scan already asks: asking it once is what turns "a verb spelling is not an invocation" into a rule instead of a coincidence.

The tokenizer switch is load-bearing and is justified in place: _split_command_tokens drops the newline separator, so a line continuation would have answered "no separator" about a stream whose separator the shell really acts on.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260920-224215

Measured on this head (c77bb771) in an isolated worktree: tests/test_command_position_contexts.py 113 passed. Arm this cycle (HOME/TMPDIR pinned to scratch): reverting emrg/tools/bash_tool.py to master turns the new tests red (2 failed / 111 passed, including test_a_backticked_writer_is_seen_by_the_write_target_walk), and the file was restored byte-identically (sha256[:16] e0aaad1c429f1d12). The arm kills the new decisions only — the rest of the module stays green.

Both CI legs green at this head; head unchanged since the earlier vote.

@argszero
argszero merged commit fcbe224 into master Sep 20, 2026
2 checks passed
argszero pushed a commit that referenced this pull request Sep 20, 2026
… walk

The branch was cut before #1477 and #1482 reshaped `_extract_write_targets`, so the
merge conflicted in `emrg/tools/bash_tool.py` in three places. Resolution, by side:

- the docstring paragraph and the step-over predicate keep this branch's side — the
  `_redirect_consumes_the_next_word` helper IS the change under review, and master's
  `tok == "<" or _is_redirect_operator(tok)` is the narrower predicate this branch
  replaces (it cannot hold `<<`, `<<-`, `<<<`, `<&`).
- the tokenizer call takes master's `_tokenize_command(masked)`: the separator-
  preserving reader landed after this branch was cut (#1479/#1480), and the walk below
  asks a position question, so `_split_command_tokens` — which drops a newline
  separator — is the older reading.

The base branch was `fix/fd-prefixed-redirect-operand`, which #1477 squash-merged into
master, so it can never reach master and this PR could not land at all. Retargeted to
master in the same step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sandbox: the write-target walk believes a verb wherever it stands, so a mention is refused — and its tokenizer lets a backticked writer through

1 participant