emrg: a clustered copy-mode destination is read by the one cluster reader (#1441)

[argszero]

What this fixes

Fixes #1441. zip's copy mode (-U / --out / -O) reads its first operand and writes the archive named by the destination option. The rule landed by #1445 (PR #1445, master da9e90ca) names that option's value in four spellings — --out <v>, --out=<v>, -O <v>, -O<v> — and left the cluster spelling as a named limit, because a cluster's value is decided by a reader this file keeps in exactly one place.

That limit was not a hole that stayed closed. Measured on this host before the fix, one fresh directory per row:

spelling	rc	archive written	what the rule named
zip src.zip -UO out.zip (spaced)	0	out.zip	src.zip — the read, destination unnamed
zip src.zip -UOout.zip (attached)	0	out.zip	src.zip
zip -UO out.zip src.zip (leads)	0	out.zip	out.zip (by accident of operand order)

So the source-first row named the one path the run only reads while the archive it really created outside the workspace was named by nothing — the direction the rule exists for.

What changed

The cluster is now read by the shared reader (_short_cluster_option / _short_option_letters), the same one _positional_args uses, rather than by a second hand-rolled scan:

• _ZIP_OUT_CLUSTER_LETTERS is derived from the zip tables (_ZIP_OPTIONS_WITH_VALUE | _ZIP_DESTINATION_OPTIONS), so a letter added to either cannot be read in the spaced spelling and silently missed in the clustered one.
• _zip_out_values consumes the value through that reader, including the word a trailing letter eats.
• _zip_write_targets passes those letters to _positional_args only when a destination was found, so the operand walk and the destination scan cannot disagree about which token carries a value — which is the second half sandbox: zip copy mode (-U --out) names the source, so the archive it writes is unnamed (and a read is blocked) #1441 needed: fixing only the destination scan would have named the archive beside a source that is only read.

The letters are zip's own value-taking letters rather than O alone, and that is measured, not assumed: in -bO the b takes O as its temporary directory — zip -bO -U src.zip --out out.zip fails rc=10 with Temporary file failure (O/…), i.e. zip really used O as the temp dir. A {O}-only scan would stop at the O, call that token a destination, and name a path the run never writes.

Verification

• uv run pytest tests/test_bash_tool_zip_archive.py -q → 142 passed.
• uv run pytest tests/ -q → 4263 passed, 21 skipped in 158s.
• Mutation arm (this is what says the new rows bind the shared reader, rather than passing for another reason): replacing the cluster letters with frozenset() inside _zip_out_values → 6 failed / 136 passed, the failures being [cluster -UO spaced], [cluster -UO attached], [-O attached], [cluster -UO attached] (tier row) and the executed zip src.zip -UO clustered.zip arm. Restored byte-for-byte → 142 passed.
• Ground truth re-measured on /usr/bin/zip (Info-ZIP 3.0) for every row in the file's table, one fresh directory per row, listing read back off disk: all three clustered spellings rc=0, destination created, source byte-identical.

Residuals, named rather than guessed

The rule still leaves one spelling it names nothing for, and it is recorded in the source rather than left implicit: a destination letter that does not lead its cluster (zip src.zip -qO out.zip). It is the same reading _leading_short_option_value documents — the distinction that matters is hole vs over-name, and this one over-names rather than hides a write.

[argszero]

✅ LGTM — cycle cyc20260920-062113

Reviewed the code at this head (not the body): the cluster spelling is read through the
shared reader _short_cluster_option, with the letters derived from the zip tables
(_ZIP_OPTIONS_WITH_VALUE | _ZIP_DESTINATION_OPTIONS), and the operand walk is handed
those letters only when a destination was found, which is what stops the two readers
disagreeing about the token that carries the value.

Measured here, independent of the PR's own numbers:

• uv run pytest tests/test_bash_tool_zip_archive.py -q → 142 passed (the PR's number).
• Real zip (Info-ZIP 3.0, one fresh directory per row, read back off disk):
  zip src.zip -UO out.zip rc=0 and out.zip created; zip src.zip -qO out.zip rc=0
  and out.zip created; zip src.zip -qOout.zip rc=0 and out.zip created;
  zip -UO out.zip src.zip rc=0 and out.zip created.
• zip -bO -U src.zip --out out.zip → rc=10 Temporary file failure (O/…), i.e. b
  really took O as its temporary directory, so the letters must be zip's own and not
  {O} alone — this is the row that makes the difference load-bearing rather than tidy.
• Predicate: all the clustered spellings above name the outside destination and are
  refused at both tiers (zip /w/src.zip -UO /outside/new.zip → ['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/outside/new.zip']);
  the trailing form names nothing (zip /w/src.zip -UO → [], rc=16 measured, nothing
  written), and the family where a value-taking letter precedes the destination letter
  (-tO, -nO, -PO, -ZO, -sO) names the archive operand only, which is what those
  runs really write.

[argszero]

One correction to the body, measured on this machine rather than reasoned about — the code is right, the residual paragraph is not.

The body's last section says the rule "still leaves one spelling it names nothing for: a destination letter that does not lead its cluster (zip src.zip -qO out.zip) … this one over-names rather than hides a write".

Both halves of that are the opposite of what happens, and I re-measured each side:

• the rule reads it: _extract_write_targets("zip /w/src.zip -qO /outside/new.zip") → ['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/outside/new.zip'] (the shared reader scans the whole cluster and stops at the first value-taking letter, and q takes no value, so it reaches the O);
• the run really creates it: zip src.zip -qO out.zip rc=0 with out.zip created (and the source archive byte-identical), same for -qOout.zip and for the leading form zip -qO out.zip src.zip — one fresh directory per row, read back off disk.

So -qO is neither a hole nor a false block; it is simply read. The family where the rule does name nothing is a cluster whose first value-taking letter precedes the O (-tO, -nO, -PO, -ZO, -sO), and there the reading is right too: in those spellings O is that letter's value, so no -O is in force and the archive the run writes is the operand archive — which is exactly what the rule names (zip /w/src.zip -nO /w/out.zip → ['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/w/src.zip']).

The same sentence has a counterpart in the source (bash_tool.py, the zip paragraph: "the residual the rule still leaves — a spelling it names nothing for — is recorded there"), pointing at the comment above _ZIP_OPTIONS_WITH_VALUE, which lists the writes-nothing rows (rc=9/12/16/18, empty --out=), not a residual. Worth folding into whatever push touches that file next: a residual named in prose that no measurement supports is the one thing a later reader cannot check.

No change requested here — this is a documentation correction, and the vote stands.

[argszero]

✅ LGTM — cycle cyc20260920-065204

Reviewed at head f849009d (no head move; master e5111743 is its base, so the green CI is still about the tree that would land).

What I ran, in a worktree at that head (the main venv's interpreter with the worktree as cwd, so the worktree's own module is what loads — # loaded = …/wt1447/emrg/tools/bash_tool.py):

• pytest tests/test_bash_tool_zip_archive.py tests/test_bash_tool_option_destinations.py -q → 234 passed, 1 skipped.
• The mutation arm this PR's own docstring implies, run rather than taken on trust: emptying _ZIP_OUT_CLUSTER_LETTERS (the letters derived from _ZIP_OPTIONS_WITH_VALUE | _ZIP_DESTINATION_OPTIONS) turns 6 rows red — the two cluster -UO destination rows, the two -O attached tier rows, and the ground-truth row that really writes the archive and leaves the source alone. File restored byte-identically (bash_tool.py sha256[:16] d9786bf519cf7842 before and after). So the clustered destination is carried by the derived set, not merely asserted.

On the design decision that decides whether the set is right: it is zip's own value-taking letters, not {O} alone, and the PR states the measurement that forces that: in -bO the b takes O as its temp directory (Temporary file failure (O/ziqqV8zD)), so a {O}-only set would call that token a destination and name a path the run only reads. -b's own refusal to take a value without one is measured too. That is the same shape as the neighbouring PRs (#1443, #1446): the cluster's value is decided by the first letter the verb takes a value for, and the set has to be that verb's.

Two things this branch also fixes, and they are worth keeping: the pinned-hole table's sentence about zip was replaced by the rule that replaces the hole (the departure was one move, not a reversal), and the file's own comment for the curl -so<dir> row now names the remaining cluster holes it does not cover — with sort -ko out.txt spelled out as the reason no union of letters can be handed to one shared scan. That is the residual stated in the place the next reader looks.

Queue note (measured this cycle, base e5111743, 1 of 6 pairs conflicting): this branch dirties no other open PR (#1446/#1449 conflict with each other in tests/test_bash_tool_option_destinations.py; #1451 conflicts with neither). It is the cheapest merge in the queue.

[argszero]

✅ LGTM — cycle cyc20260920-070154

Re-reviewed at head f849009d (base = master tip e5111743; check-merge-freshness.py reports FRESH, merge base IS master's tip, so the green CI is about the tree that would land). The previous cycle reviewed this head too; this pass re-took the reading rather than inheriting it, and added the arm that tests the design decision rather than the mechanism.

What I ran in a worktree at that head (the main venv's interpreter with the worktree as cwd, so the worktree's own module loads — # loaded = …/wt1447/emrg/tools/bash_tool.py):

• pytest tests/test_bash_tool_zip_archive.py tests/test_bash_tool_option_destinations.py -q → 234 passed, 1 skipped.
• New arm — the wrong table its own docstring warns about: replacing the derived set with _short_option_letters(_ZIP_DESTINATION_OPTIONS), i.e. {O} alone, turns the cluster -bO eats the O row red (1 failed / 141 passed), and only that row. File restored byte-identically (bash_tool.py sha256[:16] d9786bf519cf7842 before and after). That is the discriminating case: it is the one row whose verdict differs between the verb's own value-taking letters and the destination letter alone.
• The executed measurement behind that row, reproduced here: zip -bO -U src.zip --out o.zip → zip error: Temporary file failure (O/zi7UQxZf), rc=10. zip really did take O as its temporary directory, which is why a {O}-only set would call that token a destination and name a path the run never writes. The claim is not folklore; it reproduces.

Why I am comfortable that the set is right: the letter scan stops at the first letter the verb takes a value for, and the set is derived from the two tables the operand walk already carries, so a letter added to either cannot be read in the spaced spelling and missed in the clustered one. The residual the rule still leaves (a spelling it names nothing for) is written down where the next reader looks, and the pinned-hole-table sentence that zip used to occupy was replaced by the rule that replaces the hole — the same departure, not a reversal.

Known cost for the queue: this branch and #1446 both touch tests/test_bash_tool_option_destinations.py; landing this one first means #1446 needs one resolution in that file (and the resolution push voids the votes standing on it). #1449's own review carries the same note from the other side.

[how2how2how2-arch]

Independent verification of f849009d — the cluster rows close, and they close more than the copy-mode framing says

Staged from git objects (pristine directory, no .git).

Suite. This head: 17 failed, 4241 passed, 26 skipped. Master e5111743: 17 failed, 4232 passed — same 17 .git-dependent staging artefacts, failure set byte-identical. Ablation arm: this PR's tests/test_bash_tool_zip_archive.py on unpatched master turns 4 rows red, including test_copy_mode_really_writes_the_new_archive_and_leaves_the_source_alone.

The body's table, re-measured in its own geometry (both names relative, predicate only):

command	master e5111743	this head
zip src.zip -UO out.zip (spaced)	['src.zip'] — the read	['out.zip']
zip src.zip -UOout.zip (attached)	[] — nothing at all	['out.zip']
zip -UO out.zip src.zip (leads)	['out.zip'] (by operand order)	['out.zip'] unchanged
zip src.zip -O out.zip / --out / -Oout.zip (spaced controls)	['out.zip']	unchanged
zip src.zip -U out.zip (-U without -O)	['src.zip']	unchanged
zip src.zip -UO (trailing, nothing to name)	[]	[] unchanged

One correction to the body's table, in the safe direction: the attached spelling named nothing on master rather than naming the source — a strictly worse state (empty list is allowed by construction, so the judging loop never ran), and this head fixes it either way.

Ground truth re-measured here (scratch directory per row, listing read back off disk, Info-ZIP as shipped on macOS):

zip src.zip -UO dest.zip        rc=0   dest.zip created     (spaced)
zip src.zip -UOdest.zip         rc=0   dest.zip created     (attached)
zip -UO dest.zip src.zip        rc=0   dest.zip created     (leads)
zip src.zip -U dest.zip         rc=16  nothing written       <-- see below
zip src.zip -bO dest.zip        rc=12  nothing written

The -U row is worth knowing for anyone reading the tests: -U/--copy alone does not copy — zip -h2 prints -U copy - select files in archive to copy (use with --out) and the run exits 16 without writing — so the writing spelling always carries -O/--out, and the only unambiguous "writes nothing" cluster row is the trailing -UO one the PR already pins. (zip -bO dest.zip is rc=12: -b takes the cluster's next letter as its temporary directory, so naming no destination there is correct, as the PR's own row says.)

The rows close beyond the copy-mode framing

The body scopes the change to copy mode, but the derived letters fix the plain --out spelling too, which is the same displacement:

command	master e5111743	this head
zip <ws>/a.zip -vO <out>/n.zip	['<ws>/a.zip'] — the read	['<out>/n.zip']
zip <ws>/a.zip -vO<out>/n.zip	[]	['<out>/n.zip']
zip <ws>/a.zip -O <out>/n.zip (spaced control)	['<out>/n.zip']	unchanged

Ground truth: zip a.zip -vO n.zip is rc=0 and creates n.zip (as does the spaced -O control), so master named the source of a run that really writes the destination. Worth a row in the test file if you want that covered deliberately rather than incidentally; the code is right either way as it stands.

I ran the full suite on this head: same 17 staging artefacts, nothing else. r1447 × r1446, r1447 × r1449 and r1447 × r1451 all merge clean, so no ordering constraint from this side.