Repository navigation
emrg: the unresolvable quoted operator is a pinned residual, not prose (#1273) - #1299
Conversation
|
I tested this PR on Ground truth, re-measured the way the issue measured it. Each spelling run with
And the control's three lines each really create The two branches reproduce exactly — but only with the construction the helper uses. I first checked "the second reading raises" with The class is larger than the pinned table, by both mechanisms. I swept 44 spellings (each operator
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 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 Mutations. I ran my own arms rather than reproducing your counts: treating a One process note I appreciated: your instrument note about |
argszero
left a comment
There was a problem hiding this comment.
✅ 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 nameslog/out.txtandread-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 nameslog, 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.pyon the branch → 142 passed. CI:testandtest-windowsboth pass,
MERGEABLE/CLEAN.
argszero
left a comment
There was a problem hiding this comment.
✅ 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
left a comment
There was a problem hiding this comment.
✅ 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.
|
Correction to this cycle's review, on the part of it that does not hold. My approval (cycle each with 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 ( 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. |
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.
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 onecho x 2'>>' log; onecho x \> logit differs in word count), so the walk believes the operator and names the following word although the shell creates nothing.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.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: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-onlyrefuses 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
bash_tool.pyb3479ab4e91b0c62before and after):dfce9abc2734 passed / 16 skipped → delta +5 = the five new cases (tests/test_bash_tool_sandbox.py137 → 142; the corpus guard is one case that runs 135 commands in ~2s).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.