Uh oh!
There was an error while loading. Please reload this page.
fix(rest): stop answering a terminal 404 for a failed REGISTRY read on GET /api/v1/packages/:id - #11602
Conversation
…n GET /api/v1/packages/:id (#11376) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
…confirmation (#11376) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f3da516d8a9fcfeae791afef678328d756bc08e4 && git checkout f3da516d8a9fcfeae791afef678328d756bc08e4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6d3550961f57d2b5efcbb5de5dd45c0090363d03 fc2f9a72988b496d8dbfc5f5447fa3487f0030db && git checkout -B drift-repro 6d3550961f57d2b5efcbb5de5dd45c0090363d03 && git merge --no-ff fc2f9a72988b496d8dbfc5f5447fa3487f0030db
node scripts/docs-audit/affected-docs.mjs --json 6d3550961f57d2b5efcbb5de5dd45c0090363d03
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#11376
What was wrong
packages/rest/src/package-routes.ts, theGET /api/v1/packages/:idhandler, tries the durablesys_packagesread first and falls back to the in-memory registry. The fallback sat in a bare catch:so when
protocol.getMetaItems({ type: 'package' })threw, control fell straight through to the line below and the door answered404 RESOURCE_NOT_FOUND—Package "<id>" was not found.This is the worse half of the family, not a smaller one. The list door's version of the same swallow (#11130) answered a
200whosetotalmerely under-counted. This one answers a terminal negative fact:404/RESOURCE_NOT_FOUNDis not "the answer may be incomplete", it is "this package does not exist", and a caller acts on it — an installer decides the package is not installed and offers to install it, a console hides the entry, a script branches to the create path. The producer's own words for the same condition are the opposite: "whether this item exists is unknown".It is also #5532's defect resurfacing one layer up.
ObjectStackProtocolImplementation.getMetaItemswas taught not to report an unreadablesys_metadataas "that item does not exist"; this consumer-side catch then re-applied precisely that relabelling to the protocol's answer.The direction is inherited, not designed here
Per the triage ruling on the card, this transplants the shape #11063 and #11130 already landed in this same file (which in turn transplant #10965 / #10677): the producer already declares the refusal — every non-benign
sys_metadataoverlay read failure leavesgetMetaItemsthroughrethrowUnlessMetadataStoreUnprovisioned→metadataStoreUnavailableError, i.e.SERVICE_UNAVAILABLE/ 503 with an ADR-0112 status+code on the error — so the whole repair is delete the catch and let the existingsendThrownErrorcarry the producer's own status and code. No third shape was invented and no new error taxonomy was introduced.Standing family ruling — #10965 · #10677 / PR #10788 · #10789 / PR #10964 · #11063 · #11130: a read that could not happen must not be reported as a read that found nothing.
The pins are the DISCRIMINATION, both directions
New:
packages/rest/src/package-id-registry-read-refusal.test.ts— 11 cases. Asserting only the new branch would leave "turn everything into a 500" passing, so the control half is as load-bearing as the fix.503 SERVICE_UNAVAILABLEin the declared envelope; the terminal 404 words are gone (no404, noRESOURCE_NOT_FOUND, no "was not found"); an undeclared throw is500 INTERNAL_ERROR, not a 404404 RESOURCE_NOT_FOUND; CONTROL an absent protocol service is an absence, not a failed read, and still reaches that same 404; CONTROL a registry hit is still200/source: 'registry'; CONTROL a durable hit is still200and never consults the registry (spied, not inferred); and the discrimination stated directly — an outage and a clean-empty registry must not answer alikeEvery refusal is asserted as
codeandstatusin the ADR-0112 envelope — no baretoThrow(), no status on its own. No wire field is added and none is asserted.Anti-vacuity — predicted before running, measured after
Fix committed first, then only
package-routes.tsreverted toorigin/mainunder atrap … EXIT INT TERM. Mutation proved on disk by anchored greps in both directions (an editor's exit code proves nothing; a zero-hitsedexits 0):No rebuild between legs, and the claim was checked rather than assumed: the mutated symbol is reached by the relative import
./package-routes.jsinside this package, which vitest transforms from source; the onlyexports-resolved workspace deps in the suite (@objectstack/spec/api,@objectstack/types) are untouched by the mutation. An all-green ablation leg would have been the stale-artifact signature — it is not what happened.Tests 6 failed | 5 passed (11)— as predictedPredictions are left as written. One thing did go wrong and is recorded rather than tidied away: the trap's own restore confirmation was void on the first run — the trap fired with cwd inside
packages/rest, so both thegit checkout <branch> -- packages/rest/src/…restore and itsgit diff --quiet -- <same path>check got a pathspec relative to the wrong directory, git matched nothing,git diffreported "no differences", and the script printedRESTOREDover a tree that was still mutated and staged. It was caught by re-running the greps from the repository root, and the tree was restored for real before anything else was measured. The lesson is left in the test file's header for the next ablation in this package: a restore leg confirms nothing unless its pathspec resolves.Clause-②— measured: noThe card changes what a caller receives when a registry read fails. Three legs, measured rather than inherited from the family precedent:
packages/specenumerates no response/status set for this route —packages/spec/src/api/package-api.zod.tsdeclares request/response bodies and no HTTP statuses, and the ADR-0112 error-code ledger namesRESOURCE_NOT_FOUNDonly as ashadowsentry, not as a per-route enumeration.503and500were already reachable on this exact route pre-fix, through the durable read (PackageService.get()→ outer catch →sendThrownError, since service-package answers "no such package" / "no packages installed" over a driver it never queried — its own normalizeRows maps a non-answering seam onto zero rows #10965). Measured by the §3 control, which is green on both ablation legs.⇒ a wrong answer replaced by a right one; accept set unchanged.
needs:contract-reviewnot applied. PR left draft, no ready-flip, no auto-merge, no enqueue.One file beyond the declared surface, declared
packages/rest/src/package-door-5xx-message-sanitization.test.ts— comment only, no assertion touched. Its §2 header enumerates which sources in this registrar reach the outer catch, and carried "Still true of the REGISTRY source:protocol.getMetaItemskeeps its own inner catch". That is already false inmain(#11130 removed the list door's and did not update the note — verified against PR #11378's own changed-files list), and this card makes it false for the second and last registry read. Bounded in-place exemption, all four conditions measured: same defect class (a statement about the very catch being deleted), mechanical with the correct shape fixed by the tree's own code, no other claim on it (#11590 is the only other in-flightpackages/restPR and its changed-files list does not include it), same derived gate family with no new verification surface. Declared on the card at #issuecomment-5392195396 before push.Verification — all at
fc2f9a72, worktree clean, each gate quoted by its own verdict linepnpm --filter @objectstack/rest typecheck→TYPECHECK_EXIT=0packages/restsuite →SUITE_EXIT=0·Test Files 141 passed (141)·Tests 2239 passed (2239)pnpm lint(repo-wideeslint . --no-inline-config) →os-verify-lock: VERDICT command-exit 0, no findingsGate set derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(gate list derived from the tree of 'objectstack-ai/objectstack' at commit fc2f9a72; 4 paths vs merge base2dc0a770b), not from a hand-built list. All 23 run to a real verdict, none refused:17 path-derived + 6 convention-triggered — each gate's own verdict line
Exit codes captured before any pipe (
cmd > file 2>&1; ec=$?), never off atail.check:type-check-debtreports two pre-existing-1surpluses (plugin-approvals,runtime) asℹinformational; neither is in this diff's packages and the gate exits 0.Generated by Claude Code