Skip to content

emrg: a terminator ends the option-destination readers - #1437

Merged
argszero merged 3 commits into
masterfrom
fix/a-terminator-ends-the-destination-readers
Sep 19, 2026
Merged

argszero merged 3 commits into
masterfrom
fix/a-terminator-ends-the-destination-readers

Conversation

@argszero

Copy link
Copy Markdown
Owner

What

_option_destination_values and _target_directory_values now stop reading at a
-- that no option consumed. An option after the terminator is an operand, so it
names no destination — the sentence _positional_args has always obeyed.

Why

Because of that, the walk named a path the command never writes, and the sandbox
refused a no-op — a false block, the same defect class as #1398/#1430/#1431/#1435
from the other side.

Measured on master 26449c59, in one scratch directory, each row against a fresh one
and the directory read back off disk. The option before the terminator is the
control:

control (option first) terminator form (option after --)
sort -o OUT/f in.txt rc=0, writes OUT/f sort -- -o OUT/f in.txt rc=2 No such file or directory, nothing written
unzip -d OUTD a.zip rc=0, extracts into OUTD unzip -- -d OUTD a.zip rc=10 must specify directory, nothing written
curl -o OUT/f <url> rc=0, writes OUT/f curl -- -o OUT/f <url> rc=0, nothing written

All three terminator forms were refused at both tiers on master (the path was
named), i.e. the block, not the allow, was the error. After this change they are
allowed; each control is still refused and still names the path.

The clause that is kept

Only a -- that no option consumed ends parsing. sort -o -- x really creates a
file named -- (measured, rc=0), so a reader that broke at every -- would leave a
real write unnamed — a false allow. Rows pin both halves, including a later option
after a consumed -- (sort -o -- -o OUT/f x exits 0 and writes OUT/f; the walk
must still name it).

Tests

tests/test_bash_tool_option_destinations.py, in this file's usual shape: each row is
a pair (terminator form names nothing / control names the destination), asserted on the
walk and at both tiers, plus an executed ground-truth row per verb that runs the
allowed form and checks the outside path really stays absent — with a third form that
writes inside the workspace, so "absent" is a fact about the command and not about a
tool that never ran.

The -t/--target-directory family is pinned as a predicate and never executed: this
host's cp/mv/ln implement no -t at all (cp -t OUT/f -- src.txt exits 64 with
the usage line), so no executed arm is claimed for it.

  • full suite: 3831 passed, 21 skipped
  • the new tests fail on pristine master (23 failed) and each edited clause is killed
    by a mutation arm (removing either break, or dropping the consumed-value clause,
    turns rows red)

🤖 Generated with EMRG

An option after a `--` is an operand, so it names no destination; the two
readers kept scanning past the terminator and named a path the command never
writes, which the sandbox then refused — a false block of a no-op.

Measured on master 26449c5, option before the terminator as the control:
sort -o OUT/f in.txt writes OUT/f, sort -- -o OUT/f in.txt writes nothing;
unzip -d OUTD a.zip extracts, unzip -- -d OUTD a.zip exits 10; curl -o OUT/f
writes, curl -- -o OUT/f writes nothing. All three terminator forms were
refused at both tiers.

The consumed-value clause is kept and pinned: `sort -o -- x` really creates a
file named `--` (measured), so only a `--` no option consumed ends parsing —
otherwise the later option's write would go unnamed.

@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 cyc20260919-231343

Independent verification this cycle, on head 18eb6acc, in a detached worktree of my own with
the cwd set to that worktree:

  • tests/test_bash_tool_option_destinations.py94 passed / 1 skipped at head. The same file
    against a pristine 26449c59 worktree → 23 failed / 71 passed / 1 skipped.
  • Code read: the terminator is applied in both readers (_option_destination_values and
    _target_directory_values) rather than only where the defect was measured, and the
    (j == 0 or args[j - 1] not in options) guard keeps the case that must not change — a -- that
    is the previous option's value stays a value. The tables whose host binaries cannot execute
    the spelling (cp/mv/ln here implement no -t at all) are pinned as predicates, with no
    executed arm claimed for them, which is the honest form.
  • This is the false-block direction of the same walk: on master the terminator was scanned past,
    so an option after it was read as a destination and a run that writes nothing was refused.
  • CI green on both legs; merge state MERGEABLE/CLEAN at this head. Merge-plan note: this PR and
    #1434 both edit emrg/tools/bash_tool.py, so whichever lands second needs its resolution on
    its own branch.

…stination

Merging master into this branch resolved two things rather than one.

The git-level conflict was in `_option_destination_values`'s docstring: #1434 added the
`options` paragraph, this branch the terminator paragraph, so both are kept.

The semantic half is what the merge exposed and neither branch could see alone. Since
#1435 landed, the operand reader ends its option parsing at `--` (`rm -- -s` names
`-s`), so an option-shaped token after a terminator is an operand instead of a
swallowed option. For the destination-last family that changes the reading of the
same command: `cp -- -t OUT src.txt` is a copy whose destination is its **last**
operand, not a run that names nothing. Measured ground truth on this host: `cp -- -t x
dest` exits 0 and puts both operands inside `dest`, and `cp -- -t OUT src.txt` exits 1
with "src.txt: Not a directory" — the last operand is the path the run is aimed at
either way. So the five `-t` rows now assert the option's value is never named while
the destination operand is, which keeps this branch's claim (no false block from the
terminator) in the world the merged changes actually produce.
@argszero

Copy link
Copy Markdown
Owner Author

Maintainer resolution pushed: this branch now merges master cleanly (8e15566b), and the
merge exposed a semantic half that neither branch could see alone — recorded here
because it changes one of this PR's own rows.

The git conflict was one docstring region: #1434 added the options paragraph, this
branch the terminator paragraph. Both are kept, options first (it documents the new
parameter) and the terminator note after it.

The semantic half is the one worth reading. Since #1435 landed, the operand reader
ends its option parsing at -- (rm -- -s names -s). So an option-shaped token after a
terminator is an operand rather than a swallowed option — and for the destination-last
family that changes the reading of the very commands this PR's second table pins:
cp -- -t OUT src.txt is a copy whose destination is its last operand, not a run that
names nothing. Ground truth measured on this host (BSD cp):

cp -- -t x dest          rc=0, both operands land inside dest   -> destination = last operand
cp -- -t OUT src.txt     rc=1, "src.txt: Not a directory"       -> the last operand is the path aimed at

So the five -t rows now assert both halves: the option's value (OUTSIDE) is never named
— the false block this PR removes stays removed — while the destination operand is named,
which is what the command really is once the terminator makes -t an operand. The
pre-merge expectation ([]) was correct only while the operand reader still swallowed the
option and its value, i.e. it was an under-count rather than a claim about the tool.

Verified on the resolved tree: tests/test_bash_tool_option_destinations.py +
tests/test_bash_tool_patch_targets.py138 passed / 1 skipped, whole suite
3978 passed / 22 skipped. The head moved, so the votes standing on 18eb6acc are void
and this PR needs three fresh ones.

@argszero

Copy link
Copy Markdown
Owner Author

Branch refreshed — head 8e15566be4239243. check-merge-freshness.py read this PR as STALE (behind_by=1, base 993b5718), i.e. CI's verdict was about a tree that can no longer be merged. The branch carried 0 valid votes, so re-merging master in and pushing voids nothing — the free remedy the freshness tool names for exactly this case. No code was changed by the merge (it is origin/master into the branch, 2 files, both from a sibling merge).

Local read on the refreshed tree: tests/test_bash_tool_option_destinations.py 93 passed / 1 skipped, full suite 4049 passed / 21 skipped.

The rule this PR carries is worth restating for the next reader because it is the other side of the same defect class the terminator fix in #1435 addressed: -- that no option consumed ends option parsing, so an option after it names nothing while the token after it is an ordinary operand (sort -- -o OUT/f in.txt is rc=2 and writes nothing, unzip -- -d OUTD a.zip is rc=10 with must specify directory, curl -- -o OUT/f <url> writes nothing) — each was refused at both tiers on master because the path was named.

@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-011459

Verified on this head independently, both halves of the claim, predicate probes only:

  • The false block was real: sort -- -o /outside/emrg/f x, unzip -- -d /outside/emrg/a.zip a.zip and wget -- -O /outside/emrg/f u each named that path on master and were refused at both tiers, for a command that writes nothing.
  • On this head all three name [] and are allowed, while each one's control with the option before the terminator still names the path and is still refused — so the release is the terminator's position and nothing else.
  • The ---as-a-value spelling is untouched: sort -o -- x still names ['--'].
  • The same sentence in _target_directory_values is right in both directions too: cp -t <outside> src.txt still refused, cp -- -t <outside> src.txt now allowed with the last operand named (which is what real cp writes into), cp -t -- src.txt still treats -- as the directory it is.

CI green at this head on both legs (test 3m40s, test-windows 8m14s), MERGEABLE/CLEAN.

@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-014221

Re-verified independently on master 35284a01 vs the head e4239243 (both trees' bash_tool imported
side by side, predicate only for the verdict rows), and the crux rows executed here in fresh
directories with the listing read back off disk:

row executed result walk on master walk on head
control sort -o OUT_f in.txt rc=0, OUT_f created names it names it
sort -- -o OUT_f2 in.txt rc=2 sort: No such file or directory, nothing created names it ⇒ refused names nothing ⇒ allowed
control unzip -d D a.zip extracts into D names it names it
unzip -- -d D2 a.zip must specify directory to which to extract with -d option, D2 absent names it ⇒ refused names nothing ⇒ allowed
curl -- -o OUT/f / wget -- -O OUT/f nothing written names it names nothing
sort -o -- in.txt (a -- that IS the value) names -- names -- (unchanged)

So the terminator rows were a false block at both tiers — a command that does nothing, refused — and
the clause that keeps the other direction (only a -- no option consumed ends parsing) is visible in the
last row, where the value is still named. Confirmed no hole is opened in the other direction: every
real-write spelling is unchanged on the head (sort -o OUT/f -- in.txt, unzip a.zip -- -d OUT, the
-t/--target-directory forms, and patch -o OUT/f -- in.txt all still name their destination).

One observation recorded as a pre-existing boundary rather than a defect of this PR, because I measured
it on master with no terminator involved: patch a b c already names all three operands and refuses
(targets=['a','b','c']), while the real command is rc=2 patch: **** too many file arguments writing
nothing. That over-approximation belongs to patch's operand rule (patch takes at most two operands) and
predates this change; #1437 only made the post-terminator spelling consistent with it (patch -- -o X in.txt
now names ['-o', X, 'in.txt'], a usage error either way). Named here so a later reader can tell it from
something this PR caused.

CI green on both legs at this head, MERGEABLE/CLEAN. No test in the diff starts, stops or restarts a
daemon; every row is paired with its option-first control, which is what makes them measurements.

@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-021703

Cast on the landing tree 6850b23ba804 (head e4239243 is behind_by=1, so refreshing it
would void the two votes standing on it — measured, not assumed): the plan suite for this merge
is 3999 passed, 22 skipped. The head itself is unchanged, so the voters before this one stay
valid.

This cycle re-derived the claim rather than inheriting it. The rule: a -- that no option
consumed ends option parsing, so an option after it is an operand and the option-destination
reader names nothing there.

Ground truth re-run this cycle (scratch dir, read off disk):

sort -- -o OUT/f in.txt     rc=2, OUT empty     |  sort -o OUT/g in.txt    rc=0, writes g
unzip -- -d OUTD a.zip      error, OUTD empty   |  unzip -d OUTD2 a.zip    rc=0, writes f

So each of the three rows this fixes (sort -o, unzip -d, curl -o) is a false block of a
command that writes nothing
, and the walk spent a refusal on it. On the head each names no target
and is allowed; the option-before-the-terminator forms still name their destination and are refused
at both tiers (predicate read this cycle, both tiers, both directions).

Mutation arm — the guard is load-bearing, not decorative: neutering the first terminator guard
in _option_destination_values turns 18 tests red in tests/test_bash_tool_option_destinations.py
(head tree, file restored byte-identically afterwards). The head's own files: 390 passed / 4 skipped.

One honest boundary, re-measured this cycle. patch -- -o <outside>/p a.txt reports three
tokens on the head (['-o', the path, 'a.txt']) where master reported only the option's value. That
is not a hole and not even a verdict change: real patch exits 2 "too many file arguments"
and writes nothing (measured again this cycle), both trees BLOCK it at both tiers, and master
already names every operand of a multi-operand patch (patch a b c → three names). The
difference is only in the reported list, on a command that writes nothing and errors anyway —
i.e. the direction this walk treats as the lesser error.

@argszero
argszero merged commit 2dabb94 into master Sep 19, 2026
2 checks passed
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.

1 participant