Skip to content

fix(rest): stop absorbing a failed registry read into a 200 on GET /api/v1/packages - #11378

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11130-packages-registry-read-refusal
Aug 23, 2026
Merged

fix(rest): stop absorbing a failed registry read into a 200 on GET /api/v1/packages#11378
os-zhuang merged 1 commit into
mainfrom
claude/issue-11130-packages-registry-read-refusal

Conversation

@claude

@claudeclaudeBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11130

The GET /api/v1/packages list door merges two sources — the in-memory registry, read through protocol.getMetaItems({ type: 'package' }), and the durable sys_packages rows, read through PackageService.list(). #11063 stopped the door swallowing a failure of the durable half. The registry half still carried its own bare catch {} commented "Protocol unavailable — continue with database only", so the exact ambiguity #11063 closed stayed open on the other source: when getMetaItems threw, the door answered 200 with { packages, total } built from the database alone, and total was presented as a complete count either way. The surviving entries kept source: 'database', which reads as provenance rather than as a warning that the registry half is absent — nothing on the wire separated "these are all the packages" from "these are the packages I could still see".

Standing family ruling — #10965 · #10677 / PR #10788 · #10789 / PR #10964 · #11063: a read that could not happen must not be reported as a read that found nothing.

ESTABLISH-FIRST — the card left open whether this was live code at all

The card explicitly did not measure "whether a presentgetMetaItems has a reachable throw", and ruled that if it does not, the correct delivery is premise_still_valid: false plus a dead-defensive-code finding rather than a forced diff.

It does. The premise holds — and the producer already declares the refusal.

The live call path, traced end to end rather than assumed:

stepfile
ctx.registerService('protocol', new ObjectStackProtocolImplementation(…))packages/metadata-protocol/src/plugin.ts:251 (assembleMetadataProtocol)
protocol = ctx.getService<RestProtocol>('protocol')packages/rest/src/rest-api-plugin.ts:135
mountAndRecordDirectRoutes({ …, protocol })packages/rest/src/rest-api-plugin.ts:436
registerPackageRoutes(server, resolvePackageService, base, { protocol, … })packages/rest/src/direct-mount-composition.ts:126
await options.protocol.getMetaItems({ type: 'package' })packages/rest/src/package-routes.ts — the handler in question

The throw, in that implementation: getMetaItems wraps its sys_metadata overlay read and hands every failure to rethrowUnlessMetadataStoreUnprovisioned, which returns only for isMissingTableError and otherwise throws metadataStoreUnavailableErrorSERVICE_UNAVAILABLE / 503 with an ADR-0112 status+code on the error (#5532). That is the same envelope #10965 gave PackageService.list().

The measurement, driving the real ObjectStackProtocolImplementation with this door's own request ({ type: 'package' }), each line an observed reading:

MEASUREMENT outage type=package => THREW status=503 code=SERVICE_UNAVAILABLE
causeCode=ECONNREFUSED
msg="The metadata store could not be read, so whether
this item exists is unknown. Retry once the
metadata database is reachable."
CONTROL A healthy type=package => RESOLVED {"type":"package","items":[]}
CONTROL B no-such-tbl type=package => RESOLVED {"type":"package","items":[]}

The two controls are the point, not decoration. Control A shows the harness can observe a non-throw, so THREW is not an artifact of how the probe was driven. Control B shows the throw is discriminating: the one benign reason a registry read can fail — sys_metadata not provisioned yet, i.e. first boot — is not a throw on this path, so it is not the case that "everything throws in this harness". A zero would have been meaningless without them; so would this non-zero.

Independent corroboration that this is not a probe artifact: the same throw is already pinned on the real implementation, for the plural read, in packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts"the PLURAL read stops answering 'this environment declares none of these'" (#5532). The probe adds only that the type-agnostic path holds for type: 'package' specifically.

Route taken — (b), with its first leg already landed

The dispatch ruling is route (b): teach the producer to declare a refusal, then stop swallowing it — the #10965 shape. The measurement above says the producer leg is already done (#5532 did it), so what remained is precisely #11063's edit: stop swallowing. The bare catch {} around the registry read is removed and the declared refusal reaches sendThrownError, which carries the producer's own status and code through the declared envelope rather than re-deciding them.

Shape (c) not taken. No field, flag, or marker is added to the response body. Making the tolerance visible would widen the response contract, which is a decision this queue entry does not carry.

What changes, and what deliberately does not

  • Changes: a registry read that refuses now reaches the client as 503 / SERVICE_UNAVAILABLE in the declared envelope instead of a 200 whose total lied. A registry read that throws something undeclared is answered as the 500 INTERNAL_ERROR a fault deserves instead of being eaten — that arm was previously unreachable on this source.
  • Unchanged — an ABSENT protocol service. The if (options.protocol && typeof … === 'function') guard is untouched. A composition with no protocol service is an absence, not a failed read, and still answers 200 with the durable half alone. This is asserted, not assumed.
  • Unchanged — first boot.sys_metadata not being provisioned yet is not a throw on that path, it is an empty overlay.
  • Unchanged — the healthy merge. Two sources, source: 'registry' | 'database' | 'both', a truthful total.
  • Untouched — the durable half (GET /api/v1/packages swallows a failed database read into a 200 registry-only answer — the caller cannot tell a partial listing from a complete one #11063 landed it) and the dispatcher twin in packages/runtime/src/domains/packages.ts, which serves a single in-memory read and has nothing of this shape.

The pin, with both readings

packages/rest/src/package-list-registry-read-refusal.test.ts. Asserting "the door still answers 200" would pass on the old code, the fixed code, and a wrong fix, so every case pins the mechanism: status and declared code (never a bare toThrow()), that no total is reported over a read that failed, that both halves of the one merge answer the same outage identically, plus two overreach guards.

Pre-fix (anti-vacuity — the pin genuinely fails without the change):

 × answers the producer’s declared refusal (503 SERVICE_UNAVAILABLE), not a 200
× reports NO `total` over a read that failed — the corrupted complete count is gone
× answers the SAME failure identically whichever HALF of the merge refuses
× an UNDECLARED throw from the registry read is a 500 INTERNAL_ERROR, not a 200
AssertionError: expected 200 to be 503 // Object.is equality
AssertionError: expected 200 not to be 200 // Object.is equality
AssertionError: expected 200 to be 503 // Object.is equality
AssertionError: expected 200 to be 500 // Object.is equality
Test Files 1 failed (1)
Tests 4 failed | 2 passed (6)

All four reds read 200 where a refusal was owed — the defect's signature exactly. The 2 that pass pre-fix are the overreach guards (absent protocol service still 200 database-only; healthy merge still counts truthfully); they must stay green across the change, which is why they are green on both sides.

Post-fix:

 Test Files 2 passed (2)
Tests 11 passed (11)

Run together with the #11063 sibling package-list-durable-read-refusal.test.ts (6 + 5), so the durable half is shown unregressed by the same command.

Verification — union run on c4b8ec34, the final commit

Working tree clean at that sha (git status --porcelain empty), so the measured tree is the committed tree. Every gate below quotes its own printed verdict, with exit codes captured before any pipe.

gateverdict
@objectstack/rest full suiteTest Files 139 passed (139) · Tests 2207 passed (2207)
@objectstack/rest typechecktsc --noEmit — exit 0
pnpm lint (whole repo, eslint . --no-inline-config)exit 0 — no narrowing claimed
check:route-envelope✓ Express-style response modules — 4 module(s) discovered and audited … 2 conformant, 2 ratcheted, 0 exempt
check:type-check-debt (--re-measure)33 ledger entr(ies) re-measured in 243.1s, 1897 raw tsc error(s) total, none above its recorded number
check:engine-double-contractOK — 386 pinned, 133 in the DEBT ledger, 2 exempt
check:type-check-coverageOK — 65/78 workspace packages type-checked (plus the root)
check:slot-lookup · check:query-options-erasure · check:nul-bytesexit 0
check:cross-package-test-inputs · check:test-source-alias · check:type-source-resolutionexit 0
check:dispatcher-error-vocabulary · check:published-files · check:changeset-gate-self-tests · check:objectui-changesetexit 0
check-empty-changeset · check-changeset-no-major · check-adr-0087-registration · check-ci-filter-parity · check-plugin-teardown-shapeexit 0
scripts/docs-audit/check-affected-docs.mjsexit 0

Two notes on the ratchet, since the new test file lands in a ledgered layer. @objectstack/rest carries a TEST_DEBT entry (errors: 155) covering its hidden test tree, and the re-measure reports no entry above its recorded number with @objectstack/rest absent from the surplus list — so the added test file contributes zero tsc errors. The re-measure was run only after turbo run build --filter='./packages/*' --filter='./packages/*/*', because the gate refuses on an unbuilt closure and that refusal means not measured, never not applicable.

The gate list was re-derived from the actual change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (the script reads the merge-base diff itself), and the convention-triggered families it names for a new test file — query-options-erasure, type-check-coverage, type-check-debt, engine-double-contract — are all in the table above.

Filed, not fixed here

  • GET /api/v1/packages/:id swallows the same failed REGISTRY read — and answers a terminal 404 RESOURCE_NOT_FOUND for it #11376GET /api/v1/packages/:id swallows the same failed registry read and answers a terminal 404 RESOURCE_NOT_FOUND for it. Same family, same producer, same mechanical repair, but a different route with a different consequence (a terminal "does not exist" rather than a short list), and the dispatch ruling scoped this card to the list handler by name. Unassigned, for triage. That issue also records that the [#11063] comment beside the durable read overclaims when it says the detail door "has never had an inner catch" — true of its durable read, not of its registry read.

Changeset

.changeset/rest-package-list-registry-read-refusal.md@objectstack/rest: patch. User-visible: the wire answer changes on a failed registry read.


Generated by Claude Code

…pi/v1/packages (#11130)
The list door merges two sources -- the in-memory registry via
`protocol.getMetaItems({ type: 'package' })` and the durable `sys_packages`
rows via `PackageService.list()`. #11063 stopped the door swallowing a failure
of the durable half; the registry half still carried its own bare `catch {}`,
so the same ambiguity stayed open on the other source: a failed registry read
was answered as a 200 whose `total` claimed to be a complete count, with the
surviving entries marked `source: 'database'` -- provenance, not a warning.
Establish-first (the card left this open): a PRESENT `getMetaItems` DOES have
a reachable throw, and the producer ALREADY declares it. The live `protocol`
service is `ObjectStackProtocolImplementation`, whose `getMetaItems` routes
every non-benign `sys_metadata` read failure through
`rethrowUnlessMetadataStoreUnprovisioned` -> `metadataStoreUnavailableError`:
`SERVICE_UNAVAILABLE` / 503 with an ADR-0112 status+code on the error (#5532).
So route (b)'s producer leg was already landed and only #11063's edit remained.
An absent protocol service is untouched (still 200, durable half alone), first
boot is untouched (`isMissingTableError` is not a throw), and no wire field is
added -- shape (c) would be a contract decision this card does not carry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 735f5c7099d627cb31881ba6f797953b5db74420packageMentionDocs.

Which tree this was computed on

This run read content/docs from 73af61c2ccb5a07f78731a0eac8cf2e29689a107 — the merge of head c4b8ec34ba4323fba6913dd9a05f73209885a357 into base 735f5c7099d627cb31881ba6f797953b5db74420, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 73af61c2ccb5a07f78731a0eac8cf2e29689a107 && git checkout 73af61c2ccb5a07f78731a0eac8cf2e29689a107
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 735f5c7099d627cb31881ba6f797953b5db74420 c4b8ec34ba4323fba6913dd9a05f73209885a357 && git checkout -B drift-repro 735f5c7099d627cb31881ba6f797953b5db74420 && git merge --no-ff c4b8ec34ba4323fba6913dd9a05f73209885a357
node scripts/docs-audit/affected-docs.mjs --json 735f5c7099d627cb31881ba6f797953b5db74420

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 23, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 23, 2026 15:11
@os-zhuang
os-zhuang added this pull request to the merge queueAug 23, 2026
Merged via the queue into main with commit 8519095Aug 23, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-11130-packages-registry-read-refusal branch August 23, 2026 15:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GET /api/v1/packages still absorbs a failed REGISTRY read into a 200 — the other half of #11063's two-source merge

2 participants

@os-zhuang@claude