|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #11063 — `GET /api/v1/packages` must not absorb a failed durable read. |
| 5 | + * |
| 6 | + * ## What was wrong, and why "still 200" was not a pin |
| 7 | + * |
| 8 | + * The list door merged two sources — the in-memory registry (via |
| 9 | + * `protocol.getMetaItems`) and the durable `sys_packages` rows (via |
| 10 | + * `PackageService.list()`) — and wrapped the durable half in a bare |
| 11 | + * `catch {}` commented *"Database query failed — continue with registry-only |
| 12 | + * packages"*. A failed durable read was therefore reported as a 200 whose |
| 13 | + * `total` claimed to be a COMPLETE count, and whose registrar-sourced entries |
| 14 | + * kept `source: 'registry'` — which reads as PROVENANCE, not as a warning that |
| 15 | + * the database half is absent. Nothing on the wire separated *"these are all |
| 16 | + * the packages"* from *"these are the packages I could still see"*. |
| 17 | + * |
| 18 | + * This is the standing family ruling — #10965 · #10677 / PR #10788 · #10789 / |
| 19 | + * PR #10964: **a read that could not happen must not be reported as a read that |
| 20 | + * found nothing.** Here it sat one level up, in a consumer-side catch rather |
| 21 | + * than in a flattener, which is why the producer-side fix could not close it. |
| 22 | + * |
| 23 | + * ⚠️ Asserting "the listing returns 200" passes on the OLD code, on the fixed |
| 24 | + * code, and on a wrong fix — it is the empty assertion this file exists to |
| 25 | + * avoid. Every case below pins the MECHANISM instead: which status and which |
| 26 | + * declared `code` reach the client when the durable read refuses, that `total` |
| 27 | + * is not reported at all over a read that failed, and that the two read doors |
| 28 | + * answer the same failure identically. |
| 29 | + * |
| 30 | + * ## Where the halves are pinned |
| 31 | + * |
| 32 | + * The PRODUCER half — that `PackageService.list()`/`get()` refuse with |
| 33 | + * `SERVICE_UNAVAILABLE` / 503 over a seam that accepted the query and returned |
| 34 | + * no result set — is measured on a real booted engine in |
| 35 | + * `packages/runtime/src/package-service.null-seam.test.ts` (#10965). This file |
| 36 | + * pins the DOOR half: that the declared refusal travels through the REST |
| 37 | + * envelope instead of being swallowed. The refusal is reproduced locally rather |
| 38 | + * than imported so this suite stays free of a cross-package VALUE import (and |
| 39 | + * of the build-state dependence one would carry — `@objectstack/service-package` |
| 40 | + * is not aliased to `src/` in this package's vitest config); the shape it |
| 41 | + * reproduces is `packageSeamUnreadableError()` in |
| 42 | + * `packages/services/service-package/src/index.ts`. |
| 43 | + * |
| 44 | + * ⛔ No wire field is added by the fix and none is asserted here. The card's |
| 45 | + * alternative — keep the 200 and carry a declared partial-result marker — is a |
| 46 | + * response-shape change, i.e. a contract decision, and was not authorized. |
| 47 | + */ |
| 48 | + |
| 49 | +import { describe, it, expect } from 'vitest'; |
| 50 | +import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; |
| 51 | +import type { RouteHandler } from '@objectstack/spec/contracts'; |
| 52 | +import { registerPackageRoutes } from './package-routes.js'; |
| 53 | + |
| 54 | +const PKGS = '/api/v1/packages'; |
| 55 | + |
| 56 | +interface Captured { |
| 57 | + status: number; |
| 58 | + body: any; |
| 59 | +} |
| 60 | + |
| 61 | +/** Only the methods these two read doors reach. */ |
| 62 | +type Svc = Partial<{ |
| 63 | + list: () => Promise<any[]>; |
| 64 | + get: (id: string, version?: string) => Promise<any>; |
| 65 | +}>; |
| 66 | + |
| 67 | +/** |
| 68 | + * The #10965 refusal, reproduced: an ADR-0112 envelope ON THE ERROR — a |
| 69 | + * declared `status` AND a declared `code` — which is what lets it leave through |
| 70 | + * the door's shared `resolveThrownHttpError` mapping as the PRODUCER's answer |
| 71 | + * rather than as a 500 catch-all. |
| 72 | + */ |
| 73 | +function seamUnreadableError(): Error { |
| 74 | + return Object.assign( |
| 75 | + new Error( |
| 76 | + 'The package registry could not be read: the storage seam accepted the query but returned no ' |
| 77 | + + 'result set. Whether this package is installed is UNKNOWN — this is not an answer of "no".', |
| 78 | + ), |
| 79 | + { code: 'SERVICE_UNAVAILABLE', status: 503 }, |
| 80 | + ); |
| 81 | +} |
| 82 | + |
| 83 | +function mount(svc: Svc, options: any = {}) { |
| 84 | + const routes = new Map<string, RouteHandler>(); |
| 85 | + const server = { |
| 86 | + get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, |
| 87 | + post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, |
| 88 | + put: () => {}, |
| 89 | + delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, |
| 90 | + patch: () => {}, |
| 91 | + use: () => {}, |
| 92 | + listen: async () => {}, |
| 93 | + close: async () => {}, |
| 94 | + } as any; |
| 95 | + // The authorization gate (#7033 / #7023) is not this file's subject, so the |
| 96 | + // caller is stubbed holding the ADR-0106 D4 read set. |
| 97 | + registerPackageRoutes(server, () => svc as any, '/api/v1', { |
| 98 | + resolveExecutionContext: async () => ({ |
| 99 | + userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], |
| 100 | + }), |
| 101 | + ...options, |
| 102 | + }); |
| 103 | + return routes; |
| 104 | +} |
| 105 | + |
| 106 | +async function drive( |
| 107 | + routes: Map<string, RouteHandler>, |
| 108 | + method: string, |
| 109 | + path: string, |
| 110 | + req: Record<string, any> = {}, |
| 111 | +): Promise<Captured> { |
| 112 | + const handler = routes.get(`${method}:${path}`); |
| 113 | + if (!handler) throw new Error(`no handler for ${method} ${path}`); |
| 114 | + const captured: Captured = { status: 200, body: undefined }; |
| 115 | + const res: any = { |
| 116 | + json(data: any) { captured.body = data; }, |
| 117 | + send() {}, |
| 118 | + status(code: number) { captured.status = code; return res; }, |
| 119 | + header() { return res; }, |
| 120 | + }; |
| 121 | + await handler( |
| 122 | + { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, |
| 123 | + res, |
| 124 | + ); |
| 125 | + return captured; |
| 126 | +} |
| 127 | + |
| 128 | +const REGISTRY_MANIFEST = { id: 'com.acme.registry-only', version: '1.0.0' }; |
| 129 | + |
| 130 | +/** A registry half that DOES answer — so a swallowed durable failure would have |
| 131 | + * something to answer 200 with, exactly as the defect did. */ |
| 132 | +const REGISTRY_PROTOCOL = { |
| 133 | + protocol: { getMetaItems: async () => ({ items: [{ manifest: REGISTRY_MANIFEST }] }) }, |
| 134 | +}; |
| 135 | + |
| 136 | +describe('#11063 GET /packages — a failed durable read reaches the client', () => { |
| 137 | + it('answers the producer’s declared refusal (503 SERVICE_UNAVAILABLE), not a 200', async () => { |
| 138 | + const { status, body } = await drive( |
| 139 | + mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), |
| 140 | + 'GET', |
| 141 | + PKGS, |
| 142 | + ); |
| 143 | + |
| 144 | + // code AND status — the ADR-0112 envelope, never a bare `toThrow()` and |
| 145 | + // never a status on its own. |
| 146 | + expect(status).toBe(503); |
| 147 | + expect(body.success).toBe(false); |
| 148 | + expect(body.error.code).toBe('SERVICE_UNAVAILABLE'); |
| 149 | + |
| 150 | + // …carried in the DECLARED envelope, not an ad-hoc body. |
| 151 | + expect(BaseResponseSchema.safeParse(body).success, JSON.stringify(body)).toBe(true); |
| 152 | + expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); |
| 153 | + expect(typeof body.error.message).toBe('string'); |
| 154 | + expect(body.error.message.length).toBeGreaterThan(0); |
| 155 | + }); |
| 156 | + |
| 157 | + it('reports NO `total` over a read that failed — the corrupted complete count is gone', async () => { |
| 158 | + const { status, body } = await drive( |
| 159 | + mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), |
| 160 | + 'GET', |
| 161 | + PKGS, |
| 162 | + ); |
| 163 | + |
| 164 | + // The defect's signature: a `total` presented as a complete count while the |
| 165 | + // durable half was missing, and a `packages` array the caller could not |
| 166 | + // tell apart from a full listing. |
| 167 | + expect(status).not.toBe(200); |
| 168 | + expect(body.data?.total).toBeUndefined(); |
| 169 | + expect(body.data?.packages).toBeUndefined(); |
| 170 | + |
| 171 | + // And specifically NOT the registry-only listing served as if it were whole. |
| 172 | + expect(body.data?.packages).not.toEqual([ |
| 173 | + expect.objectContaining({ source: 'registry' }), |
| 174 | + ]); |
| 175 | + }); |
| 176 | + |
| 177 | + it('answers the SAME failure identically on both read doors (#11063 alignment)', async () => { |
| 178 | + // `GET /packages/:id` has never had an inner catch, so it has answered this |
| 179 | + // refusal since #10965. The list door disagreeing with it WAS the defect; |
| 180 | + // agreement is the fix, and it is worth one assertion. |
| 181 | + const list = await drive( |
| 182 | + mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), |
| 183 | + 'GET', |
| 184 | + PKGS, |
| 185 | + ); |
| 186 | + const detail = await drive( |
| 187 | + mount({ get: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), |
| 188 | + 'GET', |
| 189 | + `${PKGS}/:id`, |
| 190 | + { params: { id: 'com.acme.crm' } }, |
| 191 | + ); |
| 192 | + |
| 193 | + expect(list.status).toBe(detail.status); |
| 194 | + expect(list.body.error.code).toBe(detail.body.error.code); |
| 195 | + expect(list.body.success).toBe(detail.body.success); |
| 196 | + }); |
| 197 | + |
| 198 | + it('an UNDECLARED throw from the durable read is a 500 INTERNAL_ERROR, not a 200', async () => { |
| 199 | + // The other half of "stop absorbing": a throw carrying no declared envelope |
| 200 | + // is a server fault and now reaches the outer catch. Before the fix this |
| 201 | + // arm was unreachable on this route — which is why the sibling envelope |
| 202 | + // suite had to drive `GET /:id` to exercise it at all. |
| 203 | + const { status, body } = await drive( |
| 204 | + mount({ list: async () => { throw new Error('db down'); } }, REGISTRY_PROTOCOL), |
| 205 | + 'GET', |
| 206 | + PKGS, |
| 207 | + ); |
| 208 | + |
| 209 | + expect(status).toBe(500); |
| 210 | + expect(body.success).toBe(false); |
| 211 | + expect(body.error.code).toBe('INTERNAL_ERROR'); |
| 212 | + expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); |
| 213 | + }); |
| 214 | + |
| 215 | + it('a durable read that ANSWERS still merges both sources and counts them truthfully', async () => { |
| 216 | + // The half that keeps this from being "refuse always": nothing about the |
| 217 | + // healthy path moved. Two sources, one overlapping id, and a `total` that |
| 218 | + // is a real complete count of what was really read. |
| 219 | + const { status, body } = await drive( |
| 220 | + mount( |
| 221 | + { |
| 222 | + list: async () => [ |
| 223 | + { id: 'com.acme.registry-only', version: '1.0.0', manifest: REGISTRY_MANIFEST }, |
| 224 | + { id: 'com.acme.published', version: '2.0.0', manifest: { id: 'com.acme.published' } }, |
| 225 | + ], |
| 226 | + }, |
| 227 | + REGISTRY_PROTOCOL, |
| 228 | + ), |
| 229 | + 'GET', |
| 230 | + PKGS, |
| 231 | + ); |
| 232 | + |
| 233 | + expect(status).toBe(200); |
| 234 | + expect(body.success).toBe(true); |
| 235 | + expect(body.data.total).toBe(2); |
| 236 | + expect(body.data.packages).toHaveLength(2); |
| 237 | + |
| 238 | + const bySource = Object.fromEntries( |
| 239 | + body.data.packages.map((p: any) => [p.manifest?.id ?? p.id, p.source]), |
| 240 | + ); |
| 241 | + // The id both halves carry is `both`; the durable-only id is `database`. |
| 242 | + expect(bySource['com.acme.registry-only']).toBe('both'); |
| 243 | + expect(bySource['com.acme.published']).toBe('database'); |
| 244 | + }); |
| 245 | +}); |
0 commit comments