Skip to content

fix(rest): stop absorbing a failed durable read into a 200 packages listing - #11132

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-11063-packages-list-swallowed-read
Aug 22, 2026
Merged

fix(rest): stop absorbing a failed durable read into a 200 packages listing#11132
os-elon merged 1 commit into
mainfrom
claude/issue-11063-packages-list-swallowed-read

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#11063

The defect, on this head

packages/rest/src/package-routes.ts (lines 642-658 on my head; the card recorded 642-657 — re-located, not copied). GET /api/v1/packages merged the in-memory registry with the durable sys_packages rows and wrapped the durable half in a bare catch {} commented "Database query failed — continue with registry-only packages".

A failed durable read was therefore reported as a read that found nothing: the door answered 200 from the registry alone, total was presented as a complete count either way, and the registrar-sourced entries kept source: 'registry' — provenance, not a warning that the database half is absent. Nothing on the wire separated "these are all the packages" from "these are the packages I could still see".

The stop-and-report branch: I looked, and the degraded 200 is NOT a load-bearing deliberate posture

The grading required stopping and reporting needs_decision if there were real evidence that the degraded 200 is a deliberate posture for a supported composition. The source comment does read as intent, so this was searched rather than assumed. Six places, and the finding is consistent across all of them:

  1. The composition gate already handles the "no durable store" posture — and it is not this catch. The three read/delete routes mount only when a package service resolves (registerPackageRoutes); when none does, the registrar sits out and the runtime dispatcher's twins at byte-identical patterns serve /packages instead. A deployment that composes no marketplace capability never reaches this handler at all, so the catch is not the mechanism serving any registry-only composition.

  2. The catch was unreachable from the day it was written. It arrived in 1147e48a1b (2026-04-17) together with the registry-merge loop it wraps. At that same commit PackageService.list() already caught everything and returned [] — it could not throw. The comment describes a condition that could not occur through the only in-repo implementation of the contract. It was defensive boilerplate around a new merge loop, not a designed degradation.

  3. Its entire live effect today is to swallow the refusal 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 just created.list() still swallows its own driver faults and answers [] (logging at error); the one throw it re-raises is the branded SERVICE_UNAVAILABLE / 503 seam refusal for a seam that accepted the query and returned no result set. That single throw is exactly what this catch was absorbing.

  4. The sibling door already refuses on the very composition in question.GET /api/v1/packages/:id has no inner catch, so it has answered that same 503 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 — including on a seam-unreadable stack (measured in packages/runtime/src/package-service.null-seam.test.ts). A registry-only posture that were load-bearing would already be broken there. The two doors disagreeing is the defect.

  5. No documented contract.content/docs/references/api/package-api.mdx describes no degradation, no partial listing and no registry-only posture; no ADR declares one. ADR-0112 governs the envelope, and SERVICE_UNAVAILABLE / 503 is the standard catalog pairing metadata-protocol already uses for exactly this condition.

  6. The one test that recorded the 200 is a characterization test, not a composition.package-envelope.conformance.test.ts pinned the degraded 200 with a comment saying it was recorded because it is why the 500 case above drives GET /:id instead. Its fixture throws a bare Error — a shape the real producer never emits post-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, since generic faults are swallowed inside list(). A hand-made fake in a unit suite is not a supported composition.

Consequence, stated plainly rather than buried: on a composition whose raw-SQL seam accepts statements and returns no result set, GET /api/v1/packages now answers 503 where it previously answered a registry-only 200. That is the intended result and the truthful one — the durable half genuinely cannot be read — and it is the same answer the sibling detail door has already given on that composition since #10965.

The fix

The durable read is no longer caught at this door; the outer catch hands the throw to sendThrownError, which carries the producer's own status and code through the existing declared envelope. An undeclared throw becomes a 500 INTERNAL_ERROR through the same envelope. A durable read that answers is unchanged: both sources still merge, source is still registry / database / both, and total is still the count of what was really read.

No wire field is added and no response shape changes. The alternative the card sketched — keep the 200 plus a declared partial-result marker — is a response-shape / public-surface change, i.e. a contract decision, and was not authorized by this grading.

The dispatcher's own /packages door — the in-scope read

Read, and it does NOT have the same catch, so it is untouched.packages/runtime/src/domains/packages.ts serves GET /packages from registry.getAllPackages() — a single in-memory registry read. It attempts no durable read, so there is no failed read to absorb and no swallowing catch; its total is an honest complete count of the only source it has. It refuses with a 503 when no registry is available at all, which is the same code family this fix routes through. Not the same one-line shape, so per the grading it is reported here rather than fixed.

Tests — the mechanism, not "still 200"

"The listing returns 200" passes on the old code, on the fixed code and on a wrong fix, so no pin asserts it. New file packages/rest/src/package-list-durable-read-refusal.test.ts:

  • the declared refusal reaches the client — code AND status (503 / SERVICE_UNAVAILABLE), in the declared envelope (BaseResponseSchema.safeParse + envelopeViolations), never a bare toThrow()
  • total is not reported at all over a read that failed, and the registry half is not served as if it were whole
  • the two read doors answer the same failure identically — the alignment the grading named as its basis
  • an undeclared throw is a 500 INTERNAL_ERROR (this arm was unreachable on this route before)
  • preservation: a durable read that answers still merges both sources, and total is still a true count — the half that keeps this from being "refuse always"

package-envelope.conformance.test.ts: the pin that recorded the degraded 200 was replaced, not re-spelled — it pinned exactly the branch this PR removes, so re-spelling it would have left an assertion that passes only because nothing is produced any more.

Reverse verification — prediction stated before the run

Fix committed first, then reverted with git restore --source=HEAD~1 (tree only; porcelain showed a lone M, never MM), then restored with git checkout HEAD -- and confirmed byte-identical (git diff HEAD clean).

Predicted 5 RED, 1 GREEN — the preservation pin insensitive by design. Observed exactly that:

× 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 on both read doors (#11063 alignment)
× an UNDECLARED throw from the durable read is a 500 INTERNAL_ERROR, not a 200
× GET /packages no longer degrades to a 200 registry-only listing when the durable read fails
Test Files 2 failed (2)
Tests 5 failed | 45 passed (50)

with AssertionError: expected 200 to be 503 on the mechanism pin. Restore leg: Test Files 2 passed (2) · Tests 50 passed (50).

No rebuild was needed for this ablation and none is claimed: the suites reach the subject through the relative specifier ./package-routes.js, resolved to this package's own source, not through a dependency's exports to dist/.

Verification — all at the final commit 7e81738847

Union re-derived with node scripts/pm/dispatch-gates.mjswith no path arguments (it derives its own change set; merge-base d806081dd, 5 paths, three-dot semantics). Every family it named was run, plus the convention-triggered set for adding test files. Each gate's own verdict line:

  • @objectstack/rest full suite — Test Files 136 passed (136) · Tests 2192 passed (2192)
  • @objectstack/rest typecheck — tsc --noEmit, exit 0 (script name echoed, so not a zero-match run)
  • check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through dist/
  • check-engine-double-contract: OK — 380 pinned, 133 in the DEBT ledger, 2 exempt.
  • check-dispatcher-error-vocabulary: OK — 21 unregistered code-stamping site(s), all classified
  • OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob. (cross-package-test-inputs)
  • check-nul-bytes: OK (scanned 6445 text file(s) ... no raw ASCII control bytes).
  • where-matcher conformance holds: 280 matcher(s) discovered ... 0 silently-wrong
  • slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new
  • route-envelope4 module(s) discovered and audited, exit 0 (the two ratchet lines it prints are pre-existing entries for query-allowlist.ts / query-multiplicity.ts, untouched here)
  • check-type-check-coverage: OK — 65/78 workspace packages type-checked
  • check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 214.8s, 1895 raw tsc error(s) total, none above its recorded number.

That last one matters for this PR specifically: @objectstack/rest carries a TEST_DEBT entry of 155, and a new test file is exactly what moves that ratchet — it did not. Its first run refused outright ("32 workspace dependencies have no built type entry point on disk"), which is NOT MEASURED rather than not-applicable, so the full closure was built as the gate instructs and it was re-run. ⛔ No ledger, baseline or threshold was edited anywhere in this PR.

Exit codes were captured before any pipe (cmd > file 2>&1; EXIT=$?), never read off a tail.

Out-of-scope finding

Filed #11130 (unassigned): the registry half of this same two-source merge still carries its own swallowing catch around protocol.getMetaItems, so the identical ambiguity remains open one producer over. Deliberately not fixed here — the grading scoped this card to the durable read, and unlike list() that producer has no declared refusal to let through, so the right wire answer is a real decision rather than a mechanical removal.

Bounded repair outside the strict defect surface, declared

packages/rest/src/package-door-5xx-message-sanitization.test.tscomment only, no assertion changed. Its header stated that on GET /packages "both of its data sources sit in their own inner try { … } catch {}, so nothing below reaches the outer catch". That sentence describes the exact line this PR removes and is now false; leaving it would tell the next author that a path this PR opened does not exist. The site is left driving the gate resolver deliberately, and the comment now says why and points at where the durable-read arm is pinned.

Clause-② path limb

NO. Measured with three-dot git diff --name-only $(git merge-base origin/main HEAD) HEAD against merge-base d806081dd — never two-dot origin/main, which would attribute sibling PRs that landed after this branch was cut. The change set is exactly:

.changeset/packages-list-durable-read-refusal.md
packages/rest/src/package-door-5xx-message-sanitization.test.ts
packages/rest/src/package-envelope.conformance.test.ts
packages/rest/src/package-list-durable-read-refusal.test.ts
packages/rest/src/package-routes.ts

Nothing under packages/spec/src/**. The declaration limb is also clear: no wire field, no response-shape change, no new error code — the fix lets an already-declared envelope through.

Changeset

.changeset/packages-list-durable-read-refusal.md@objectstack/rest: patch, with the reasoning written into the changeset body. Nothing an author can write changes (no spec key, export, config field, request or response shape added, removed or renamed), so it carries no migration and is not breaking; no capability is added, so it is not a feature. What changes is that one door stops reporting a failure as a successful complete answer. This is also the disposition the producer-side half of this same family shipped under.

Still in flight

CI is not waited on here. Local gates are green at 7e81738847 as quoted above; the CI farm runs the full set once regardless, and any gate that reds after this report returns as a patch round on this same claim.


Generated by Claude Code

…isting (#11063)
`GET /api/v1/packages` merged the in-memory registry with the durable
`sys_packages` rows and wrapped the durable half in a bare `catch {}`
commented "Database query failed — continue with registry-only packages".
A read that could not happen was reported as a read that found nothing:
the door answered 200 from the registry alone, `total` claimed a COMPLETE
count either way, and the registrar-sourced entries kept `source:
'registry'` — provenance, not a warning that the database half is absent.
The durable read is no longer caught at this door. `PackageService.list()`
still swallows its own driver faults and answers `[]`, re-throwing only the
declared seam refusal (`SERVICE_UNAVAILABLE` / 503), so that refusal now
reaches the client through the existing declared envelope carrying the
producer's own status and code. An undeclared throw becomes a 500
`INTERNAL_ERROR` through the same envelope. A durable read that answers is
unchanged.
This aligns the two read doors: `GET /api/v1/packages/:id` has no inner
catch and has answered that same refusal since the producer-side change.
No wire field is added and no response shape changes — the alternative the
card sketched (keep the 200 plus a declared partial-result marker) is a
contract decision and was not authorized.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 5 documentable anchor(s).

15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx(via packages.uninstall (sdk), /packages/:id (route))
  • content/docs/api/metadata-api.mdx(via /packages/:id (route))
  • content/docs/automation/approvals.mdx(via /packages/:id (route))
  • content/docs/data-modeling/formulas.mdx(via /packages/:id (route))
  • content/docs/deployment/publish-and-preview.mdx(via /packages/:id (route))
  • content/docs/deployment/self-hosting.mdx(via /packages/:id (route))
  • content/docs/kernel/contracts/metadata-service.mdx(via /packages/:id (route))
  • content/docs/permissions/access-recipes.mdx(via /packages/:id (route))
  • content/docs/permissions/authentication.mdx(via /packages/:id (route))
  • content/docs/permissions/permission-sets.mdx(via /packages/:id (route))
  • content/docs/permissions/record-view-auditing.mdx(via /packages/:id (route))
  • content/docs/plugins/adding-a-metadata-type.mdx(via /packages/:id (route))
  • content/docs/plugins/packages.mdx(via /packages/:id (route))
  • content/docs/ui/actions.mdx(via /packages/:id (route))
  • content/docs/ui/audience-based-interfaces.mdx(via /packages/:id (route))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx(via /packages/:id (route))
  • content/docs/releases/v17.mdx(via /packages/:id (route))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

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 95437e7d2d74ffe428e747c3a90545814e87b632packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6a576c802a677c9bdf723d2d76c67f6b20d892c6 — the merge of head 7e8173884797c49bd460a50f9fe3067e0420e1da into base 95437e7d2d74ffe428e747c3a90545814e87b632, 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 6a576c802a677c9bdf723d2d76c67f6b20d892c6 && git checkout 6a576c802a677c9bdf723d2d76c67f6b20d892c6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 95437e7d2d74ffe428e747c3a90545814e87b632 7e8173884797c49bd460a50f9fe3067e0420e1da && git checkout -B drift-repro 95437e7d2d74ffe428e747c3a90545814e87b632 && git merge --no-ff 7e8173884797c49bd460a50f9fe3067e0420e1da
node scripts/docs-audit/affected-docs.mjs --json 95437e7d2d74ffe428e747c3a90545814e87b632

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 95437e7d2d74ffe428e747c3a90545814e87b632 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 22, 2026
@os-elon
os-elon marked this pull request as ready for review August 22, 2026 20:28
@os-elon
os-elon added this pull request to the merge queueAug 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32596808471 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 5.92s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 98 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Merged via the queue into main with commit acb4dbcAug 22, 2026
32 checks passed
@os-elon
os-elon deleted the claude/issue-11063-packages-list-swallowed-read branch August 22, 2026 20:43
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 swallows a failed database read into a 200 registry-only answer — the caller cannot tell a partial listing from a complete one

2 participants

@os-elon@claude