Skip to content

GET /api/v1/packages/:id swallows the same failed REGISTRY read — and answers a terminal 404 RESOURCE_NOT_FOUND for it #11376

Description

@os-zhuang

Found while implementing #11130 (the list door's registry half). Not fixed there — that card's dispatch ruling scoped the edit to the GET /api/v1/packages handler by name. Filed unassigned for triage.

The defect

packages/rest/src/package-routes.ts, the GET /api/v1/packages/:id handler, tries the durable read first and falls back to the registry:

// Fall back to registry (in-memory loaded packages)
if (options.protocol && typeof options.protocol.getMetaItems === 'function') {
  try {
    const result = await options.protocol.getMetaItems({ type: 'package' });
    const match = result?.items?.find((item: any) =>
      (item.manifest?.id || item.id) === packageId
    );
    if (match) {
      sendOk(res, { package: { ...match, source: 'registry' } });
      return;
    }
  } catch {
    // Protocol unavailable
  }
}

sendError(res, 404, 'RESOURCE_NOT_FOUND', `Package "${packageId}" was not found.`);

When getMetaItems throws, the catch absorbs it and control falls straight through to the line below. The door then answers 404 RESOURCE_NOT_FOUND — Package "<id>" was not found.

Same standing family ruling as its siblings — #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.

Why this is the worse half, not a smaller one

The list door's version (#11130) answered a 200 with a total that under-counted. This one answers a terminal negative fact: 404 / RESOURCE_NOT_FOUND is not "the listing may be short", it is "this package does not exist". 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 this condition are the opposite: "whether this item exists is unknown".

It is also the exact defect #5532 fixed one layer down, resurfacing one layer up. ObjectStackProtocolImplementation.getMetaItems was taught not to report an unreadable sys_metadata as "that item does not exist"; this consumer-side catch then re-applies precisely that relabelling to the protocol's answer.

The producer already declares the refusal

Measured on the live implementation while working #11130. The protocol service this registrar is handed is ObjectStackProtocolImplementation (packages/metadata-protocol/src/plugin.ts registers it; rest-api-plugin.ts resolves it and direct-mount-composition.ts passes it through). Its getMetaItems routes every non-benign sys_metadata overlay read failure through rethrowUnlessMetadataStoreUnprovisioned → metadataStoreUnavailableError: SERVICE_UNAVAILABLE / 503 with an ADR-0112 status+code on the error (#5532).

Driven with this door's own request:

outage (ECONNREFUSED)   type=package => THREW status=503 code=SERVICE_UNAVAILABLE
                                        msg="The metadata store could not be read, so whether
                                             this item exists is unknown. …"
healthy store           type=package => RESOLVED {"type":"package","items":[]}
no such table (1st boot) type=package => RESOLVED {"type":"package","items":[]}

So there is a declared envelope here waiting to be let through, and removing the catch is mechanical — the same edit #11063 made for the durable half and #11130 made for the list door's registry half. sendThrownError already carries the producer's status and code.

Worth stating: the in-tree comment currently overclaims

The [#11063] note beside the durable read asserts:

GET /packages/:id next door has never had an inner catch, so it has answered that same 503 since #10965.

True of that door's durable read. Its registry read is the block quoted above. Whichever way this is triaged, that sentence wants a qualifier.

Not measured here

  • Whether a caller depends on the 404. The behaviour change is narrower than the list door's (it only moves the answer on a failed read, never on a genuine miss), but the 404 → 503 flip on this route was not surveyed against consumers.
  • The DELETE /api/v1/packages/:id handler was not read for this shape.

Refs

#11130 · #11063 · #10965 · #5532 · #10677 / PR #10788 · #10789 / PR #10964 · ADR-0112

Activity

  1. added theissue type on Aug 23, 2026
  2. claude commented on Aug 24, 2026

    @claude
    Contributor

    Triage (daily round, session session_01Kktexqp6uVuFMztvvTMf3V, 2026-08-24): lands in packages/rest/src/package-routes.ts :id handler → domain:cli + pm:queue, Bug. Inherits the standing family ruling (#10965 / #10677 siblings): a failed REGISTRY read must not be answered as a terminal 404 — same repair shape, transplanted.


    Generated by Claude Code

  3. self-assigned this
    on Aug 24, 2026
  4. os-zhuang commented on Aug 24, 2026

    @os-zhuang
    ContributorAuthor

    Claim — domain:cli seat

    Session session_019siH5jDmk5hrayvfyojUqR, branch claude/issue-11376-package-id-registry-read. pm:queue → pm:dispatched + assignee in one stroke, read back.

    Serial released: this card shares packages/rest/src/package-routes.ts with #11378 (#11130), which merged. Re-verified from open PRs, not a branch sweep: no open PR modifies that file — in-flight #11590 (#10345) lands in error-response.ts, confirmed from its own changed-files list.

    Declared file surface: packages/rest/src/package-routes.ts (the :id handler) and its test twin.

    Direction is inherited, not fresh

    Triage (5389685834): "Inherits the standing family ruling (#10965 / #10677 siblings): a failed REGISTRY read must not be answered as a terminal 404 — same repair shape, transplanted."

    ⛔ This is not an invitation to redesign the error taxonomy. Read what #10965 and #10677 actually did and transplant that shape. If the sibling repair does not fit this handler for a reason you can measure, stop and report rather than inventing a third shape — a family that has answered the same question twice does not get a new answer on the third site without a ruling.

    Clause-② — measure and declare honestly

    The card changes what a caller receives when a REGISTRY read fails: a terminal 404 RESOURCE_NOT_FOUND becomes something that does not claim the resource is absent. Whether that widens the accept set or the public surface is your call to measure, not to assume — the family precedent suggests no (a wrong answer replaced by a right one, accept set unchanged), but say what you find. If it comes out yes, apply needs:contract-review to card and PR, read the label back after the size-labeler settles, and keep the PR draft.

    Pins

    The defect is that a failed read is indistinguishable from an absent resource. So pin the discrimination, both directions: a genuine miss still answers 404, and a failed REGISTRY read does not. Asserting only the new branch would leave "make everything a 500" passing.

    Anti-vacuity: shown failing without the fix. Prove the mutation on disk by grepping both the injected and the removed text — an editor exit code proves nothing and a zero-hit sed exits 0 — and carry trap … EXIT INT TERM. Predict the red/green split before running, and if the prediction is wrong, record that it was wrong rather than fitting it to the measurement.

    Delivery

    Changeset. Push, open a draft PR with Fixes #11376, run the repo's fast checks plus the packages/rest suite, derive the gate set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (⛔ not a hand-built path list), and quote each gate's own verdict line. A gate that refuses is NOT MEASURED — report it refused, and re-run it to a real verdict if you can build what it needs.


    Generated by Claude Code

  5. os-zhuang commented on Aug 24, 2026

    @os-zhuang
    ContributorAuthor

    Declared file surface — one increment, same round

    Session session_019siH5jDmk5hrayvfyojUqR, branch claude/issue-11376-package-id-registry-read.

    Claimed surface was packages/rest/src/package-routes.ts (the :id handler) and its test twin. Adding one file under the bounded in-place exemption, declared here before push rather than after:

    • packages/rest/src/package-door-5xx-message-sanitization.test.ts — comment only, no assertion touched.

    Why it is in-class rather than scope creep. That file's §2 header enumerates which data sources in this registrar reach the outer sendThrownError and which are absorbed. It carries:

    ⚠️ Still true of the REGISTRY source: protocol.getMetaItems keeps its own inner catch, which #11063 deliberately did not touch …

    That sentence is already false in main — #11130 (PR #11378) removed the list door's registry catch and did not update this note (verified against that commit's own changed-files list: package-routes.ts, the new pin file, the changeset — nothing else). This card removes the detail door's, which makes it false for the second and last registry read in the registrar. Leaving it would ship a PR that makes an in-tree statement more wrong, in the one file whose job is to say where the catches are.

    The four conditions, each measured rather than asserted:

    1. Same defect class — it is a statement about the very catch this card deletes, and it is the same class as the overclaim the card itself flags in package-routes.ts (the [#11063] note saying the :id door "has never had an inner catch"; that one is inside the declared surface and is qualified in the same commit).
    2. Mechanical, shape fixed by existing evidence — the correct sentence is determined by the tree's own code, not by a judgment call.
    3. No other claim holds it — re-verified from open PRs, not a branch sweep. fix(rest): serve a sandboxed hook refusal's declared code at every status, not only where a bespoke arm caught it first #11590 (rest: the /api/v1/data write path renders code only for 400 — a 409 refusal reaches the client with the status but no machine-readable code #10345) is the only other in-flight packages/rest PR; its changed-files list is error-response.ts, rest-hook-refusal-code-parity.test.ts, rest-hook-script-fault-envelope.test.ts, rest-sandbox-declared-status.test.ts, rest-thrown-code-vocabulary.test.ts, rest.test.ts and a changeset. No overlap.
    4. Same gate family, no new verification surface — same package, same derived gate set (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack names it under the same packages/rest/src/** and packages/** sources as the two declared files; it adds no family).

    Named in the PR body with the same evidence, per the exemption's second obligation.


    Generated by Claude Code


    Generated by Claude Code

  6. claude commented on Aug 24, 2026

    @claude
    Contributor
    {
      "issue": 11376,
      "status": "done",
      "branch": "claude/issue-11376-package-id-registry-read",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11602",
      "premise_still_valid": true,
      "summary": "Verified the premise against origin/main before editing: the bare `catch {} // Protocol unavailable` around the registry fallback in the `GET /api/v1/packages/:id` handler was present exactly as the card quoted it, and control did fall through to `sendError(res, 404, 'RESOURCE_NOT_FOUND', ...)`. Transplanted the sibling shape rather than inventing a third one — #11063 (this door's durable half) and #11130 (the list door's registry half) both landed in this same file and both are the same edit: delete the consumer-side catch so the producer's already-declared refusal (SERVICE_UNAVAILABLE / 503 with an ADR-0112 status+code on the error) reaches the client through the existing `sendThrownError`. Added `packages/rest/src/package-id-registry-read-refusal.test.ts` (11 cases) pinning the DISCRIMINATION in both directions: a failed read is now 503 and an undeclared throw is 500, while a genuine miss, an absent protocol service, a registry hit and a durable hit all keep byte-identical answers. Also qualified the `[#11063]` note in `package-routes.ts` that the card flags as overclaiming (it asserted the `:id` door 'has never had an inner catch' — true only of its durable read), and, under the bounded in-place exemption declared on the card at comment 5392195396, corrected one comment in `package-door-5xx-message-sanitization.test.ts` whose 'the REGISTRY source keeps its own inner catch' sentence was already false after #11130 and would have been made more false by this PR. Clause-② measured as `no` (see open_questions for the three legs) so `needs:contract-review` was NOT applied; PR left draft with no ready-flip, no auto-merge, no enqueue.",
      "tests": "All measurements at commit fc2f9a72 with a clean worktree (`worktree clean? [0 path(s) dirty]` printed by the run itself). Heavy work serialized through `bash scripts/pm/os-verify-lock.sh -c '...'`; exit codes captured before any pipe (`cmd > file 2>&1; ec=$?`), never off a `tail`. (1) `pnpm --filter @objectstack/rest typecheck` -> `TYPECHECK_EXIT=0`. (2) `packages/rest` full suite -> `SUITE_EXIT=0`, `Test Files  141 passed (141)`, `Tests  2239 passed (2239)`. (3) `pnpm lint` (repo-wide `eslint . --no-inline-config`, NOT narrowed) -> `os-verify-lock: VERDICT command-exit 0`, no findings. (4) Gate set derived with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack`, which printed `gate list derived from the tree of 'objectstack-ai/objectstack' at commit fc2f9a72` and `4 path(s) vs merge base 2dc0a770b`; all 17 path-derived plus 6 convention-triggered gates run to a REAL verdict, NONE refused, all EXIT=0 — quoted by their own verdict lines in the PR body. `check:type-check-debt` refuses on an unbuilt closure, so the workspace was built first (`pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*' --concurrency=2` -> `Tasks: 70 successful, 70 total`) and it then answered `--re-measure: OK — 32 ledger entr(ies) re-measured in 247.5s, 1897 raw tsc error(s) total, none above its recorded number`. ABLATION: fix committed first (a071e500), then ONLY `packages/rest/src/package-routes.ts` reverted to `origin/main` under `trap restore EXIT INT TERM`. NO REBUILD between legs, and that claim was checked rather than assumed — the mutated symbol is reached by the relative import `./package-routes.js` inside the same package, which vitest transforms from source, and the suite's only `exports`-resolved deps (`@objectstack/spec/api`, `@objectstack/types`) are untouched by the mutation; an all-green ablated leg would have been the stale-artifact signature and is not what happened. MUTATION CONFIRMED ON DISK by anchored greps in BOTH directions, not by an editor exit code: injected `[#11376]` 2 -> 0, removed `^          // Protocol unavailable$` 0 -> 1, removed `^        } catch {$` 0 -> 1, with the script aborting if either count was wrong. Predicted BEFORE running (recorded in scratchpad PREDICTION.md): 6 red / 5 green, split 3/1/2 across the three sections. Measured: `Tests  6 failed | 5 passed (11)`, and the six failures are exactly the six predicted cases — prediction CORRECT, left as written. ⚠️ ONE PROCESS FAILURE, recorded rather than tidied away: the trap's own restore confirmation was VOID. The trap fired with cwd inside `packages/rest`, so its `git checkout <branch> -- packages/rest/src/...` restore and its `git diff --quiet -- <same path>` check both got a pathspec relative to the wrong directory; git matched nothing, `git diff` reported no differences, and the script printed `RESTORED: file matches committed HEAD` over a tree that was STILL MUTATED and staged. Caught only by re-running the greps from the repository root immediately afterwards (`injected '[#11376]' = 0 (expect 2)`, `TREE DIFFERS vs HEAD`); the file was then restored for real and re-confirmed from the root (`= 2`, `TREE MATCHES HEAD`) BEFORE any further measurement, so no reading above was taken on a mutated tree. The lesson is left in the new test file's header for the next ablation in this package.",
      "open_questions": [
        {
          "question": "Clause-② — does this widen the accept set or the public surface? (dispatch asked for a measurement, not the family's assumption)",
          "options": [
            "yes — a caller-visible answer changed on a route, so treat it as a contract change: apply needs:contract-review to card and PR, read the label back after the size-labeler settles, keep the PR draft",
            "no — a wrong answer replaced by a right one on a failure path, accept set unchanged, no declared surface widened"
          ],
          "recommendation": "no, on three measured legs. (1) ACCEPT SET UNCHANGED: no input that was refused is now accepted and none that was accepted is now refused — only the answer on a FAILED read moves. Measured, not argued: the four §2 controls (genuine miss, absent protocol service, registry hit, durable hit) are green on BOTH ablation legs, i.e. every input whose answer was already correct keeps a byte-identical answer. (2) NO DECLARED SURFACE WIDENED: no wire field is added, no route is added, and packages/spec enumerates no response/status set for this route — `packages/spec/src/api/package-api.zod.ts` declares request/response BODIES and no HTTP statuses (grepped for 404/503/RESOURCE_NOT_FOUND/SERVICE_UNAVAILABLE: only unrelated `status` enum members for package lifecycle), and the ADR-0112 error-code ledger names RESOURCE_NOT_FOUND once, as a `shadows` entry, not a per-route enumeration. (3) NO STATUS NEW TO THE ROUTE: 503 and 500 were ALREADY reachable on this exact route pre-fix via the durable read (PackageService.get() -> outer catch -> sendThrownError, since #10965) — pinned by the §3 control, which is green on both ablation legs. So `needs:contract-review` was not applied and nothing was flipped ready. If the PM reads leg (3) differently — e.g. counts 'the status a caller sees for THIS input' rather than 'the status set this route can answer' — the label is one call away and the PR is still draft."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  7. os-zhuang commented on Aug 24, 2026

    @os-zhuang
    ContributorAuthor

    Clause-② ruled: no — and the disagreement the dev named is the right one to name

    domain:cli seat, session session_019siH5jDmk5hrayvfyojUqR. Answering the open question on PR #11602 so it is not left hanging on a label decision.

    The dev's three legs hold, and the third is where the real question lives. It offered the fork honestly:

    "If the PM reads leg (3) differently — e.g. counts 'the status a caller sees for THIS input' rather than 'the status set this route can answer' …"

    The route's answerable set is the right reading, and the per-input reading is wrong — not as a matter of taste, but because the per-input reading would swallow the rule that surrounds it. Every bug fix changes what some input sees; that is what makes it a fix. If that counted as widening the public surface, Clause-② would capture every bug repair on a route, and the skill's own mechanical test says the opposite: 「接受集不变、拉回已声明契约(declared = enforced 的恢复)⇒ bug/整理 ⇒ 代裁车道」.

    This card is that shape exactly. The producer already declared SERVICE_UNAVAILABLE / 503 with an ADR-0112 status and code; a consumer-side catch {} was eating it and answering 404 RESOURCE_NOT_FOUND instead. Deleting the catch does not add an answer the route could not give — leg (3) pins that 503 and 500 have been reachable here since #10965 via the durable read — it stops one input from being answered with someone else's answer. Declared, restored to enforced.

    Legs (1) and (2) carry their own weight and were measured rather than argued: four controls byte-identical on both ablation legs, and packages/spec enumerates no status set for this route at all (package-api.zod.ts declares bodies; the grep for 404/503/RESOURCE_NOT_FOUND/SERVICE_UNAVAILABLE found only unrelated package-lifecycle status members).

    ⇒ ⛔ needs:contract-review is not applied. The direction was inherited from #11063 / #11130, both in this same file, so there is no fresh contract judgement here for a reviewer to make.

    ⭐ The process failure in the report is the most valuable thing in it

    "the trap fired with cwd inside packages/rest, so its git checkout <branch> -- packages/rest/src/… restore and its git diff --quiet -- <same path> check both got a pathspec relative to the wrong directory; git matched nothing, git diff reported no differences, and the script printed RESTORED: file matches committed HEAD over a tree that was STILL MUTATED and staged."

    A restore confirmation can itself be false, and this one was false in the most dangerous way: it printed success. It was caught only because the on-disk anchored greps were re-run from the repository root afterwards — the same discipline that exists because a zero-hit sed exits 0, applied one layer further out. The tree was genuinely restored and re-confirmed before any reading was taken, so nothing above rests on a mutated tree.

    Reported rather than tidied away, and the lesson left in the new test file's header for the next ablation in this package. That is the behaviour this lane wants: a wrong prediction reported rather than fitted, and a false green named rather than quietly re-run.

    The prediction itself was correct — 6 red / 5 green called before running, and the six failures were exactly the six predicted.

    State

    ⛔ Not armed yet: CI at read time still had Lint & Repo Gates and three Dogfood Regression Gate shards in_progress. in_progress is not success, and the aggregate is only a negative screen. Arming waits for those to complete — the label question above is settled independently and does not gate on them.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions