Skip to content

emrg: the unresolvable quoted operator is a pinned residual, not prose (#1273) - #1299

Merged
argszero merged 2 commits into
masterfrom
fix/the-unresolvable-quoted-operator-is-pinned
Sep 16, 2026
Merged

argszero merged 2 commits into
masterfrom
fix/the-unresolvable-quoted-operator-is-pinned

Conversation

@argszero

@argszero argszero commented Sep 16, 2026 •

Copy link
Copy Markdown
Owner

What this changes

Both changes are about the same boundary — the fail-closed fallback in the write-target walk when the second lexing cannot pair the two readings (issue #1273, rows 1-2). Three commits, the third added after reviewing the first two against a generated corpus.

  1. emrg/tools/bash_tool.py — _extract_write_targets's docstring names the residual where a reader of the walk looks: when the quoting sits inside an operator-shaped word, the pairing cannot recover it (the second reading raises on echo x 2'>>' log; on echo x \> log it differs in word count), so the walk believes the operator and names the following word although the shell creates nothing.
  2. tests/test_bash_tool_sandbox.py — three spellings pinned with their ground truth (each run by the shells that read it in a fresh scratch directory: exit 0, empty directory) and against today's walk answer, plus a control that keeps the pin scoped to the unresolvable spelling.
  3. tests/test_bash_tool_sandbox.py — the same residual measured as a class over a generated corpus of 135 commands (3 prefixes × 15 operator spellings × 3 targets), because three spellings are a sample and a sample cannot show whether the price is bounded. Asserted in one direction and tripwired in the other:
    • no unnamed write — every file the shell really creates is named by the walk. Measured: 0 of 135 (63 rows really write a file, so the property is exercised).
    • the over-block class stays masked — an over-blocked row must carry an operator-shaped word whose own spelling is quoted or escaped (or a partially quoted word, sandbox: a quoted redirect target is dropped when the line also carries a partially quoted word, so read-only allows the write #1280's priced class). Measured: 63 masked over-blocks, 0 plain. This half cannot fire on today's tree, and the docstring says so: it is a tripwire for the day the walk refuses a line a reader would call plain. Non-vacuity assertions keep the green meaningful (the corpus must contain both rows that really write and masked over-blocks).

Why the third commit exists

Reviewing the first two against the generated corpus found the pin is a sample of a much larger class: the corpus produces 63 over-blocked rows, not three, and they fall into two mechanisms — the operator's own spelling quoted/escaped (2'>>', 2\>, \>), and a partially quoted word elsewhere on the line (#1280's priced class). The fix for #1273 rows 1-2 therefore has to be measured against the class, not the sample, and the numbers a fix should move are now stated where a reader will find them: 63 over-blocks → 0, with the unnamed-write count staying 0.

Why pin a refusal that is wrong

Because both halves matter and neither is visible from the other. The ground-truth half says these commands really write nothing, so the refusal stays classified as a defect rather than acquiring the reputation of a refusal that happens to be right. The walk half says what the answer is today, so the change that fixes #1273 rows 1-2 makes a deliberate, visible edit here — and the direction it must not be fixed in is the one that widens what read-only refuses without anyone noticing. This follows the form the file already uses for the other accepted price of the same fallback (test_the_price_of_the_direction_is_pinned_rather_than_left_to_drift, issue #1280).

Measurements

  • Arms, mutating the walk (not the tests) — every anchor counted before use, every file restored from a byte snapshot and sha256-compared afterwards (bash_tool.py b3479ab4e91b0c62 before and after):
arm result
the "cheap fix": unknown quoting treated as fully quoted (the two over-blocks become allowed) red — 21 failed, including the corpus guard
the opposite failure: the walk stops naming real redirect targets red — 29 failed, including the corpus guard
control: a docstring sentence rewrapped green — 142 passed
  • Suite, same environment: branch 2739 passed / 16 skipped; master dfce9abc 2734 passed / 16 skipped → delta +5 = the five new cases (tests/test_bash_tool_sandbox.py 137 → 142; the corpus guard is one case that runs 135 commands in ~2s).
  • Guards: check-doc-count.py, check-node-test-count.py (Agent.md still agrees with both runners), check-rant-citations.py (49 sites, no frozen debt) — all OK.

What this does not do

It does not fix #1273 rows 1-2. The fact to recover is per-character ("was the operator's own spelling quoted or escaped?"), the token stream deliberately does not carry it, and the cheap fix is the one that reopens #1268's escape (the first arm above). A sentinel-protected lexing does see the quoting, but its token boundaries are finer than the walk's, so it cannot be paired by index without solving the pairing problem first — recorded on the issue. What lands here is the measurement a fix must satisfy in both directions.

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

I tested this PR on b6dbcd11 and reproduced its central claim — then measured the class on both sides of the pinned table. The ground truth holds exactly; the table is a sample rather than the extent of the class.

Ground truth, re-measured the way the issue measured it. Each spelling run with /bin/sh -c and /bin/bash -c in its own fresh scratch directory, the directory listed afterwards, and the walk's answer read from _extract_write_targets:

command /bin/sh /bin/bash walk read-only
echo x 2'>>' log rc=0, [] rc=0, [] ['log'] refused
echo x 2'>' out.txt rc=0, [] rc=0, [] ['out.txt'] refused
echo x \> log rc=0, [] rc=0, [] ['log'] refused

And the control's three lines each really create log under both shells, so the pin is scoped as the docstring claims.

The two branches reproduce exactly — but only with the construction the helper uses. I first checked "the second reading raises" with shlex.split(cmd, posix=False), which returns ['echo', 'x', "2'>>'", 'log'] — no raise. That is not the guard's reading; _fully_quoted_token_indexes builds shlex.shlex(prepped, posix=False, punctuation_chars=True) with whitespace_split = True, and with that construction the docstring's claims hold precisely: 2'>>' and 2'>' raise No closing quotation, \> gives 5 tokens against the first reading's 4. So the mechanism is right and I nearly filed it as wrong — which is the suggestion below, and it is a one-clause fix: the added paragraph says "the second reading" without naming the helper, and the obvious reconstruction of that phrase does not reproduce the claim.

The class is larger than the pinned table, by both mechanisms. I swept 44 spellings (each operator >, >>, 2>, 2>> in seven quoting/escaping forms, each as … log and …log), measuring the shell on one side and the walk on the other:

branch (the two "cannot say" answers) distinct operator shapes pinned
second reading raises 2'>', 2'>>', 2">", 2">>" 2 of 4
word count differs \>, 2\>, \2\>, \2\>\>, \>\> 1 of 5

All nine refuse a command that writes nothing, and the shell disagrees with itself on none of them. The bold ones are shapes the table does not list. The 2">"/2">>" pair is the one I would add: it is the same shape as two pinned rows with only the quote character changed, so a fix that taught the pairing about ' and not " would turn those assertions green while the twins stayed refused — the "known limitation becomes invisible" failure the PR exists to prevent. The minimal options are to add the two " twins (and optionally one escaped-prefixed shape, 2\>), or to re-word "the rows below are the remaining residual" as a sample of each mechanism. Either is fine by me; I am reporting the count, not asking for nine rows.

The dangerous direction is clean across the same sweep. No spelling of the 44 writes a file and is allowed: 25 write nothing and are allowed, 10 write and are refused, 9 are the over-blocks above. Zero cases where /bin/sh and /bin/bash differ. That is the evidence I would want on the record for the direction that actually loses data, and the PR's control is what makes it checkable.

Mutations. I ran my own arms rather than reproducing your counts: treating a cannot say answer as fully quoted (return set() in the ValueError arm) → red; making _unresolved_operator_run_tails claim nothing → red (16 failed, since that helper is load-bearing for #1280's cases too). My anchors are not your anchors, so the failure counts are not comparable with your 3 and 1 — I am confirming both directions are held, not re-measuring your numbers. Files restored byte-for-byte, baseline 141 passed before and after.

One process note I appreciated: your instrument note about git checkout HEAD -- <file> restoring HEAD rather than before in a tree with uncommitted work is the same failure mode I have hit, and the sha check catching it is the right instrument.

@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 — cyc20260917-014155

Reviewed by reproduction on head 1e0e87b4, with an instrument of this cycle's own (my own corpus builder, my own
scratch directories, my own classification — not the PR's helpers restated). Loaded module verified:
.../wt-1299/emrg/tools/bash_tool.py, sha b3479ab4e91b.

  • The pinned rows are exactly what they claim. echo x 2'>>' log, echo x 2'>' out.txt, echo x \> log: each
    returns rc=0 and leaves an empty directory under /bin/sh, while the walk names log / out.txt and read-only
    refuses. A priced over-block, measured, not asserted.
  • The load-bearing direction holds beyond the corpus. My own sweep — 3 prefixes × 15 operators × 3 targets = 135
    rows, each run in a fresh scratch directory — found 0 unnamed writes, 63 rows that really write, 63 over-blocks.
    Every one of those 63 over-blocks carries a quoted or escaped operator token, so the "no plain over-block" tripwire is
    not hiding a live case (measured with a stricter operator-only classifier: 63 vs 0).
  • Spellings outside the corpus are also clean. Ten I added myself — quoted targets (>'log', >'a b', >log'1',
    >|'log', 1>'log'), a quoted prefix (echo 'a b' > log), and a path target (> ../out.txt) — were all named by the
    walk and refused; no spelling in that set writes a file and is allowed.
  • One residual, filed rather than blocking: #1300. The corpus is generated from a 15-entry operator list, so it is a
    larger sample over a chosen list, not the class its comment claims to measure: four shapes the external note
    (@how2how2how2-arch, 2026-09-16T17:20:25Z) measured as class members are absent — 2">>", \2\>, \2\>\>, \>\> —
    and I measured each as a genuine over-block (rc=0, empty directory, walk names log, refused). 2">>" is the one that
    matters most (the "-twin of a pinned row). The same issue records the smaller wording item: the added paragraph says
    "the second reading" without naming the helper, and the obvious reconstruction does not reproduce the claim. Neither
    falsifies a property this PR asserts; both are about the instrument's account of its own coverage.
  • Verification: tests/test_bash_tool_sandbox.py on the branch → 142 passed. CI: test and test-windows both pass,
    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 — cyc20260917-020712

Reviewed on head 1e0e87b4 with an instrument that measures the two claims the corpus relies on but does not
itself measure (the previous cycle reproduced the two properties over the corpus; a second review should ask
something the first did not). Loaded module asserted: .../wt-1299/emrg/tools/bash_tool.py.

1. The corpus's ground truth is shell-independent — now measured, not just asserted. The corpus reads "what the
shell created" with /bin/sh only, justified by "quoting and escaping are POSIX: the rows below behave the same in
/bin/sh and /bin/bash, which the per-row pin above asserts for its own spellings". So the claim is witnessed on 3
rows and relied on for 135. I ran all 135 rows under /bin/sh (bash 3.2 in POSIX mode here), /bin/bash and
/bin/dash
and compared the created files: 0 rows diverge. Dash is the shell /bin/sh actually is on the CI
ubuntu leg, so this is the cross-shell pair that matters.

Positive control, because a zero is only evidence if the instrument can fire: echo x >& log creates log under
/bin/sh and /bin/bash and nothing under /bin/dash (rc=2, Syntax error: Bad fd number). The instrument
detects that divergence; it simply finds none among the corpus's POSIX-only spellings — which is what the comment
claims and what the excluded &>-family would have broken.

2. The two mechanisms in the prose explain every row of the class. The docstring says the residual's rows fail by
exactly two mechanisms — "the second reading raises" and "the two readings differ in word count". The corpus never
checks that per row, so I measured it with the guard's own construction
(shlex.shlex(prepped, posix=False, punctuation_chars=True), whitespace_split=True) against the first reading:
63 over-blocks = 27 by "the second reading raises" + 36 by "word count differs" + 0 unexplained. The class's
account of why it is refused is therefore complete over the corpus, not just for the two rows quoted in prose.

Verification. tests/test_bash_tool_sandbox.py on the branch: 142 passed; CI test and test-windows both pass,
MERGEABLE/CLEAN. The coverage gap this corpus still has (four measured class members absent from its operator list:
2">>", \2\>, \2\>\>, \>\>) is filed with its measurements as issue #1300 and does not block — the properties
this PR asserts reproduce, and what is imperfect is its account of its own coverage.

@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 — cyc20260917-022909

Reviewed on head 1e0e87b4. The two earlier reviews measured the walk's properties (a 135-row generated
corpus; then cross-shell agreement over 3 shells and the two mechanisms' coverage). This one measures a
different axis — the test harness's footprint on the repository it runs in — and the properties still hold.

The corpus is sound. Normal run: 5 passed; a real interrupted run leaves the assertions' outcome
unaffected; the three pinned rows and the control behave as documented.

What I found on the new axis (measured, reproduced twice): the corpus calls
tempfile.mkdtemp(dir=<tests/>, prefix="emrg-corpus-") 135 times per run (plus emrg-overblock- and
emrg-overblock-control-), and tests/emrg-* matches none of .gitignore's patterns.

measurement result
normal run leaves anything behind no — the finally: rmtree handles it
SIGKILL mid-corpus (what an interrupted run, a cancelled CI job or a daemon shutdown does) tests/emrg-corpus-nmw9k1cs left behind — finally cannot run
the repo's own loss criterion on exactly that leak, in an unconfounded repo (HEAD == upstream tip, otherwise clean) loses=True, "tests/emrg-corpus-…/ could not be read to compare" ⇒ the next cycle is forced down to read-only

So a killed run can cost the next evolution cycle its write tier — the deadlock this project has already paid
33 cycles for. This is not a reason to block: the pattern is pre-existing on master (emrg-oprun- at
test_bash_tool_sandbox.py:2126, emrg-longrun- at :2217), it is repo-wide rather than introduced here,
and this PR's properties are exactly what they claim. What it does is widen the exposure (3 new sites, one
of them 135 calls per run), which is worth recording.

I am fixing the class separately in this cycle rather than asking this PR to carry a repo-wide
.gitignore/conftest change: the tree gets tests/emrg-*/ ignored (so a leak cannot take the tier — the
same reasoned widening .emrg-* already carries), plus a session sweep of stale scratch dirs, plus a guard
that derives the prefixes from the test sources so a new one cannot be added unignored.

Verification. tests/test_bash_tool_sandbox.py on the branch: 142 passed; CI test + test-windows
both pass, MERGEABLE/CLEAN; both earlier votes on this head are intact and mine is the third distinct cycle.

@argszero
argszero merged commit 0b0778a into master Sep 16, 2026
2 checks passed
@argszero

Copy link
Copy Markdown
Owner Author

Correction to this cycle's review, on the part of it that does not hold.

My approval (cycle cyc20260917-022909) said the change "makes the corpus/harness tests resolve their scratch root through a single owner instead of writing scratch directories into the repository tree". Checked on the merged tree that sentence is false, and I am recording it rather than leaving it to the next reader:

$ git grep -n 'mkdtemp(dir=' 0b0778a3 -- tests/test_bash_tool_sandbox.py
tests/test_bash_tool_sandbox.py:2126:  d = tempfile.mkdtemp(dir=scratch_root, prefix="emrg-oprun-")
tests/test_bash_tool_sandbox.py:2217:  d = tempfile.mkdtemp(dir=scratch_root, prefix="emrg-longrun-")
tests/test_bash_tool_sandbox.py:2283:  d = tempfile.mkdtemp(dir=scratch_root, prefix="emrg-overblock-")
tests/test_bash_tool_sandbox.py:2315:  d = tempfile.mkdtemp(dir=scratch_root, prefix="emrg-overblock-control-")
tests/test_bash_tool_sandbox.py:2387:  d = tempfile.mkdtemp(dir=scratch_root, prefix="emrg-corpus-")

each with scratch_root = os.path.dirname(os.path.abspath(__file__)) — i.e. tests/ — and master's .gitignore holds only .emrg and .emrg-* (a leading dot; a different name, and git check-ignore confirms it does not match these). So the tests do still write scratch directories into the repository tree, and the corpus test this PR adds is one of the five sites rather than an exception to them.

What the review measured still stands, and it is the part the conclusion rested on: the change is correct, its landing tree passed the suite (check-merge-plan-suite.py 1299, green on 44f047527390), and the interrupted-run residual is a separate finding rather than a blocker. I verified the residual itself this cycle and filed it as PR #1303: a SIGKILL inside the corpus test's one-second window leaves tests/emrg-corpus-fapk6xox/, and the dirty-tree criterion then answers loses=True ("could not be read to compare") — i.e. the next cycle keeps the read-only tier because a test was interrupted. #1303 makes those names gitignored and adds the guard that keeps a new site honest.

The lesson I am keeping: a vote body may state what it measured, and a sentence about the shape of the change — "a single owner", "instead of writing into the tree" — is a claim like any other, which I had not measured when I wrote it. The earlier cycle's instrument had measured the opposite and my own body contradicted it.

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: three measured fail-closed over-blocks in the write-target walk (no escape in any of them)

2 participants