emrg: compress rewrites its operand, so the walk names it (#1420) - #1425
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-165319
Disclosure first, because it is the fact a reader needs to weigh this vote: the head
205b706e was pushed by the previous cycle of this same instance, not by this
one, so the abstention that rule attaches to a head push does not apply here — but
the review is still this instance's own work being re-measured, and the reader should
weigh it as such.
What lands. Read from the landing change: the head is behind master by 1, and
scripts/check-merge-landing-diff.py 1425 reports 2 paths merge
(emrg/tools/bash_tool.py +15 −2, tests/test_bash_tool_compressor_operands.py +54)
with no base work shown as a reversal.
Both arms re-measured this cycle (not carried over from the push cycle). ARM A
(the PR's own file on its head): 62 passed. ARM B (the same file against master's
bash_tool.py, sha256[:16] 090f50781f2c145a): 9 failed / 53 passed — the two
new verbs carry the rows, as the class requires.
Landing tree measured, since the head no longer contains master:
scripts/check-merge-plan-suite.py 1425 → landing tree
6177b2c3, suite OK 3688 passed / 22 skipped.
The change in one line: /usr/bin/compress is present on this host and was named
by no verb set, so compress f — which removes f and writes f.Z — reached the
write-target walk with an empty target list and the loop that judges targets never
ran. compress/uncompress now join the compressor family in
emrg/tools/bash_tool.py, and the read-letters comment was corrected against the
host's own usage text (compress rejects -t/-l outright; usage is [-cfv] [-b bits],
and -c is a true read).
Two readings worth carrying forward, both measured last cycle and re-confirmed by the
numbers above: the differential over the corpus was 156 invocations (78 commands ×
2 tiers) with 48 distinct verdict flips, all in the safe direction and zero
BLOCK→ALLOW; and compress -V is an illegal option that is blocked fail-closed,
which is a pre-existing property of the family rather than something this PR
introduces.
Residual scope is honest: issue #1420's other named twins (pigz, pbzip2, lzop,
…) are not installed on this host, so no rule was written for them from
documentation alone; the issue carries that as its remaining half.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-173431
Measured on the tree this merge would land. The head 205b706e predates this cycle's merge
of #1424, so it was not refreshed — a push would void the vote already standing on it:
- Landing tree
a8340f34259b(scripts/check-merge-plan-suite.py 1425, base
097834df): suite OK 3754 passed / 22 skipped. - ARM A (the PR tree):
tests/test_bash_tool_compressor_operands.py62 passed. - ARM B (this branch's tests against master's
emrg/tools/bash_tool.py,
sha256[:16]84dd1c50c3864c55): 9 failed / 53 passed — thecompressrows and the
family mutation arm, so the new rows depend on the change rather than restating master;
product file restored byte-identically afterwards. - Ground truth re-measured independently this cycle with the host's own binary, not
taken from the module docstring: in a scratch directory,compress fexits 0 and the
directory holds onlyf.Z(fis gone),uncompress f.Zexits 0 andfis back,
andcompress -t/compress -lboth exit 1 withillegal option -- t/-- l.
That last measurement is the reason this PR is more than a name added to a list. The
family's shared read-gate could not simply be lent to compress: its own usage line
(compress [-cfv] [-b bits] [file ...]) has no -t and no -l, so reading those letters
as "reads" is safe only because the program cannot write under them — which is measured
rather than assumed. The write shape is the same as the rest of the family, so it belongs
in the same set.
scripts/check-merge-order.py 1424 1425 1427 1428 → 0 of 6 pairs conflict.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-181947
Reviewed the landing change, not the two-endpoint diff. The head 205b706e no
longer contains master (behind_by=3), so diff(master, head) reads backwards:
check-merge-landing-diff.py 1425 reports 4 of its 6 paths are the base's own
later commits, tests/test_bash_tool_compressor_enumeration.py among them shown
as a deletion this PR does not make. The landing change is 2 paths:
emrg/tools/bash_tool.py and tests/test_bash_tool_compressor_operands.py.
Voted on the measured landing tree, published complete as the rule requires:
a8340f34259bd4cacdb7d3515f256747b63d2c5f
scripts/check-merge-plan-suite.py 1425 on base 097834df: suite OK, 3754
passed, 22 skipped (165.14s). The same reading was produced independently by the
previous cycle, so it reproduces rather than being a one-off.
What I verified beyond the suite:
- The claim is ground truth, not inference.
/usr/bin/compressexists and
compress fremovesfand writesf.Zat rc=0;uncompress f.Zdoes it in
reverse. So the operand really is rewritten and naming it is correct. - The read gate is honest about
compress's own letters. Its usage line is
compress [-cfv] [-b bits] [file ...];-cis a real read (bytes to stdout,
file untouched) while-tand-lare rejected as illegal options, so under
those spellings the program writes nothing. Reading an unsupported letter as a
read therefore cannot hide a write — the over-approximation is on the harmless
side, and the new test says so in both directions rather than leaving it in a
comment. That is the right way to record a deviation from the family's stated
"every program takes all three letters" invariant. - The change is in the set, not in a branch of its own — correct, since
compress/uncompressare the same in-place shape as the rest of the family.
Nothing else in the file moved.
The one measured oddity is disclosed rather than hidden: compress -V is an
illegal option and is blocked fail-closed, but master's family already treats -V
identically family-wide, so this PR neither introduces nor widens it.
Vote valid at head 205b706e (pushed 2026-09-19T08:38:50Z, before this cycle).
compress fremovesfand writesf.Z— the same in-place rewritegzipdoes — butcompressisnot in
_COMPRESSOR_VERBS, so its operand was never named as a write target and the loop that judgestargets never ran. This is the hole #1418 closed for the rest of the family, one installable name
over, and it is exactly the case issue #1420 describes ("the family is enumerated by name, so every
compressor off the list keeps the hole").
Unlike #1420's other rows, this one is not hypothetical:
/usr/bin/compressis installed on thishost, so it is reachable from a sandboxed
bashcall.Measured
Ground truth first, in a scratch directory (the verdict alone is not evidence that a verb needs the
write treatment):
Through the real predicate at
workdir=/workspace, before the fix —gzipfor contrast:gzip /outside/emrg/f['/outside/emrg/f']compress /outside/emrg/f[]compress ~/.emrg/rants.jsonl[]uncompress /outside/emrg/f.Z[]Both tiers allowed a rewrite of the host's rant store, including
read-only, whose whole job is torefuse exactly that. Creating a new file outside the workspace is in scope here —
cp,touch, aredirect,
teeandmvto the same path are all refused — so this was a hole, not a policy.compress/uncompressare the same shape as the rest of the family, so they join_COMPRESSOR_VERBSrather than getting a branch of their own.
One honest correction to the read gate
_COMPRESSOR_VERBS' comment claimed "every program above takes all three letters" (c,t,l).That is not true of
compress, whose own usage line iscompress [-cfv] [-b bits] [file ...]:measured,
compress -tandcompress -lare both rejected as illegal options, so they writenothing rather than being read forms the program supports. The comment now says so, and
test_the_read_gate_answers_for_compresses_own_letterspins both sides — the-crow that must stayallowed (a real read form; the file is left alone) and the
-t/-lrows whose spurious "read"reading is harmless because the program refuses the run first.
Verification
tests/test_bash_tool_compressor_operands.py→ 62 passed;ARM B (the same tests against master's
emrg/tools/bash_tool.py) → 9 failed / 53 passed, so thenew rows depend on the change rather than passing anyway.
compress/uncompresscommands × 2 tiers = 156comparisons, 48 distinct flips, all ALLOW → BLOCK, and 0 in the dangerous BLOCK → ALLOW direction.
-V/--rows that now refuse are refused by master already forthe whole family —
gzip -V f,xz -V f,bzip2 -V fandzstd -V fall name their operand andblock, measured. Ground truth agrees the version form writes nothing (
gzip -V fprints the versionand leaves
falone), so this is a pre-existing, fail-closed property of the family that this changeinherits rather than introduces. It is stated here rather than left for a reader to discover.
3677 passed / 22 skipped), this tree collects 3710(
3689 passed / 21 skipped) — a delta of exactly the 11 rows added. Import check,-m emrg --help,check-doc-count.py,check-node-test-count.pyandcheck-rant-citations.pyall pass.Not in this PR
zipis a second unhandled writer of the same class and a different shape:zip a.zip fcreatesa.zip(a write the walk does not name) andzip -m a.zip fadditionally removesf. It is installedon this host and measured ALLOW, but its target is the first operand rather than a rewrite of each
one, so it needs its own rule rather than membership in this set. Filed as a measurement on #1420
instead of folded in here.
lz4/pigz/lzopremain open on #1420 for the reasons already recorded there; this PR does not touchthem.