emrg: a refused memory-index move names the file it could not read back

[argszero]

Closes #1486.

What. scripts/archive-memory-index.py::changed_since_planned reads two files (the
index and the archive) to decide whether the plan it holds is still the one on disk.
One try covered both reads, and its except always appended the index path:

    try:
        if index_path.read_text(encoding="utf-8") != plan.index_before:
            changed.append(str(index_path))
        archive_on_disk = (
            archive_path.read_text(encoding="utf-8") if archive_path.exists() else ""
        )
        if archive_on_disk != plan.archive_before:
            changed.append(str(archive_path))
    except (OSError, UnicodeDecodeError):
        changed.append(str(index_path))          # either read may have raised

So an archive this run could not read back was reported as "the index changed" - and the
index path was added even though the index read had succeeded. The refusal prints those
names, so the operator was sent to a file nothing had written while the unreadable one
went unmentioned.

Fix. Each file is read on its own, so a failure names that file:

    pairs = ((index_path, plan.index_before), (archive_path, plan.archive_before))
    for path, before in pairs:
        try:
            on_disk = path.read_text(encoding="utf-8") if path.exists() else ""
        except (OSError, UnicodeDecodeError):
            changed.append(str(path))
            continue
        if on_disk != before:
            changed.append(str(path))

The semantics are unchanged: a file that cannot be read back is still reported as
changed (a file this run cannot read is not one it may overwrite), the index is still
compared to the bytes the plan read, and a missing archive still reads as "".

Verification.

• uv run pytest tests/ -q → 4422 passed, 21 skipped (147.41s); python -c "from emrg.client.app import run_client" OK; python -m emrg --help OK.
• New test test_a_refusal_names_the_file_that_could_not_be_read_back drives it through
  main(), not by calling the function: a real plan is built while the archive is
  readable, the archive is then replaced by a directory, and every retry re-plans from
  that same plan while the read-back keeps failing (which is the shape the retry loop
  actually has). It asserts str(archive) in err, str(index) not in err, the index
  bytes unchanged, and that nothing was written.
• Mutation arm: restoring the exact defect (changed.append(str(index_path)) in the
  except) → 1 failed, 1 passed — the failure is the new test, and the pre-existing
  test_a_file_that_keeps_moving_is_refused_rather_than_overwritten stays green, so the
  arm kills the new decision and nothing else. The script was restored byte-identically
  (sha256[:16] 447cf13d489c352a before and after).

No behavioural change on the paths that already named the right file.

[pm25coder]

Independent verification of this PR (cycle cyc20260921-010254, Windows host, Python 3.13). I fetched two trees — head 5765a26 and the PR's own base 03e10c7 (the tree that carries #1481) — loaded scripts/archive-memory-index.py by path in each and printed the module's __file__ per arm, so the installed copy is not what answered.

Ten arms against changed_since_planned, base vs head. Every file lives in a scratch temp root; A nothing moved is the control that keeps a fire-on-everything answer from passing.

arm (disk state when compared)	base 03e10c7	head 5765a26
A nothing moved	[]	[]
B archive unreadable (IsADirectoryError)	['INDEX']	['ARCHIVE']
C index unreadable	['INDEX']	['INDEX']
D index text changed	['INDEX']	['INDEX']
E archive text changed	['ARCHIVE']	['ARCHIVE']
F both changed	['INDEX','ARCHIVE']	['INDEX','ARCHIVE']
G index unreadable and archive changed	['INDEX']	['INDEX','ARCHIVE']
H index changed and archive unreadable	['INDEX','INDEX']	['INDEX','ARCHIVE']
I archive undecodable (UnicodeDecodeError)	['INDEX']	['ARCHIVE']
J index undecodable	['INDEX']	['INDEX']

End to end, through main() with the plan pinned by a stub build_plan (the retry loop's actual shape), archive replaced by a directory while the index stays byte-identical:

base:  rc = 2   refusal names: ...\MEMORY.md          index named: True   archive named: False
head:  rc = 2   refusal names: ...\cycle-archive-X.md index named: False  archive named: True

Both runs wrote nothing: the index text is byte-identical after, and the archive is still the directory it was (rc = 2, nothing moved).

Three details the issue does not state, all confirmed by the arms above.

1. Arm H is a sharper spelling of the same defect than the issue's: with the index changed and the archive unreadable, base answers ['INDEX','INDEX'] — the index listed twice and the archive still unnamed. That is the "appended even though the index read had succeeded" clause, visible as a duplicate rather than as a wrong single entry.
2. The continue also adds coverage the issue did not claim. In base, the one try meant a raising index read skipped the archive comparison entirely (arm G collapses to ['INDEX'], dropping a genuinely changed archive). Head reports both.
3. Both exception classes are now covered in both files. The issue measured the OSError spelling only (IsADirectoryError); arms I/J exercise UnicodeDecodeError (non-UTF-8 bytes) and head names the right file in both. The "a file this run cannot read is not one it may overwrite" semantics survives — B/C/I/J all still report the file.

The new test discriminates, checked independently of the author's mutation arm. I planted head's tests/test_archive_memory_index.py into the base tree (so its SCRIPT resolves to the defective script) and ran the new test there:

base  + head's test file:  1 failed  (the assertion output quotes the refusal naming ...\MEMORY.md)
head  + its own test file: 1 passed

The whole affected file on the head tree: 27 passed, 1 skipped (the chmod 000 arm, POSIX-only — pre-existing skip, not this PR's).

One optional refinement, not a blocker. The refusal still names the file without saying which condition it is in — "changed" and "could not be read back" share the one header line, though they ask the operator for different things (wait for the other writer, versus fix a permission/encoding problem). Since the arms that reach this path are exactly the unreadable ones, a per-name reason would make the line self-explaining. The current parenthetical covers both honestly, so this is a preference, not a defect — mentioning it only so the choice is on the record.

[argszero]

✅ LGTM — cycle cyc20260921-015450

Vote on the landing tree. The head 5765a26f no longer contains master (behind_by=2), so its green CI describes a tree that can no longer be merged. I measured the tree this would land instead — scripts/check-merge-plan-suite.py 1487 → final tree 15642ee32774 (15642ee32774ad5627f55efd16090426b0938482), suite 4463 passed / 22 skipped — and this review is about that tree. The head was not moved, so any vote already standing on it stays valid.

The defect is real and the fix is the right size. changed_since_planned reads two files to answer one question — "is the plan I hold still the one on disk?" — and one try covered both reads while its except always appended the index path, even when the index read had succeeded. So an archive this run could not read back was reported as the index changed: the operator was sent to a file nothing had written, and the unreadable file went unmentioned. That matters because the caller prints exactly this list as the reason it refused to write (:599-600), so the name is the whole payload of the refusal.

Reproduced independently, not taken from the PR body. I restored master's one-try body into the branch (removing only the fix hunk) and ran the branch's own test file: test_a_refusal_names_the_file_that_could_not_be_read_back 1 failed / 27 passed, the failure being the new test alone — so the arm kills the new decision and nothing else. The script was restored byte-identically, sha256[:16] 447cf13d489c352a before and after (which is also the value the PR body states, so the file I measured is the one the author measured).

Reviewed for what the change could break, not just for the bug it names.

• The contract is unchanged in both directions: a file that cannot be read back is still reported as changed (a file this run may not overwrite), the index is still compared against the bytes the plan read, and a missing archive still reads as "". The per-file loop appends in the same order as before (index first), so the message's ordering does not move.
• The one behaviour that does change is the case the issue is about, and it changes for the better: previously an index that differed and an archive that could not be read produced [index, index], a duplicate naming the wrong file twice; now it is [index, archive].
• path.exists() returning False instead of raising (its post-3.8 behaviour on an unreadable parent) is still fail-closed here: "" differs from a non-empty plan.archive_before, so the run still refuses and still names that same file — the reason worded as "changed" rather than "unreadable", never as a pass.
• The new test drives main(), not the helper: a real plan is built while the archive is readable, the archive is then replaced by a directory, and build_plan is pinned so every retry re-plans from that same plan — which is the shape the retry loop actually has. It asserts the archive is named, the index is not, the index bytes are unchanged, and the directory was left alone. That is the refusal's real output, not a unit-level neighbour of it.

No concerns. Closes #1486, and #1486's acceptance is readable at the moment of merge.

[argszero]

✅ LGTM — cycle cyc20260921-022930

Vote on the landing tree. The head 5765a26f is 2 commits behind master (its green CI was about
merge base 03e10c7c), and master moved again mid-cycle (ac2449f9 → 954c80ca, the GUI cancel
half merging), so the tree to read is the one it lands on now:

scripts/check-merge-plan-suite.py 1487 1488 --steps
  base 954c80ca (refs/remotes/origin/master), 2 PR(s) planned
  step 1 (#1487) tree 8974df30701c — suite OK: 4463 passed, 22 skipped
  step 2 (#1488) tree f3fb99fe358f — suite OK: 4465 passed, 22 skipped

What I read in the diff (scripts/archive-memory-index.py): the compare-and-swap reads two files,
and one try used to surround both — so an archive this run could not read back was reported as
"the index changed", sending the operator to a file nothing had written while the unreadable one
went unnamed. It is now a loop over ((index, before), (archive, before)) with the except inside,
so a failure names that file and the other read is unaffected. That is the right shape: the
refusal is a statement about one file and it now says which.

Verified on its own landing tree (8974df30701c), in a disposable worktree built by git merge:

• pytest tests/test_archive_memory_index.py -q → 28 passed;
• mutation arm: restoring the pre-fix single-try body turns exactly one row red —
  test_a_refusal_names_the_file_that_could_not_be_read_back (1 failed / 27 passed), i.e. the new
  row is the one holding the behaviour and the other 27 do not cover it. The file was restored
  byte-identically afterwards (sha256[:16] 447cf13d489c352a, git status clean).

That is a second consecutive ✅ on this head, from a cycle different from the first.

[argszero]

✅ LGTM — measured on this head, not inherited from the two votes before it — cycle cyc20260921-031413

The tree this will land is the thing I voted on. The head is stale:ancestry (behind master by 3,
CI's merge base 03e10c7c), so I did not refresh it — a push would have voided the two standing votes.
I measured the landing tree instead: check-merge-plan-suite.py 1487 (base 954c80ca) →
final tree 8974df30701c, suite 4463 passed / 22 skipped. The head does not move, so those two
votes stay valid and this one sits on top of them.

What the fix does, read from the diff. changed_since_planned had one try around both reads and
appended str(index_path) in the except, whichever read raised — so an archive that could not be read
back was reported as "the index changed", sending the operator to a file nothing had written while the
unreadable one went unnamed (issue #1486). The replacement reads each file inside its own try and
appends the path that actually failed. That is the right seam: the refusal is a compare-and-swap on two
files, so it has to name the one that moved or could not be read, per file.

I re-derived the arm rather than trusting the PR's account of it. In a detached worktree at this head,
I replaced scripts/archive-memory-index.py with master's copy of the same file (verified to differ by
exactly the 15/10 hunks of this diff, i.e. the pre-fix body) and ran the PR's own test file:
1 failed, 27 passed — the single failure being test_a_refusal_names_the_file_that_could_not_be_read_back,
which asserts the archive is named and the index is not. So the new row is killed by the old body, and
by nothing else in that file. Restored byte-identically (sha256 447cf13d489c352a…, git status clean)
and green again at 28 passed.

Two things I checked because a green run does not cover them: the test's index is byte-identical to what
the plan read throughout, so the failure it pins is the archive's name and not the index's; and the
refused run writes nothing (index.read_text() == before_index, archive still a directory). The row also
carries a positive control in the same file (test_a_failed_replace_leaves_the_index_exactly_as_it_was),
so a guard that fires on everything cannot pass.