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

.changeset/packages-list-durable-read-refusal.md

@@ -0,0 +1,13 @@
+---
+"@objectstack/rest": patch
+---
+
+`GET /api/v1/packages` no longer absorbs a failed durable read into a 200 registry-only listing.
+
+The handler merged two sources — the in-memory registry and the durable `sys_packages` rows read through `PackageService.list()` — 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 therefore reported as a read that found nothing: the door answered `200` with `{ packages, total }` built from the registry alone, `total` was presented as a COMPLETE count either way, and the registrar-sourced entries kept `source: 'registry'`, which reads as provenance rather than as 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 durable read is no longer caught at this door. `PackageService.list()` still swallows its own driver faults and answers `[]`, and re-throws only the declared seam refusal introduced alongside it (`SERVICE_UNAVAILABLE` / 503, raised when the storage seam accepted the query and returned no result set) — so that refusal now travels to 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: both sources still merge, `source` is still `registry` / `database` / `both`, and `total` is still the count of what was really read.
+
+This aligns the two read doors. `GET /api/v1/packages/:id` has no such inner catch and has answered that same refusal since the producer-side change; the list door answering `200` while the detail door refused was the inconsistency.
+
+**Bump level — why `patch` and not `minor` or `major`.** Nothing an author can write changes: no spec key, export, config field, request shape or response shape is added, removed or renamed, so this carries no migration and is not breaking. No capability is added either, so it is not a feature. What changes is that one door stops reporting a failure as a successful complete answer — a correctness fix to an existing contract, and the same disposition the producer-side half of this fix shipped under. Callers that treated a `200` from this door as "the complete package list" were already being told something untrue when the durable read failed; they now receive the declared refusal instead, exactly as they already did from the sibling detail route.

packages/rest/src/package-door-5xx-message-sanitization.test.ts

@@ -330,11 +330,25 @@ describe('[#8136] a real sys_metadata failure, walked in process through this do
 // route: all four handlers exit through the same `sendThrownError`, so each is
 // driven separately rather than assumed to share the fix.
 //
-// `GET /packages` is different BY DESIGN: both of its data sources sit in their
-// own inner `try { … } catch {}`, so nothing below reaches the outer catch.
-// What does is the gate — `refusePackageRequest` calls
-// `options.resolveExecutionContext(req)`, and a resolver that throws
-// SYNCHRONOUSLY throws before the `.catch(() => undefined)` is attached.
+// [#11063] `GET /packages` used to be different BY DESIGN — the sentence here
+// read: "both of its data sources sit in their own inner `try { … } catch {}`,
+// so nothing below reaches the outer catch". That is no longer true of the
+// DURABLE source: #11063 removed its inner catch, because absorbing a failed
+// durable read reported it as a 200 whose `total` claimed a complete count. A
+// throw from `packageService.list()` now reaches this same outer catch and this
+// same `sendThrownError`.
+//
+// This site is nevertheless left driving the GATE, deliberately: the resolver
+// throw is the one path that reaches the outer catch on this route regardless of
+// what either data source does, so it keeps proving the DOOR rather than one
+// source — `refusePackageRequest` calls `options.resolveExecutionContext(req)`,
+// and a resolver that throws SYNCHRONOUSLY throws before the
+// `.catch(() => undefined)` is attached. The list door's durable-read arm is
+// pinned separately in `package-list-durable-read-refusal.test.ts`.
+//
+// ⚠️ Still true of the REGISTRY source: `protocol.getMetaItems` keeps its own
+// inner catch, which #11063 deliberately did not touch (different producer, no
+// declared refusal to carry, and widening the change there was not authorized).
 
 interface Site {
  name: string;

packages/rest/src/package-envelope.conformance.test.ts

@@ -331,9 +331,12 @@ describe('packages envelope (#3843) — error bodies', () => {
  ),
  },
  {
- // NOT `GET /packages`: that route catches a failing `list()` in an INNER
- // try and degrades to the registry-only listing, so its 500 arm is
- // unreachable that way (pinned below). `GET /:id` has no inner catch.
+ // [#11063] Was: "NOT `GET /packages` — that route catches a failing
+ // `list()` in an INNER try and degrades to the registry-only listing, so
+ // its 500 arm is unreachable that way." That inner catch is gone; both
+ // read doors now reach this arm. `GET /:id` is kept as this case's
+ // subject so the case itself is unchanged, and the list door's own 500
+ // arm is pinned in `package-list-durable-read-refusal.test.ts`.
  name: 'an unexpected throw from the package service',
  status: 500,
  code: 'INTERNAL_ERROR',
@@ -375,20 +378,35 @@ describe('packages envelope (#3843) — error bodies', () => {
  }
  });
 
- it('GET /packages still degrades to a 200 registry-only listing when the database is down', async () => {
- // Pre-existing, deliberate (`// Database query failed — continue with
- // registry-only packages`) and unchanged by #3843 — recorded here because it
- // is why the 500 case above drives `GET /:id` instead.
+ it('GET /packages no longer degrades to a 200 registry-only listing when the durable read fails (#11063)', async () => {
+ // REPLACED, not re-spelled. This pin used to record the opposite — a 200
+ // carrying the registry half alone — described as "pre-existing, deliberate
+ // (`// Database query failed — continue with registry-only packages`)". It
+ // pinned exactly the branch #11063 removed, so re-spelling it would have
+ // left an assertion that passes only because nothing is produced any more.
+ //
+ // ⚠️ Note what this fixture models: a BARE `Error`. Since #10965 the real
+ // `PackageService.list()` swallows its own driver faults and still answers
+ // `[]`, and re-throws only the declared `SERVICE_UNAVAILABLE` / 503 seam
+ // refusal — so this shape is the UNDECLARED arm (a 500 server fault), and
+ // the declared-refusal arm is pinned in
+ // `package-list-durable-read-refusal.test.ts` alongside the `total` and
+ // both-doors-agree assertions.
  const { status, body } = await drive(
  mount({ list: async () => { throw new Error('db down'); } }, {
  protocol: { getMetaItems: async () => ({ items: [{ manifest: MANIFEST }] }) },
  }),
  'GET',
  PKGS,
  );
- expect(status).toBe(200);
- expect(body.success).toBe(true);
- expect(body.data.packages).toHaveLength(1);
+ expect(status).toBe(500);
+ expect(body.success).toBe(false);
+ expect(body.error.code).toBe('INTERNAL_ERROR');
+ // The registry half is not served as if it were a complete listing, and no
+ // `total` is reported over a read that failed.
+ expect(body.data?.packages).toBeUndefined();
+ expect(body.data?.total).toBeUndefined();
+ expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]);
  });
 
  it('a repeated `?version=` is refused identically on both verbs (#6307)', async () => {

packages/rest/src/package-list-durable-read-refusal.test.ts

@@ -0,0 +1,245 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #11063 — `GET /api/v1/packages` must not absorb a failed durable read.
+ *
+ * ## What was wrong, and why "still 200" was not a pin
+ *
+ * The list door merged two sources — the in-memory registry (via
+ * `protocol.getMetaItems`) and the durable `sys_packages` rows (via
+ * `PackageService.list()`) — 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 200 whose
+ * `total` claimed to be a COMPLETE count, and whose registrar-sourced entries
+ * kept `source: 'registry'` — which reads as PROVENANCE, not as 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"*.
+ *
+ * This is the standing family ruling — #10965 · #10677 / PR #10788 · #10789 /
+ * PR #10964: **a read that could not happen must not be reported as a read that
+ * found nothing.** Here it sat one level up, in a consumer-side catch rather
+ * than in a flattener, which is why the producer-side fix could not close it.
+ *
+ * ⚠️ Asserting "the listing returns 200" passes on the OLD code, on the fixed
+ * code, and on a wrong fix — it is the empty assertion this file exists to
+ * avoid. Every case below pins the MECHANISM instead: which status and which
+ * declared `code` reach the client when the durable read refuses, that `total`
+ * is not reported at all over a read that failed, and that the two read doors
+ * answer the same failure identically.
+ *
+ * ## Where the halves are pinned
+ *
+ * The PRODUCER half — that `PackageService.list()`/`get()` refuse with
+ * `SERVICE_UNAVAILABLE` / 503 over a seam that accepted the query and returned
+ * no result set — is measured on a real booted engine in
+ * `packages/runtime/src/package-service.null-seam.test.ts` (#10965). This file
+ * pins the DOOR half: that the declared refusal travels through the REST
+ * envelope instead of being swallowed. The refusal is reproduced locally rather
+ * than imported so this suite stays free of a cross-package VALUE import (and
+ * of the build-state dependence one would carry — `@objectstack/service-package`
+ * is not aliased to `src/` in this package's vitest config); the shape it
+ * reproduces is `packageSeamUnreadableError()` in
+ * `packages/services/service-package/src/index.ts`.
+ *
+ * ⛔ No wire field is added by the fix and none is asserted here. The card's
+ * alternative — keep the 200 and carry a declared partial-result marker — is a
+ * response-shape change, i.e. a contract decision, and was not authorized.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
+import type { RouteHandler } from '@objectstack/spec/contracts';
+import { registerPackageRoutes } from './package-routes.js';
+
+const PKGS = '/api/v1/packages';
+
+interface Captured {
+ status: number;
+ body: any;
+}
+
+/** Only the methods these two read doors reach. */
+type Svc = Partial<{
+ list: () => Promise<any[]>;
+ get: (id: string, version?: string) => Promise<any>;
+}>;
+
+/**
+ * The #10965 refusal, reproduced: an ADR-0112 envelope ON THE ERROR — a
+ * declared `status` AND a declared `code` — which is what lets it leave through
+ * the door's shared `resolveThrownHttpError` mapping as the PRODUCER's answer
+ * rather than as a 500 catch-all.
+ */
+function seamUnreadableError(): Error {
+ return Object.assign(
+ new Error(
+ 'The package registry could not be read: the storage seam accepted the query but returned no '
+ + 'result set. Whether this package is installed is UNKNOWN — this is not an answer of "no".',
+ ),
+ { code: 'SERVICE_UNAVAILABLE', status: 503 },
+ );
+}
+
+function mount(svc: Svc, options: any = {}) {
+ const routes = new Map<string, RouteHandler>();
+ const server = {
+ get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); },
+ post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); },
+ put: () => {},
+ delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); },
+ patch: () => {},
+ use: () => {},
+ listen: async () => {},
+ close: async () => {},
+ } as any;
+ // The authorization gate (#7033 / #7023) is not this file's subject, so the
+ // caller is stubbed holding the ADR-0106 D4 read set.
+ registerPackageRoutes(server, () => svc as any, '/api/v1', {
+ resolveExecutionContext: async () => ({
+ userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'],
+ }),
+ ...options,
+ });
+ return routes;
+}
+
+async function drive(
+ routes: Map<string, RouteHandler>,
+ method: string,
+ path: string,
+ req: Record<string, any> = {},
+): Promise<Captured> {
+ const handler = routes.get(`${method}:${path}`);
+ if (!handler) throw new Error(`no handler for ${method} ${path}`);
+ const captured: Captured = { status: 200, body: undefined };
+ const res: any = {
+ json(data: any) { captured.body = data; },
+ send() {},
+ status(code: number) { captured.status = code; return res; },
+ header() { return res; },
+ };
+ await handler(
+ { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any,
+ res,
+ );
+ return captured;
+}
+
+const REGISTRY_MANIFEST = { id: 'com.acme.registry-only', version: '1.0.0' };
+
+/** A registry half that DOES answer — so a swallowed durable failure would have
+ * something to answer 200 with, exactly as the defect did. */
+const REGISTRY_PROTOCOL = {
+ protocol: { getMetaItems: async () => ({ items: [{ manifest: REGISTRY_MANIFEST }] }) },
+};
+
+describe('#11063 GET /packages — a failed durable read reaches the client', () => {
+ it('answers the producer’s declared refusal (503 SERVICE_UNAVAILABLE), not a 200', async () => {
+ const { status, body } = await drive(
+ mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL),
+ 'GET',
+ PKGS,
+ );
+
+ // code AND status — the ADR-0112 envelope, never a bare `toThrow()` and
+ // never a status on its own.
+ expect(status).toBe(503);
+ expect(body.success).toBe(false);
+ expect(body.error.code).toBe('SERVICE_UNAVAILABLE');
+
+ // …carried in the DECLARED envelope, not an ad-hoc body.
+ expect(BaseResponseSchema.safeParse(body).success, JSON.stringify(body)).toBe(true);
+ expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]);
+ expect(typeof body.error.message).toBe('string');
+ expect(body.error.message.length).toBeGreaterThan(0);
+ });
+
+ it('reports NO `total` over a read that failed — the corrupted complete count is gone', async () => {
+ const { status, body } = await drive(
+ mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL),
+ 'GET',
+ PKGS,
+ );
+
+ // The defect's signature: a `total` presented as a complete count while the
+ // durable half was missing, and a `packages` array the caller could not
+ // tell apart from a full listing.
+ expect(status).not.toBe(200);
+ expect(body.data?.total).toBeUndefined();
+ expect(body.data?.packages).toBeUndefined();
+
+ // And specifically NOT the registry-only listing served as if it were whole.
+ expect(body.data?.packages).not.toEqual([
+ expect.objectContaining({ source: 'registry' }),
+ ]);
+ });
+
+ it('answers the SAME failure identically on both read doors (#11063 alignment)', async () => {
+ // `GET /packages/:id` has never had an inner catch, so it has answered this
+ // refusal since #10965. The list door disagreeing with it WAS the defect;
+ // agreement is the fix, and it is worth one assertion.
+ const list = await drive(
+ mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL),
+ 'GET',
+ PKGS,
+ );
+ const detail = await drive(
+ mount({ get: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL),
+ 'GET',
+ `${PKGS}/:id`,
+ { params: { id: 'com.acme.crm' } },
+ );
+
+ expect(list.status).toBe(detail.status);
+ expect(list.body.error.code).toBe(detail.body.error.code);
+ expect(list.body.success).toBe(detail.body.success);
+ });
+
+ it('an UNDECLARED throw from the durable read is a 500 INTERNAL_ERROR, not a 200', async () => {
+ // The other half of "stop absorbing": a throw carrying no declared envelope
+ // is a server fault and now reaches the outer catch. Before the fix this
+ // arm was unreachable on this route — which is why the sibling envelope
+ // suite had to drive `GET /:id` to exercise it at all.
+ const { status, body } = await drive(
+ mount({ list: async () => { throw new Error('db down'); } }, REGISTRY_PROTOCOL),
+ 'GET',
+ PKGS,
+ );
+
+ expect(status).toBe(500);
+ expect(body.success).toBe(false);
+ expect(body.error.code).toBe('INTERNAL_ERROR');
+ expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]);
+ });
+
+ it('a durable read that ANSWERS still merges both sources and counts them truthfully', async () => {
+ // The half that keeps this from being "refuse always": nothing about the
+ // healthy path moved. Two sources, one overlapping id, and a `total` that
+ // is a real complete count of what was really read.
+ const { status, body } = await drive(
+ mount(
+ {
+ list: async () => [
+ { id: 'com.acme.registry-only', version: '1.0.0', manifest: REGISTRY_MANIFEST },
+ { id: 'com.acme.published', version: '2.0.0', manifest: { id: 'com.acme.published' } },
+ ],
+ },
+ REGISTRY_PROTOCOL,
+ ),
+ 'GET',
+ PKGS,
+ );
+
+ expect(status).toBe(200);
+ expect(body.success).toBe(true);
+ expect(body.data.total).toBe(2);
+ expect(body.data.packages).toHaveLength(2);
+
+ const bySource = Object.fromEntries(
+ body.data.packages.map((p: any) => [p.manifest?.id ?? p.id, p.source]),
+ );
+ // The id both halves carry is `both`; the durable-only id is `database`.
+ expect(bySource['com.acme.registry-only']).toBe('both');
+ expect(bySource['com.acme.published']).toBe('database');
+ });
+});