diff --git a/.changeset/route-audit-tranche-3-service-mounts.md b/.changeset/route-audit-tranche-3-service-mounts.md new file mode 100644 index 0000000000..19b8c612a3 --- /dev/null +++ b/.changeset/route-audit-tranche-3-service-mounts.md @@ -0,0 +1,51 @@ +--- +"@objectstack/service-storage": patch +"@objectstack/service-i18n": patch +"@objectstack/client": patch +--- + +fix(client,service-i18n): ledger the autonomously-mounted service routes, and repair the two i18n calls that reached nothing (#3636) + +Tranche 3 of the #3563 route audit — the last un-audited server surface. The +dispatcher ledger (#3563) and the REST ledger (#3587) each stop at their own +package boundary, and two services mount routes outside both: they reach for +the `http-server` service and register straight on `IHttpServer`, so neither +`RouteManager` nor `RestServer.getRoutes()` has ever seen them. That left the +SDK's entire storage surface, plus all of i18n, in the pre-#3563 posture: +expressed, working, guarded by nothing. + +**Ledgers + guards.** `storage-route-ledger.ts` (10 routes) and +`i18n-route-ledger.ts` (3) sit next to the registrars that mount them, each +enumerated for real — the registrar runs against a capturing mock +`IHttpServer` and its registration calls *are* the route set, so a new route +lands with a reviewed disposition or fails CI. The client half is +`packages/client/src/service-route-ledger-coverage.test.ts`; ledgers cross the +boundary as relative source imports, never a service→client package edge. + +**Two wire-level 404s fixed.** `i18n.getTranslations` sent +`/i18n/translations?locale=xx` and `i18n.getFieldLabels` sent +`/i18n/labels/:object?locale=xx`, while every serving surface — service-i18n's +mounts, the dispatcher's HTTP mounts, and the `plugin-rest-api.zod.ts` +contract — mounts only the path form. Neither call could ever be answered. +Both had carried a green `sdk` row in the dispatcher ledger since tranche 1, +because that guard asks whether the client *method* exists, not whether it +speaks a URL anything mounts. The client now sends the path dialect, the same +resolution #3611 gave `meta.getView`, and a new suite drives the real client +at a real router so a revert cannot pass quietly. + +**One response-shape fix.** service-i18n's success bodies omitted the +`success` flag that `ObjectStackClient.unwrapResponse` keys on, so the SDK +returned the raw `{ data: … }` wrapper against that provider while returning +the declared unwrapped shape against the dispatcher — one method, two shapes, +decided by which plugin mounted the route. Its three handlers now emit the +`{ success: true, data }` envelope the `i18n` route group declares. `data` did +not move, so direct body readers are unaffected. + +Storage audited clean: 7 routes SDK-expressed, 3 reviewed `server-only` (the +browser capability URL objectql stamps into file-field payloads, and the two +local-driver loopbacks). The chunked-upload family, flagged for triage, turned +out fully expressed. Both ledgers ratchet `gap` and `mismatch` at zero. + +Filed, not fixed: `GET {base}/_local/file/:key` is built by three call sites +and mounted by none (#3641); the cross-surface URL conformance guard that would +have caught all of the above mechanically is the capstone (#3642). diff --git a/docs/audits/2026-07-dispatcher-client-route-coverage.md b/docs/audits/2026-07-dispatcher-client-route-coverage.md index 4921a8f357..87cd537c4c 100644 --- a/docs/audits/2026-07-dispatcher-client-route-coverage.md +++ b/docs/audits/2026-07-dispatcher-client-route-coverage.md @@ -157,6 +157,49 @@ expression exists for: `GET /search`, `POST /email/send`, `forms/:slug` `external-datasource-routes.ts`). Auditing that surface with the same three-column method is the next tranche of #3563. +**Closed in #3587** — `packages/rest/src/rest-route-ledger.ts` plus its +conformance guard; the 43 audited gaps and 2 mismatches both ratchet at zero. + +## 9. The third surface: autonomous service mounts (#3636) + +Neither ledger sees a service that reaches for the `http-server` service and +registers straight on `IHttpServer` — it bypasses `RouteManager` and +`RestServer.getRoutes()` alike. Two do: `service-storage` +(`storage-routes.ts`, 10 routes — the SDK's entire storage surface) and +`service-i18n` (`i18n-service-plugin.ts`, 3 routes). Both now carry a +per-package ledger next to the registrar, enumerated by driving the real +registrar against a capturing mock `IHttpServer`, with the client half in +`packages/client/src/service-route-ledger-coverage.test.ts` (no +service→client package edge — the tranche-1 lesson). + +Dispositions: storage audited clean at 7 `sdk` / 3 `server-only` (the +browser-facing `/files/:fileId` redirect objectql stamps into file-field +payloads, and the two `_local/raw/:token` local-driver loopbacks). The chunked +upload family — flagged in #3636 as needing triage — turned out fully +SDK-expressed (`initChunkedUpload` / `uploadPart` / `completeChunkedUpload` / +`resumeUpload`), so no gap to close. + +i18n audited at **two mismatches**, both fixed in the same PR: +`i18n.getTranslations` sent `/translations?locale=xx` and +`i18n.getFieldLabels` sent `/labels/:object?locale=xx`, while every serving +surface — service-i18n's mounts, the dispatcher's HTTP mounts, and the +`plugin-rest-api.zod.ts` contract — mounts only the path form. Both were +wire-level 404s, and both had carried a green `sdk` row in +`route-ledger.ts` since tranche 1: **§1 coverage rows assert the client method +exists, not that it speaks a URL anything mounts.** The same audit found +service-i18n omitting the `success` flag from its `{ data }` bodies, so +`unwrapResponse` returned the raw wrapper against that provider while +returning the declared shape against the dispatcher — one method, two shapes, +decided by which plugin mounted the route. + +Also filed, not fixed: `GET {base}/_local/file/:key` is built by three call +sites and mounted by none (#3641). + +**The gap all three ledgers still share** is the reverse direction — no guard +compares the URL a client method *builds* against the patterns any surface +*mounts*. Four instances of that class have now been found one at a time +(#3584 ×2, #3611, #3636 ×2). Mechanizing it is the capstone, #3642. + ## Follow-up slicing (proposed) 1. **`client.actions.invoke(...)`** — closes the largest hole (3 routes). @@ -166,7 +209,9 @@ three-column method is the next tranche of #3563. 5. **Mismatch reconciliation** (§4) — done in #3584: analytics client aligned to the dispatcher, storage protocol documented as canonical (see §4 Resolution). 6. **Docs**: delete or regenerate README surface table + CLIENT_SPEC_COMPLIANCE.md; extend client-sdk.mdx. 7. **Deprecate `DEFAULT_DISPATCHER_ROUTES`**; point at the ledger. -8. **REST-surface tranche** (§8) with the same ledger+guard treatment. +8. **REST-surface tranche** (§8) with the same ledger+guard treatment — done in #3587. +9. **Autonomous service mounts** (§9) — done in #3636. +10. **Cross-surface URL conformance** (§9, the reverse direction) — #3642. Each gap closed must flip its ledger row to `sdk` and lower the ratchet bound in the conformance test — the guard enforces both directions from PR-1 onward. diff --git a/packages/client/src/client.test.ts b/packages/client/src/client.test.ts index 24ddeaeeb2..e5de385cbb 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -664,29 +664,42 @@ describe('i18n namespace', () => { expect(url).toContain('/api/v1/i18n/locales'); }); - it('should get translations', async () => { + it('i18n.getTranslations speaks the path-param dialect every surface mounts (#3636)', async () => { const { client, fetchMock } = createMockClient({ success: true, data: { locale: 'zh-CN', translations: { hello: '你好' } } }); - const result = await client.i18n.getTranslations('zh-CN', { namespace: 'common' }); + const result = await client.i18n.getTranslations('zh-CN'); expect(result.locale).toBe('zh-CN'); - const url = fetchMock.mock.calls[0][0] as string; - expect(url).toContain('/api/v1/i18n/translations'); - expect(url).toContain('locale=zh-CN'); - expect(url).toContain('namespace=common'); + // NOT `/translations?locale=zh-CN` — no server mounts a bare + // /translations, so the old query dialect 404'd everywhere. + expect(String(fetchMock.mock.calls[0][0])).toBe( + 'http://localhost:3000/api/v1/i18n/translations/zh-CN', + ); + }); + + it('i18n.getTranslations keeps namespace/keys as query params on the path form', async () => { + const { client, fetchMock } = createMockClient({ + success: true, + data: { locale: 'zh-CN', translations: { hello: '你好' } } + }); + await client.i18n.getTranslations('zh-CN', { namespace: 'common', keys: ['a', 'b'] }); + expect(String(fetchMock.mock.calls[0][0])).toBe( + 'http://localhost:3000/api/v1/i18n/translations/zh-CN?namespace=common&keys=a%2Cb', + ); }); - it('should get field labels', async () => { + it('i18n.getFieldLabels puts both object and locale on the path (#3636)', async () => { const { client, fetchMock } = createMockClient({ success: true, data: { object: 'customer', labels: { name: '名前' } } }); const result = await client.i18n.getFieldLabels('customer', 'ja'); expect(result.object).toBe('customer'); - const url = fetchMock.mock.calls[0][0] as string; - expect(url).toContain('/api/v1/i18n/labels/customer'); - expect(url).toContain('locale=ja'); + // NOT `/labels/customer?locale=ja` — the mount is /labels/:object/:locale. + expect(String(fetchMock.mock.calls[0][0])).toBe( + 'http://localhost:3000/api/v1/i18n/labels/customer/ja', + ); }); }); diff --git a/packages/client/src/i18n-wire-dialect.test.ts b/packages/client/src/i18n-wire-dialect.test.ts new file mode 100644 index 0000000000..97e70f84e4 --- /dev/null +++ b/packages/client/src/i18n-wire-dialect.test.ts @@ -0,0 +1,91 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * i18n wire-dialect proof (#3636) — the SDK's i18n calls resolve against a + * REAL router, not a pinned URL string. + * + * The URL pins in `client.test.ts` say what the client sends. They cannot say + * whether anything answers: that is exactly how `i18n.getTranslations` and + * `i18n.getFieldLabels` shipped a `?locale=` dialect no server has ever + * mounted, passing every client-side test while 404-ing in production. + * + * So this suite mounts the two route patterns every serving surface declares — + * `service-i18n`'s autonomous mounts, the dispatcher's HTTP mounts, and the + * `plugin-rest-api.zod.ts` contract all agree on them — on a real Hono server, + * and drives the real client at it. The old dialect is asserted DEAD in the + * same suite, so a revert cannot pass quietly. + * + * service-i18n itself is deliberately not imported: it is not a dependency of + * `@objectstack/client` and must not become one (the tranche-1 no-package-edge + * lesson). The route PATTERNS are the contract under test, and they are + * identical on both surfaces. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; +import type { IHttpServer, IHttpRequest, IHttpResponse } from '@objectstack/spec/contracts'; +import { ObjectStackClient } from './index'; + +describe('i18n SDK calls hit a real router (#3636)', () => { + let baseUrl: string; + let kernel: LiteKernel; + let client: ObjectStackClient; + + beforeAll(async () => { + kernel = new LiteKernel(); + const hono = new HonoServerPlugin({ port: 0 }); + kernel.use(hono); + await kernel.bootstrap(); + + // The same three patterns I18nServicePlugin.registerI18nRoutes mounts, + // answering with the same `{ success: true, data }` envelope it now emits + // — see packages/services/service-i18n/src/i18n-route-ledger.ts. + const server = kernel.getService('http-server'); + server.get('/api/v1/i18n/locales', async (_req: IHttpRequest, res: IHttpResponse) => { + res.json({ success: true, data: { locales: [{ code: 'zh-CN', label: 'zh-CN', isDefault: true }] } }); + }); + server.get('/api/v1/i18n/translations/:locale', async (req: IHttpRequest, res: IHttpResponse) => { + res.json({ success: true, data: { locale: req.params.locale, translations: { hello: '你好' } } }); + }); + server.get('/api/v1/i18n/labels/:object/:locale', async (req: IHttpRequest, res: IHttpResponse) => { + res.json({ success: true, data: { object: req.params.object, locale: req.params.locale, labels: { name: '名称' } } }); + }); + + baseUrl = `http://localhost:${(server as unknown as { getPort(): number }).getPort()}`; + client = new ObjectStackClient({ baseUrl }); + }); + + afterAll(async () => { + await kernel?.shutdown(); + }); + + it('getTranslations resolves — the locale arrives as a path param', async () => { + const res = await client.i18n.getTranslations('zh-CN'); + expect(res.locale).toBe('zh-CN'); + expect(res.translations).toEqual({ hello: '你好' }); + }); + + it('getFieldLabels resolves — object AND locale arrive as path params', async () => { + const res = await client.i18n.getFieldLabels('customer', 'zh-CN'); + expect(res.object).toBe('customer'); + expect(res.locale).toBe('zh-CN'); + expect(res.labels).toEqual({ name: '名称' }); + }); + + it('getLocales resolves', async () => { + const res = await client.i18n.getLocales(); + expect(res.locales).toHaveLength(1); + }); + + it('the abandoned query dialect is dead on the wire — 404, both shapes', async () => { + // This is what the client sent before #3636. Nothing routes it. + for (const dead of [ + '/api/v1/i18n/translations?locale=zh-CN', + '/api/v1/i18n/labels/customer?locale=zh-CN', + ]) { + const res = await fetch(`${baseUrl}${dead}`); + expect(res.status, `${dead} should not resolve on any surface`).toBe(404); + } + }); +}); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 31b729f3ac..a193d27d9b 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -3494,24 +3494,39 @@ export class ObjectStackClient { }, /** - * Get translations for a locale + * Get translations for a locale. + * + * Speaks the path-param dialect (`/translations/:locale`) — the shape + * `plugin-rest-api.zod.ts` declares and the ONLY shape any serving surface + * mounts (service-i18n's autonomous routes, the dispatcher's HTTP mounts). + * The `?locale=` query form this used to send matched no route anywhere + * and 404'd on the wire; the dispatcher's domain body accepts it, but + * nothing ever routes a bare `/translations` to that body (#3636). */ getTranslations: async (locale: string, options?: { namespace?: string; keys?: string[] }): Promise => { const route = this.getRoute('i18n'); const params = new URLSearchParams(); - params.set('locale', locale); if (options?.namespace) params.set('namespace', options.namespace); if (options?.keys) params.set('keys', options.keys.join(',')); - const res = await this.fetch(`${this.baseUrl}${route}/translations?${params.toString()}`); + const query = params.toString(); + const res = await this.fetch( + `${this.baseUrl}${route}/translations/${encodeURIComponent(locale)}${query ? `?${query}` : ''}`, + ); return this.unwrapResponse(res); }, /** - * Get translated field labels for an object + * Get translated field labels for an object. + * + * Both the object and the locale ride the path (`/labels/:object/:locale`) + * — same reason as `getTranslations` above: the `?locale=` form could + * never match the two-path-param mount (#3636). */ getFieldLabels: async (object: string, locale: string): Promise => { const route = this.getRoute('i18n'); - const res = await this.fetch(`${this.baseUrl}${route}/labels/${encodeURIComponent(object)}?locale=${encodeURIComponent(locale)}`); + const res = await this.fetch( + `${this.baseUrl}${route}/labels/${encodeURIComponent(object)}/${encodeURIComponent(locale)}`, + ); return this.unwrapResponse(res); } }; diff --git a/packages/client/src/service-route-ledger-coverage.test.ts b/packages/client/src/service-route-ledger-coverage.test.ts new file mode 100644 index 0000000000..2cdc7122d5 --- /dev/null +++ b/packages/client/src/service-route-ledger-coverage.test.ts @@ -0,0 +1,90 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Service route-ledger ↔ client-surface conformance (#3636) — the client half + * of the guards whose server halves live next to the registrars that mount the + * routes: + * + * - `packages/services/service-storage/src/storage-route-ledger.conformance.test.ts` + * - `packages/services/service-i18n/src/i18n-route-ledger.conformance.test.ts` + * + * Same contract as the two ledger guards next door + * (`route-ledger-coverage.test.ts`, `rest-route-ledger-coverage.test.ts`): + * every ledger entry that names a client method must resolve to a real + * function on an instantiated client. + * + * Both ledgers are imported as relative SOURCE files deliberately: they are + * pure data (no imports), and a client→service package edge for them would be + * backwards — the services are where the routes are declared, so the ledgers + * live there, and each package verifies its own half. That is the tranche-1 + * lesson (`no runtime→client package edge`) applied verbatim: CI's per-package + * test tasks build only their own dependency closure, so a real package edge + * here would be unbuildable. + * + * With these two, all THREE server surfaces the SDK reaches are ledgered — + * dispatcher (#3563), REST (#3587), autonomous service mounts (#3636). + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectStackClient } from './index'; +import { STORAGE_ROUTE_LEDGER } from '../../services/service-storage/src/storage-route-ledger'; +import { I18N_ROUTE_LEDGER } from '../../services/service-i18n/src/i18n-route-ledger'; + +describe('service route ledgers ↔ @objectstack/client surface', () => { + const client = new ObjectStackClient({ baseUrl: 'http://localhost:9' }); + + const resolve = (path: string): unknown => + path.split('.').reduce((o, k) => (o == null ? o : (o as Record)[k]), client); + + const brokenIn = (ledger: readonly { route: string; client?: string }[]): string[] => + ledger + .filter((e) => e.client != null) + .filter((e) => typeof resolve(e.client!) !== 'function') + .map((e) => `${e.route} → client.${e.client}`); + + it('every storage ledger entry naming a client method resolves to a real function', () => { + const broken = brokenIn(STORAGE_ROUTE_LEDGER); + expect( + broken, + `storage ledger entries claiming a client method that does not exist: ${broken.join('; ')}`, + ).toEqual([]); + }); + + it('every i18n ledger entry naming a client method resolves to a real function', () => { + const broken = brokenIn(I18N_ROUTE_LEDGER); + expect( + broken, + `i18n ledger entries claiming a client method that does not exist: ${broken.join('; ')}`, + ).toEqual([]); + }); + + it('the whole `storage` namespace is backed by a ledger row', () => { + // The reverse direction, scoped to the namespace this tranche audited: + // a client method reaching a storage route that no ledger row backs is + // the pre-#3563 posture (expressed, working, unguarded) reappearing. + const claimed = new Set( + STORAGE_ROUTE_LEDGER.map((e) => e.client).filter((c): c is string => c != null), + ); + const unbacked = Object.keys((client as unknown as { storage: object }).storage) + .map((m) => `storage.${m}`) + .filter((m) => !claimed.has(m)); + expect( + unbacked, + `client.storage methods no storage-route-ledger row backs: ${unbacked.join(', ')}. ` + + 'Either the route is unledgered or the method targets a surface that does not exist (#3636).', + ).toEqual([]); + }); + + it('the whole `i18n` namespace is backed by a ledger row', () => { + const claimed = new Set( + I18N_ROUTE_LEDGER.map((e) => e.client).filter((c): c is string => c != null), + ); + const unbacked = Object.keys((client as unknown as { i18n: object }).i18n) + .map((m) => `i18n.${m}`) + .filter((m) => !claimed.has(m)); + expect( + unbacked, + `client.i18n methods no i18n-route-ledger row backs: ${unbacked.join(', ')}.`, + ).toEqual([]); + }); +}); diff --git a/packages/rest/src/rest-route-ledger.ts b/packages/rest/src/rest-route-ledger.ts index 5352aba71a..3c18c39ac2 100644 --- a/packages/rest/src/rest-route-ledger.ts +++ b/packages/rest/src/rest-route-ledger.ts @@ -28,10 +28,13 @@ * `external-datasource-routes.ts`) — the conformance test enumerates those by * capturing a mock server's registration calls, so they are guarded too. * - * NOT COVERED HERE (known third surface): services that autonomously mount + * NOT COVERED HERE (the third surface): services that autonomously mount * routes on the host `IHttpServer` — `service-storage` (`storage-routes.ts`, * the SDK's whole storage surface) and `service-i18n`. Those live outside - * `@objectstack/rest`; auditing them is follow-up work under #3587. + * `@objectstack/rest` and carry their own per-package ledgers + guards since + * #3636 (tranche 3): `packages/services/service-storage/src/storage-route-ledger.ts` + * and `packages/services/service-i18n/src/i18n-route-ledger.ts`. All three + * surfaces the SDK reaches are now ledgered. * * This module is package-internal (not exported from the index): it is the * guard's data, not public API. It must stay import-free — the client-side diff --git a/packages/runtime/src/route-ledger.ts b/packages/runtime/src/route-ledger.ts index 8b740b1ae0..427d36361c 100644 --- a/packages/runtime/src/route-ledger.ts +++ b/packages/runtime/src/route-ledger.ts @@ -18,9 +18,18 @@ * path). The REST server (`@objectstack/rest`) mounts a second, larger surface * (search, forms, reports, sharing rules, …) that the client also reaches; * that surface has its own ledger + guard since #3587 — - * `packages/rest/src/rest-route-ledger.ts`. Background: + * `packages/rest/src/rest-route-ledger.ts`. A third surface — services that + * mount autonomously on the host `IHttpServer` (`service-storage`, + * `service-i18n`) — carries per-package ledgers since #3636. Background: * `docs/audits/2026-07-dispatcher-client-route-coverage.md`. * + * A ROW HERE IS NOT A WIRE GUARANTEE. This ledger pairs a dispatcher-internal + * path pattern with a client method that exists; it does not check that the + * client builds a URL any server actually mounts. The three `/i18n` rows below + * looked healthy for exactly that reason while two of the three SDK methods + * spoke a `?locale=` dialect nothing routed (#3636). Cross-surface URL + * conformance is the capstone guard tracked in #3642. + * * This module is runtime-internal (not exported from the package index): it is * the guard's data, not public API. Promotion to `@objectstack/spec` is a * deliberate later step if other packages need it. diff --git a/packages/services/service-i18n/src/i18n-route-ledger.conformance.test.ts b/packages/services/service-i18n/src/i18n-route-ledger.conformance.test.ts new file mode 100644 index 0000000000..fe81507ec3 --- /dev/null +++ b/packages/services/service-i18n/src/i18n-route-ledger.conformance.test.ts @@ -0,0 +1,133 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * i18n route-ledger conformance (#3636) — the guard that keeps the + * autonomously-mounted i18n surface and `@objectstack/client` from drifting + * apart silently, mirroring the dispatcher guard (#3569) and the REST guard + * (#3587). + * + * Directions made loud here: + * + * 1. A route `I18nServicePlugin` mounts with no ledger entry. + * 2. A ledger entry for a route the plugin no longer mounts. + * + * Enumeration is real: the plugin is driven through its actual lifecycle + * (`init` → `start` → `kernel:ready`) against a capturing mock `IHttpServer`, + * so the registration calls ARE the route set — including the `kernel:ready` + * deferral, which a hand-pinned list would quietly stop exercising. + * + * The client half — every `sdk` row resolving to a real method — lives in + * `packages/client/src/service-route-ledger-coverage.test.ts`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { I18nServicePlugin } from './i18n-service-plugin'; +import { I18N_ROUTE_LEDGER } from './i18n-route-ledger'; + +/** Minimal IHttpServer mock that records registrations. */ +function createMockServer() { + return { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +/** PluginContext mock exposing the `http-server` service and replaying hooks. */ +function createMockContext(httpServer: unknown) { + const hooks = new Map Promise>>(); + return { + registerService: vi.fn(), + replaceService: vi.fn(), + getService: vi.fn((name: string) => { + if (name === 'http-server') return httpServer; + throw new Error(`Service '${name}' not found`); + }), + getServices: vi.fn(() => new Map()), + getKernel: vi.fn(), + hook: vi.fn((name: string, handler: (...args: unknown[]) => Promise) => { + if (!hooks.has(name)) hooks.set(name, []); + hooks.get(name)!.push(handler); + }), + trigger: vi.fn(async (name: string, ...args: unknown[]) => { + for (const h of hooks.get(name) ?? []) await h(...args); + }), + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + }; +} + +/** `VERB /path` keys for every route the plugin mounts at the DEFAULT base. */ +async function enumerateI18nRoutes(): Promise> { + const server = createMockServer(); + const ctx = createMockContext(server); + const plugin = new I18nServicePlugin(); + await plugin.init!(ctx as never); + await plugin.start!(ctx as never); + await ctx.trigger('kernel:ready'); + + const keys = new Set(); + for (const verb of ['get', 'post', 'put', 'patch', 'delete'] as const) { + for (const call of server[verb].mock.calls) { + keys.add(`${verb.toUpperCase()} ${call[0]}`); + } + } + return keys; +} + +const ledgerKeys = (): Set => new Set(I18N_ROUTE_LEDGER.map((e) => e.route)); + +describe('i18n route ledger ↔ I18nServicePlugin enumeration', () => { + it('every mounted i18n route has a ledger entry', async () => { + const ledger = ledgerKeys(); + const missing = [...(await enumerateI18nRoutes())].filter((k) => !ledger.has(k)); + expect( + missing, + `i18n routes with no i18n-route-ledger entry: ${missing.join(', ')}. ` + + 'A new route needs a reviewed disposition in i18n-route-ledger.ts (#3636).', + ).toEqual([]); + }); + + it('every ledger entry is really mounted by the plugin', async () => { + const live = await enumerateI18nRoutes(); + const stale = [...ledgerKeys()].filter((k) => !live.has(k)); + expect( + stale, + `i18n-route-ledger entries the plugin no longer mounts: ${stale.join(', ')}. ` + + 'Remove or reclassify them so the ledger stays truthful.', + ).toEqual([]); + }); + + it('no route is ledgered twice', () => { + const seen = new Set(); + const dupes = I18N_ROUTE_LEDGER.map((e) => e.route).filter((r) => !seen.add(r)); + expect(dupes, `duplicate i18n-route-ledger rows: ${dupes.join(', ')}`).toEqual([]); + }); +}); + +describe('i18n route ledger hygiene', () => { + it('every `sdk` entry names its client method; every non-sdk entry carries a rationale', () => { + const sdkWithout = I18N_ROUTE_LEDGER.filter((e) => e.disposition === 'sdk' && !e.client).map((e) => e.route); + expect(sdkWithout, 'sdk-disposition entries missing a client method name').toEqual([]); + + const bareNonSdk = I18N_ROUTE_LEDGER.filter((e) => e.disposition !== 'sdk' && !e.note).map((e) => e.route); + expect(bareNonSdk, 'non-sdk entries must say WHY they are not SDK surface').toEqual([]); + }); + + it('gap and mismatch counts only shrink — update the ledger (and these numbers) when closing them', () => { + // Ratchet, not aspiration. This surface audited at TWO mismatches (#3636): + // getTranslations and getFieldLabels each spoke a `?locale=` dialect no + // server mounts. Both were closed in the same PR by moving the client to + // the path form the spec declares, so all three routes are SDK-expressed + // and these bounds stay 0. + const gaps = I18N_ROUTE_LEDGER.filter((e) => e.disposition === 'gap').length; + expect(gaps).toBeLessThanOrEqual(0); + + const mismatches = I18N_ROUTE_LEDGER.filter((e) => e.disposition === 'mismatch').length; + expect(mismatches).toBeLessThanOrEqual(0); + }); +}); diff --git a/packages/services/service-i18n/src/i18n-route-ledger.ts b/packages/services/service-i18n/src/i18n-route-ledger.ts new file mode 100644 index 0000000000..9f6bdd1921 --- /dev/null +++ b/packages/services/service-i18n/src/i18n-route-ledger.ts @@ -0,0 +1,70 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * i18n route ledger — the audited disposition of every HTTP route + * `I18nServicePlugin` mounts, against what `@objectstack/client` can express + * (#3636, tranche 3 of the #3563 audit). + * + * WHY THIS EXISTS. Like `service-storage`, this plugin reaches for the + * `http-server` service at `kernel:ready` and registers straight on + * `IHttpServer`, so neither the dispatcher ledger nor the REST ledger has ever + * seen these routes. The dispatcher ledger DOES carry three `/i18n/*` rows — + * but those guard the dispatcher's own mounts, and its guard only asks whether + * the named client method EXISTS. It never asked whether the client speaks a + * URL shape any server actually mounts, which is precisely what this tranche + * caught: `i18n.getTranslations` and `i18n.getFieldLabels` shipped the + * `?locale=` query dialect while every serving surface — this plugin, the + * dispatcher's HTTP mounts, and the `plugin-rest-api.zod.ts` contract — mounts + * only the path form. Both were 404s on the wire. Fixed in #3636 by moving the + * client to the path dialect, the same resolution #3611 gave `meta.getView`. + * + * `i18n-route-ledger.conformance.test.ts` fails when a route appears with no + * ledger entry, and when an entry names a route the plugin no longer mounts. + * The client half lives in + * `packages/client/src/service-route-ledger-coverage.test.ts` (no + * service→client package edge, per the tranche-1 lesson). + * + * SCOPE & SHAPE. Rows carry full wire paths at the DEFAULT base + * (`/api/v1/i18n`); `I18nServicePluginOptions.basePath` can move the family, + * and the conformance test enumerates at the same default. The whole family is + * behind the `registerRoutes` option (default on) — when a host turns it off, + * the dispatcher's `/i18n` domain still serves the same three shapes, which is + * why route registration stays multi-provider. + * + * This module is package-internal (not exported from the index): it is the + * guard's data, not public API. It must stay import-free — the client-side + * guard imports it as a relative SOURCE file. + */ + +/** Disposition of a single i18n route. Same vocabulary as the REST ledger. */ +export type I18nRouteDisposition = + /** Expressed by the SDK — `client` names the method (dotted path). */ + | 'sdk' + /** Should be in the SDK and is not — an open, acknowledged gap. */ + | 'gap' + /** Deliberately not SDK surface. */ + | 'server-only' + /** Public, unauthenticated browser-facing route. */ + | 'public' + /** Server and client disagree on the shape — needs reconciliation. */ + | 'mismatch'; + +export interface I18nRouteLedgerEntry { + /** `VERB /api/v1/i18n/...` — full wire path at the default base. */ + route: string; + /** Registrar family, for grouping and diff messages. */ + family: string; + disposition: I18nRouteDisposition; + /** Dotted method path on `ObjectStackClient` — required when disposition is `sdk`. */ + client?: string; + /** One-line rationale. Required for every non-`sdk` disposition. */ + note?: string; +} + +export const I18N_ROUTE_LEDGER: readonly I18nRouteLedgerEntry[] = [ + { route: 'GET /api/v1/i18n/locales', family: 'i18n', disposition: 'sdk', client: 'i18n.getLocales' }, + { route: 'GET /api/v1/i18n/translations/:locale', family: 'i18n', disposition: 'sdk', client: 'i18n.getTranslations', + note: 'was a wire-level 404 — the client sent /translations?locale=xx, a shape no server mounts (the dispatcher domain body accepts it, but nothing routes a bare /translations to that body). Since #3636 the client sends the path form the spec declares.' }, + { route: 'GET /api/v1/i18n/labels/:object/:locale', family: 'i18n', disposition: 'sdk', client: 'i18n.getFieldLabels', + note: 'ditto — the client sent /labels/:object?locale=xx against a two-path-param mount (#3636).' }, +]; diff --git a/packages/services/service-i18n/src/i18n-service-plugin.test.ts b/packages/services/service-i18n/src/i18n-service-plugin.test.ts index 4800d67f13..a3069c9891 100644 --- a/packages/services/service-i18n/src/i18n-service-plugin.test.ts +++ b/packages/services/service-i18n/src/i18n-service-plugin.test.ts @@ -196,6 +196,7 @@ describe('I18nServicePlugin', () => { await handler(req, res); expect(res._data).toEqual({ + success: true, data: { locales: [ { code: 'en', label: 'en', isDefault: true }, @@ -216,6 +217,7 @@ describe('I18nServicePlugin', () => { await handler(req, res); expect(res._data).toEqual({ + success: true, data: { locale: 'en', translations: { greeting: 'Hello', 'o.account.fields.name': 'Account Name' }, @@ -246,6 +248,7 @@ describe('I18nServicePlugin', () => { await handler(req, res); expect(res._data).toEqual({ + success: true, data: { object: 'account', locale: 'en', diff --git a/packages/services/service-i18n/src/i18n-service-plugin.ts b/packages/services/service-i18n/src/i18n-service-plugin.ts index abe0dbb647..ae69b64475 100644 --- a/packages/services/service-i18n/src/i18n-service-plugin.ts +++ b/packages/services/service-i18n/src/i18n-service-plugin.ts @@ -129,10 +129,21 @@ export class I18nServicePlugin implements Plugin { /** * Register i18n REST routes with the HTTP server. * - * Routes: + * Routes (ledgered in `i18n-route-ledger.ts`, guarded by + * `i18n-route-ledger.conformance.test.ts` — #3636): * - GET /api/v1/i18n/locales → list available locales * - GET /api/v1/i18n/translations/:locale → get translations for a locale * - GET /api/v1/i18n/labels/:object/:locale → get field labels for an object + * + * Success bodies carry the `{ success: true, data }` BaseResponse envelope + * the `i18n` route group declares (`plugin-rest-api.zod.ts` → + * `response_envelope`), matching what the dispatcher's `/i18n` domain emits + * for the same three shapes. They used to omit the `success` flag, which is + * the flag `ObjectStackClient.unwrapResponse` keys on — so the SDK handed + * callers the raw `{ data: … }` wrapper against THIS provider while + * returning the declared unwrapped shape against the dispatcher. Same + * method, two shapes, decided by which plugin happened to mount the route + * (#3636). `data` did not move, so direct body readers are unaffected. */ private registerI18nRoutes(httpServer: IHttpServer, ctx: PluginContext): void { if (!this.i18n) return; @@ -146,6 +157,7 @@ export class I18nServicePlugin implements Plugin { const locales = i18n.getLocales(); const defaultLocale = i18n.getDefaultLocale?.() ?? 'en'; res.json({ + success: true, data: { locales: locales.map((code) => ({ code, @@ -168,7 +180,7 @@ export class I18nServicePlugin implements Plugin { return; } const translations = i18n.getTranslations(locale); - res.json({ data: { locale, translations } }); + res.json({ success: true, data: { locale, translations } }); } catch (error: any) { res.status(500).json({ error: error.message }); } @@ -189,7 +201,7 @@ export class I18nServicePlugin implements Plugin { if (hasGetFieldLabels) { const labels = (i18n as II18nService & { getFieldLabels(obj: string, loc: string): Record }) .getFieldLabels(objectName, locale); - res.json({ data: { object: objectName, locale, labels } }); + res.json({ success: true, data: { object: objectName, locale, labels } }); } else { // Fallback: derive field labels from full translation bundle const translations = i18n.getTranslations(locale); @@ -200,7 +212,7 @@ export class I18nServicePlugin implements Plugin { labels[key.substring(prefix.length)] = value as string; } } - res.json({ data: { object: objectName, locale, labels } }); + res.json({ success: true, data: { object: objectName, locale, labels } }); } } catch (error: any) { res.status(500).json({ error: error.message }); diff --git a/packages/services/service-storage/src/storage-route-ledger.conformance.test.ts b/packages/services/service-storage/src/storage-route-ledger.conformance.test.ts new file mode 100644 index 0000000000..f4357d5933 --- /dev/null +++ b/packages/services/service-storage/src/storage-route-ledger.conformance.test.ts @@ -0,0 +1,117 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Storage route-ledger conformance (#3636) — the guard that keeps the + * autonomously-mounted storage surface and `@objectstack/client` from drifting + * apart silently, mirroring the dispatcher guard (#3569) and the REST guard + * (#3587). + * + * Directions made loud here: + * + * 1. A route `registerStorageRoutes` mounts with no ledger entry — a new + * route landed without a reviewed SDK disposition. + * 2. A ledger entry for a route the registrar no longer mounts — the ledger + * went stale. + * + * Enumeration is real, not a hand-pinned list: the registrar runs against a + * capturing mock `IHttpServer` and its registration calls ARE the route set — + * the same seam tranche 2 used for the two RouteManager-bypassing registrars. + * + * The third direction — "every `sdk` row names a client method that exists" — + * lives in `packages/client/src/service-route-ledger-coverage.test.ts`, next + * to the SDK it introspects: a service→client package edge would be backwards + * (the routes are declared here, so the ledger lives here) and each package + * verifies its own half. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { registerStorageRoutes } from './storage-routes'; +import { STORAGE_ROUTE_LEDGER } from './storage-route-ledger'; + +/** Minimal IHttpServer mock that records registrations. */ +function createMockServer() { + return { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +/** + * `VERB /path` keys for every route the registrar mounts at the DEFAULT base. + * + * Registration only closes over `storage`/`store` — nothing is called until a + * request arrives — so bare stubs enumerate the full surface. Options are left + * empty on purpose: no storage route is opt-in, and any that became so would + * surface here as a ledger diff rather than vanishing silently. + */ +function enumerateStorageRoutes(): Set { + const server = createMockServer(); + registerStorageRoutes(server as any, {} as any, {} as any); + const keys = new Set(); + for (const verb of ['get', 'post', 'put', 'patch', 'delete'] as const) { + for (const call of server[verb].mock.calls) { + keys.add(`${verb.toUpperCase()} ${call[0]}`); + } + } + return keys; +} + +const ledgerKeys = (): Set => new Set(STORAGE_ROUTE_LEDGER.map((e) => e.route)); + +describe('storage route ledger ↔ registerStorageRoutes enumeration', () => { + it('every mounted storage route has a ledger entry', () => { + const ledger = ledgerKeys(); + const missing = [...enumerateStorageRoutes()].filter((k) => !ledger.has(k)); + expect( + missing, + `Storage routes with no storage-route-ledger entry: ${missing.join(', ')}. ` + + 'A new route needs a reviewed disposition in storage-route-ledger.ts (#3636).', + ).toEqual([]); + }); + + it('every ledger entry is really mounted by the registrar', () => { + const live = enumerateStorageRoutes(); + const stale = [...ledgerKeys()].filter((k) => !live.has(k)); + expect( + stale, + `storage-route-ledger entries the registrar no longer mounts: ${stale.join(', ')}. ` + + 'Remove or reclassify them so the ledger stays truthful.', + ).toEqual([]); + }); + + it('no route is ledgered twice', () => { + const seen = new Set(); + const dupes = STORAGE_ROUTE_LEDGER.map((e) => e.route).filter((r) => !seen.add(r)); + expect(dupes, `duplicate storage-route-ledger rows: ${dupes.join(', ')}`).toEqual([]); + }); +}); + +describe('storage route ledger hygiene', () => { + it('every `sdk` entry names its client method; every non-sdk entry carries a rationale', () => { + const sdkWithout = STORAGE_ROUTE_LEDGER.filter((e) => e.disposition === 'sdk' && !e.client).map((e) => e.route); + expect(sdkWithout, 'sdk-disposition entries missing a client method name').toEqual([]); + + const bareNonSdk = STORAGE_ROUTE_LEDGER.filter((e) => e.disposition !== 'sdk' && !e.note).map((e) => e.route); + expect(bareNonSdk, 'non-sdk entries must say WHY they are not SDK surface').toEqual([]); + }); + + it('gap and mismatch counts only shrink — update the ledger (and these numbers) when closing them', () => { + // Ratchet, not aspiration. The storage surface audited CLEAN at #3636: + // seven routes carry SDK expression, three are reviewed `server-only` + // (the browser capability URL and the two local-driver loopbacks). Every + // storage route is therefore either SDK-expressed or explicitly not SDK + // surface — a new `gap` or `mismatch` row demands a reviewed decision, so + // these bounds stay 0. + const gaps = STORAGE_ROUTE_LEDGER.filter((e) => e.disposition === 'gap').length; + expect(gaps).toBeLessThanOrEqual(0); + + const mismatches = STORAGE_ROUTE_LEDGER.filter((e) => e.disposition === 'mismatch').length; + expect(mismatches).toBeLessThanOrEqual(0); + }); +}); diff --git a/packages/services/service-storage/src/storage-route-ledger.ts b/packages/services/service-storage/src/storage-route-ledger.ts new file mode 100644 index 0000000000..58e25bd4c7 --- /dev/null +++ b/packages/services/service-storage/src/storage-route-ledger.ts @@ -0,0 +1,86 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Storage route ledger — the audited disposition of every HTTP route + * `registerStorageRoutes` mounts, against what `@objectstack/client` can + * express (#3636, tranche 3 of the #3563 audit). + * + * WHY THIS EXISTS. Tranches 1 and 2 ledgered the dispatcher + * (`packages/runtime/src/route-ledger.ts`) and the REST server + * (`packages/rest/src/rest-route-ledger.ts`). Both stop at their own package + * boundary, and this surface is outside both: `StorageServicePlugin` reaches + * for the `http-server` service and registers straight on `IHttpServer`, so + * neither `RouteManager` nor `RestServer.getRoutes()` has ever seen these + * routes. That left the SDK's ENTIRE storage surface — upload, chunked + * upload, download-URL resolution — in the exact pre-#3563 posture: expressed, + * working, and guarded by nothing. + * + * `storage-route-ledger.conformance.test.ts` fails when a storage route + * appears with no ledger entry, and when an entry names a route the registrar + * no longer mounts. The client half — every `sdk` row resolving to a real + * method — lives in `packages/client/src/service-route-ledger-coverage.test.ts`, + * next to the SDK it introspects, for the tranche-1 build-cycle reason: a + * service→client package edge would be backwards. + * + * SCOPE & SHAPE. Rows carry full wire paths at the DEFAULT base + * (`/api/v1/storage`); `StorageRoutesOptions.basePath` can move the whole + * family, and the conformance test enumerates at the same default so the two + * stay comparable. `GET {base}/_local/file/:key` is deliberately ABSENT — three + * call sites build that URL but no registrar has ever mounted it (#3641). + * + * This module is package-internal (not exported from the index): it is the + * guard's data, not public API. It must stay import-free — the client-side + * guard imports it as a relative SOURCE file. + */ + +/** Disposition of a single storage route. Same vocabulary as the REST ledger. */ +export type StorageRouteDisposition = + /** Expressed by the SDK — `client` names the method (dotted path). */ + | 'sdk' + /** Should be in the SDK and is not — an open, acknowledged gap. */ + | 'gap' + /** Deliberately not SDK surface (driver loopbacks, browser capability URLs). */ + | 'server-only' + /** Public, unauthenticated browser-facing route. */ + | 'public' + /** Server and client disagree on the shape — needs reconciliation. */ + | 'mismatch'; + +export interface StorageRouteLedgerEntry { + /** `VERB /api/v1/storage/...` — full wire path at the default base. */ + route: string; + /** Registrar family, for grouping and diff messages. */ + family: string; + disposition: StorageRouteDisposition; + /** Dotted method path on `ObjectStackClient` — required when disposition is `sdk`. */ + client?: string; + /** One-line rationale. Required for every non-`sdk` disposition. */ + note?: string; +} + +export const STORAGE_ROUTE_LEDGER: readonly StorageRouteLedgerEntry[] = [ + // ── presigned upload (the SDK's default upload path) ────────────────────── + { route: 'POST /api/v1/storage/upload/presigned', family: 'upload-presigned', disposition: 'sdk', client: 'storage.getPresignedUrl', + note: 'also step 1 of the composite storage.upload' }, + { route: 'POST /api/v1/storage/upload/complete', family: 'upload-presigned', disposition: 'sdk', client: 'storage.upload', + note: 'step 3 of the composite upload; no standalone client method mints the commit on its own' }, + + // ── chunked / resumable upload ──────────────────────────────────────────── + { route: 'POST /api/v1/storage/upload/chunked', family: 'upload-chunked', disposition: 'sdk', client: 'storage.initChunkedUpload' }, + { route: 'PUT /api/v1/storage/upload/chunked/:uploadId/chunk/:chunkIndex', family: 'upload-chunked', disposition: 'sdk', client: 'storage.uploadPart' }, + { route: 'POST /api/v1/storage/upload/chunked/:uploadId/complete', family: 'upload-chunked', disposition: 'sdk', client: 'storage.completeChunkedUpload' }, + { route: 'GET /api/v1/storage/upload/chunked/:uploadId/progress', family: 'upload-chunked', disposition: 'sdk', client: 'storage.resumeUpload', + note: 'progress is read as the first step of resumeUpload; the SDK exposes no bare progress poll' }, + + // ── download ────────────────────────────────────────────────────────────── + { route: 'GET /api/v1/storage/files/:fileId/url', family: 'download', disposition: 'sdk', client: 'storage.getDownloadUrl', + note: 'JSON { url } — the authorization-gated signed-URL resolve the dispatcher ledger points at (#3584)' }, + { route: 'GET /api/v1/storage/files/:fileId', family: 'download', disposition: 'server-only', + note: 'stable 302 to the same signed URL. This is the value objectql stamps into file/image field payloads (engine.ts buildFileValue), followed verbatim by / — a browser URL, not an SDK call. The SDK resolves via the /url sibling above.' }, + + // ── local-driver loopback (LocalStorageAdapter presign targets) ─────────── + { route: 'PUT /api/v1/storage/_local/raw/:token', family: 'local-driver', disposition: 'server-only', + note: 'the uploadUrl LocalStorageAdapter mints for its own presign tokens. storage.upload does PUT it, but opaquely — via fetchImpl on whatever uploadUrl came back, exactly as it would an S3 presigned URL. HMAC-token-authorized, adapter-internal, never a named SDK method.' }, + { route: 'GET /api/v1/storage/_local/raw/:token', family: 'local-driver', disposition: 'server-only', + note: 'ditto for download: the target of getPresignedDownload/getSignedUrl on the local adapter, handed to the browser as an opaque signed link.' }, +];