diff --git a/.changeset/save-meta-response-full-fields.md b/.changeset/save-meta-response-full-fields.md new file mode 100644 index 0000000000..25fb60f838 --- /dev/null +++ b/.changeset/save-meta-response-full-fields.md @@ -0,0 +1,56 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): `SaveMetaItemResponseSchema` declares the whole save response — `version` / `seq` / `state` / `projectionApplied` (#5745) + +`PUT /api/v1/meta/:type/:name` has always answered with more than the schema +admitted. The declaration stopped at `{ success, message? }` while the route +returns `{ success, version, seq, state, message }` — plus `projectionApplied` +when a projector is registered — so the contract described a proper subset of +the real body. + +Because these are plain `z.object` schemas, the gap failed in the quietest way +available: `safeParse` stayed **green** and the undeclared keys were silently +**stripped**. Measured on `origin/main` before the change: + +``` +raw keys : ["success","version","seq","state","message"] +after parse : ["success","message"] +STRIPPED : ["version","seq","state"] +safeParse ok : true +``` + +`version` is the field this matters most for: it is the token the ADR-0008 +optimistic-concurrency chain already runs on — echo it back as `If-Match` on +the next write and a concurrent edit returns 409 `metadata_conflict` instead of +silently overwriting. It was being carried on the wire with no contract behind +it, so a consumer that parsed the response lost exactly the value the OCC +handshake needs. + +**Consumer-visible change.** Before, a `SaveMetaItemResponseSchema.parse()` +dropped the four fields and `SaveMetaItemResponse` could not name them at the +type level. Now they survive the parse and are typed: + +- `version: string` — required. Opaque content hash; the ADR-0008 `If-Match` + token. Echo it verbatim, never parse it. +- `seq: number` (integer) — required. Metadata-event sequence number; orders + writes, but is not an OCC token. +- `state: 'draft' | 'active'` — required. The lifecycle the body landed in. +- `projectionApplied?: { success: boolean; error?: string }` — optional. The + ADR-0094 mutation-projector outcome, present only when a projector is + registered for that metadata type. Its absence means "no projector ran", + never "the projection failed"; a caller that needs the derived read model to + be live must check `projectionApplied.success` rather than trust the 200. + +The three required fields are required because measurement says the producer +always emits them, not by assumption: `saveMetaItem` has a single success +return — the repository write path — and the REST route hands that object to +`res.json()` verbatim. A second, receipt-less legacy return would have forced +all three to be optional; it was proved unreachable and deleted in #5264 / +PR #5782, which is what makes `required` safe to state here. + +No runtime behaviour changes: the route already returned these fields, and +nothing parsed the response through this schema. `client.meta.saveItem`'s +return-type annotation is deliberately left for the cli lane (#5545) so it is +written against the landed contract rather than ahead of it. diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index d64a35e40d..2b59cfed4d 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -1421,6 +1421,10 @@ List packages response | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **success** | `boolean` | ✅ | | +| **version** | `string` | ✅ | Content hash of the just-committed body, and the token the ADR-0008 optimistic-concurrency chain runs on: send it back as the `If-Match` request header on the next write to that item and a concurrent edit is reported as 409 `metadata_conflict` instead of silently overwritten. Opaque to callers — echo it verbatim, never parse it. Currently emitted as `sha256:<64 hex chars>`, but the format is not part of this contract. | +| **seq** | `integer` | ✅ | Monotonic sequence number of the metadata event this write appended to the item history (sys_metadata_history.event_seq). Orders writes; unlike `version` it is not an OCC token. | +| **state** | `Enum<'draft' \| 'active'>` | ✅ | Lifecycle the body was written into: "draft" when the request asked for draft mode (`?mode=draft`), otherwise "active" (published and live). A draft is staged only — it is not served to the runtime until published. | +| **projectionApplied** | `{ success: boolean; error?: string }` | optional | Outcome of the awaited ADR-0094 mutation projector — the post-persist step that materializes this metadata into its derived data-plane read model (e.g. `permission` → `sys_permission_set`). Present ONLY when a projector is registered for this metadata type, which is why it is optional: its absence means "no projector ran", never "the projection failed". Best-effort by design — a projector failure is reported here and logged, never thrown, so a caller that needs the read model to be live must check `projectionApplied.success` rather than rely on the 200. | | **message** | `string` | optional | | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md index 65d5b02e66..fae2e8813e 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md @@ -262,7 +262,7 @@ directory rather than per file. | Dir | Sites | |---|---| | `ai/` | 77 | -| `api/` | 395 | +| `api/` | 396 | | `cloud/` | 82 | | `identity/` | 33 | | `integration/` | 10 | diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index d20cda3614..8460d4507a 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -7938,7 +7938,12 @@ export class ObjectStackProtocolImplementation implements version: result.version, seq: result.seq, ...(projectionApplied ? { projectionApplied } : {}), - state: mode === 'draft' ? 'draft' : 'active', + // #5745 — the literal union, not `string`. An object-literal + // property widens a two-literal ternary to `string`, which made + // this method fail to satisfy `MetadataProtocol.saveMetaItem` + // once the spec declared `state` as the closed set it has always + // emitted. Type-only: the value is unchanged. + state: (mode === 'draft' ? 'draft' : 'active') as 'draft' | 'active', message: orgId ? `Saved customization overlay (org=${orgId}, state=${mode === 'draft' ? 'draft' : 'active'}) — type=${request.type}, name=${request.name} [seq=${result.seq}]` : `Saved customization overlay (env-wide, state=${mode === 'draft' ? 'draft' : 'active'}) — type=${request.type}, name=${request.name} [seq=${result.seq}]`, diff --git a/packages/objectql/src/save-meta-response-conformance.test.ts b/packages/objectql/src/save-meta-response-conformance.test.ts new file mode 100644 index 0000000000..e0b6611f6e --- /dev/null +++ b/packages/objectql/src/save-meta-response-conformance.test.ts @@ -0,0 +1,203 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5745 — conformance gate: the body `saveMetaItem` really returns must parse + * through `SaveMetaItemResponseSchema` with NOTHING stripped. + * + * This is the producer side of the declaration. The spec-side suite + * (`packages/spec/src/api/protocol.test.ts`) pins what the schema says; this + * one pins that the schema still matches what the code emits, driving the REAL + * protocol against a REAL ObjectQL engine. The two together are what makes + * "declared = returned" checkable — a future field added to the response, or an + * existing one dropped, turns this red instead of silently vanishing at parse. + * + * Why the REST layer needs no separate case: the route hands this exact object + * to `res.json()` verbatim (`rest-server.ts`, `PUT /meta/:type/:name`), so the + * protocol return IS the wire body. + * + * Before the #5745 declaration this file's first assertion was red in a + * specific, quiet way: `safeParse` SUCCEEDED and `version` / `seq` / `state` + * were dropped from the parsed result, so the "stripped keys" set was + * non-empty. That is the direction it must never drift back to. + */ +import { describe, it, expect } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SaveMetaItemResponseSchema } from '@objectstack/spec/api'; +import { ObjectQL } from './engine.js'; + +const sysMetadataObject = { + name: 'sys_metadata', + label: 'System Metadata', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + type: { name: 'type', label: 'Type', type: 'text' as const, required: true }, + name: { name: 'name', label: 'Name', type: 'text' as const, required: true }, + organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + metadata: { name: 'metadata', label: 'Body', type: 'longtext' as const }, + checksum: { name: 'checksum', label: 'Checksum', type: 'text' as const, maxLength: 71 }, + state: { name: 'state', label: 'State', type: 'text' as const }, + version: { name: 'version', label: 'Version', type: 'number' as const }, + created_at: { name: 'created_at', label: 'Created', type: 'datetime' as const }, + updated_at: { name: 'updated_at', label: 'Updated', type: 'datetime' as const }, + }, +}; + +function makeMemoryDriver() { + const stores = new Map>>(); + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { s = new Map(); stores.set(obj, s); } + return s; + }; + let nextId = 0; + const matchesWhere = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + if (Array.isArray(where.$and)) return where.$and.every((w: any) => matchesWhere(row, w)); + if (Array.isArray(where.$or)) return where.$or.some((w: any) => matchesWhere(row, w)); + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + const rowVal = row[k]; + const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; + const a = rowVal === undefined ? null : rowVal; + const b = expected === undefined ? null : expected; + if (a !== b) return false; + } + return true; + }; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {} as any, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any) { + return Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where)); + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const cur = s.get(id); + if (!cur) throw new Error(`not found: ${object}/${id}`); + const updated = { ...cur, ...data, id }; + s.set(id, updated); + return updated; + }, + async upsert(object: string, data: Record) { + const id = data.id as string | undefined; + if (id && storeFor(object).has(id)) return this.update(object, id, data); + return this.create(object, data); + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { return (await this.find(object, ast)).length; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, stores }; +} + +async function makeProtocol() { + const engine = new ObjectQL(); + const { driver } = makeMemoryDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(sysMetadataObject as any); + return new ObjectStackProtocolImplementation(engine); +} + +const LOG = (...a: any[]) => appendFileSync(OUT, a.join(' ') + '\n'); + +const viewBody = (label: string) => ({ name: 'cases', type: 'grid', label, columns: ['id'] }); + +/** Keys the producer emitted that the schema refused to carry through. */ +function strippedKeys(raw: Record): string[] { + const parsed = SaveMetaItemResponseSchema.parse(raw) as Record; + return Object.keys(raw).filter((k) => !(k in parsed)); +} + +describe('saveMetaItem response conforms to SaveMetaItemResponseSchema (#5745)', () => { + it('publish-mode save: parses green and strips nothing', async () => { + const p = await makeProtocol(); + const raw: any = await p.saveMetaItem({ + type: 'view', name: 'cases', organizationId: 'org_x', item: viewBody('A'), + }); + + expect(strippedKeys(raw)).toEqual([]); + const parsed = SaveMetaItemResponseSchema.parse(raw); + expect(parsed.success).toBe(true); + expect(parsed.state).toBe('active'); + expect(parsed.seq).toBe(1); + // The ADR-0008 OCC token survives parse — this is the value a caller + // echoes back as `If-Match` on the next write to this item. + expect(parsed.version).toBe(raw.version); + expect(typeof parsed.version).toBe('string'); + }); + + it('draft-mode save: state is "draft" and still strips nothing', async () => { + const p = await makeProtocol(); + const raw: any = await p.saveMetaItem({ + type: 'view', name: 'cases', organizationId: 'org_x', item: viewBody('D'), mode: 'draft', + }); + + expect(strippedKeys(raw)).toEqual([]); + expect(SaveMetaItemResponseSchema.parse(raw).state).toBe('draft'); + }); + + it('with an ADR-0094 projector registered: projectionApplied is carried through', async () => { + const p = await makeProtocol(); + p.registerMutationProjector('view', async () => { throw new Error('boom-from-projector'); }); + + const raw: any = await p.saveMetaItem({ + type: 'view', name: 'cases', organizationId: 'org_x', item: viewBody('P'), + }); + + expect(Object.keys(raw)).toContain('projectionApplied'); + expect(strippedKeys(raw)).toEqual([]); + const parsed = SaveMetaItemResponseSchema.parse(raw); + // Best-effort by contract: the projector threw, the write still succeeded, + // and the failure is reported here rather than as a non-200. + expect(parsed.success).toBe(true); + expect(parsed.projectionApplied).toEqual({ success: false, error: 'boom-from-projector' }); + }); + + it('no projector registered → projectionApplied is absent, which is why it alone is optional', async () => { + const p = await makeProtocol(); + const raw: any = await p.saveMetaItem({ + type: 'view', name: 'cases', organizationId: 'org_x', item: viewBody('N'), + }); + + expect(raw.projectionApplied).toBeUndefined(); + expect(SaveMetaItemResponseSchema.safeParse(raw).success).toBe(true); + }); + + it('version / seq / state are required because no reachable success return omits them', async () => { + // `saveMetaItem` now has exactly ONE success return — the repository + // write path — and it always sets all three. The shape that carried + // none of them was the legacy raw-engine return, deleted in #5264 / + // PR #5782 after being proved unreachable; the gate that made it + // unreachable is the one exercised here, and it is still what keeps a + // second, receipt-less write path from appearing. A type declaring + // neither `allowOrgOverride` nor `allowRuntimeCreate` (`agent`, `job`) + // is refused outright rather than persisted without a receipt. + // + // This is the tripwire for the `required` decision: if that gate is + // ever relaxed so such a type is written some other way, whatever + // receipt that path returns has to be re-measured before these three + // fields can stay required. + const p = await makeProtocol(); + await expect( + p.saveMetaItem({ type: 'agent', name: 'helper', organizationId: 'org_x', item: { name: 'helper' } }), + ).rejects.toMatchObject({ code: 'NOT_CREATABLE', status: 403 }); + }); +}); diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 38d8ac84a7..c219c02156 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -1811,7 +1811,11 @@ "api/SaveMetaItemRequest:name", "api/SaveMetaItemRequest:type", "api/SaveMetaItemResponse:message", + "api/SaveMetaItemResponse:projectionApplied", + "api/SaveMetaItemResponse:seq", + "api/SaveMetaItemResponse:state", "api/SaveMetaItemResponse:success", + "api/SaveMetaItemResponse:version", "api/ScheduleExportRequest:delivery", "api/ScheduleExportRequest:fields", "api/ScheduleExportRequest:filter", diff --git a/packages/spec/src/api/protocol.test.ts b/packages/spec/src/api/protocol.test.ts index 0dd9bc12f9..774aae5689 100644 --- a/packages/spec/src/api/protocol.test.ts +++ b/packages/spec/src/api/protocol.test.ts @@ -600,3 +600,79 @@ describe('HttpFindQueryParamsSchema', () => { expect(HttpFindQueryParamsSchema.safeParse({}).success).toBe(true); }); }); + +import { SaveMetaItemResponseSchema } from './protocol.zod'; + +/** + * #5745 — `SaveMetaItemResponseSchema` must describe the FULL body the save + * route returns, not a subset of it. + * + * Before this suite the declaration was `{ success, message? }`, so parsing a + * real response silently DROPPED `version` / `seq` / `state` / + * `projectionApplied` — `safeParse` stayed green while the data disappeared, + * which is the worst shape of failure to leave for a consumer. The first case + * below is the one that was red: it asserts nothing is stripped. + * + * Optionality is measured, not assumed (evidence in the PR): the sole producer + * always emits `version` / `seq` / `state` on its only reachable success + * return, and emits `projectionApplied` only when an ADR-0094 mutation + * projector is registered for the type. + */ +describe('SaveMetaItemResponseSchema (#5745 — declares the full save response)', () => { + /** A verbatim capture of a real `saveMetaItem` return (repo write path). */ + const realResponse = { + success: true, + version: 'sha256:7aad99c8d969efb5067fff275fb3e5be7ec90f9cd610d41709fcddbf8c34b1f0', + seq: 1, + state: 'active', + message: 'Saved customization overlay (org=org_x, state=active) — type=view, name=cases [seq=1]', + }; + + it('round-trips a real response without stripping any field', () => { + const parsed = SaveMetaItemResponseSchema.parse(realResponse); + expect(Object.keys(parsed).sort()).toEqual(Object.keys(realResponse).sort()); + expect(parsed).toEqual(realResponse); + }); + + it('carries the ADR-0008 OCC token: version survives parse as the If-Match value', () => { + const parsed = SaveMetaItemResponseSchema.parse(realResponse); + expect(parsed.version).toBe(realResponse.version); + }); + + it('keeps seq as an integer and rejects a fractional one', () => { + expect(SaveMetaItemResponseSchema.parse(realResponse).seq).toBe(1); + expect(SaveMetaItemResponseSchema.safeParse({ ...realResponse, seq: 1.5 }).success).toBe(false); + }); + + it('accepts both lifecycle states and rejects any other', () => { + expect(SaveMetaItemResponseSchema.safeParse({ ...realResponse, state: 'draft' }).success).toBe(true); + expect(SaveMetaItemResponseSchema.safeParse({ ...realResponse, state: 'active' }).success).toBe(true); + expect(SaveMetaItemResponseSchema.safeParse({ ...realResponse, state: 'published' }).success).toBe(false); + }); + + it('requires version / seq / state — the producer always emits them', () => { + for (const missing of ['version', 'seq', 'state'] as const) { + const body: Record = { ...realResponse }; + delete body[missing]; + expect( + SaveMetaItemResponseSchema.safeParse(body).success, + `omitting '${missing}' must fail parse`, + ).toBe(false); + } + }); + + it('leaves projectionApplied optional — absent means no projector ran', () => { + expect(SaveMetaItemResponseSchema.safeParse(realResponse).success).toBe(true); + const withProjection = SaveMetaItemResponseSchema.parse({ + ...realResponse, + projectionApplied: { success: false, error: 'boom-from-projector' }, + }); + expect(withProjection.projectionApplied).toEqual({ success: false, error: 'boom-from-projector' }); + }); + + it('projectionApplied.success is required once the key is present', () => { + expect( + SaveMetaItemResponseSchema.safeParse({ ...realResponse, projectionApplied: { error: 'x' } }).success, + ).toBe(false); + }); +}); diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index dfb0f5d017..bd3f13a764 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -249,9 +249,60 @@ export const SaveMetaItemRequestSchema = lazySchema(() => z.object({ /** * Save Metadata Item Response + * + * Describes the FULL body `PUT /api/v1/meta/:type/:name` returns, not a subset + * of it (#5745, settled by the #5563 maintainer ruling "补齐 spec 字段"). The + * declaration previously stopped at `{ success, message }`, so a `.parse()` of + * a real response silently STRIPPED `version` / `seq` / `state` — and + * `SaveMetaItemResponse` could not even name them at the type level. `version` + * in particular is the token the ADR-0008 optimistic-concurrency chain already + * runs on (echo it back as `If-Match` on the next write to get a 409 instead of + * a lost update), so leaving it undeclared meant the OCC carrier existed on the + * wire with no contract behind it. + * + * Presence was measured against `origin/main`, not assumed: the sole producer is + * `ObjectStackProtocolImplementation.saveMetaItem`, whose single success return + * is the repository write path, and the REST route hands that object to + * `res.json()` verbatim. That path always sets `version` / `seq` / `state`, so + * the three are REQUIRED here; `projectionApplied` is conditional on an + * ADR-0094 mutation projector being registered for the type, so it alone is + * optional. (A second, receipt-less legacy return used to exist and would have + * forced all three to be optional — it was proved unreachable and deleted in + * #5264 / PR #5782, which is why `required` is safe to state.) */ export const SaveMetaItemResponseSchema = lazySchema(() => z.object({ success: z.boolean(), + version: z.string().describe( + 'Content hash of the just-committed body, and the token the ADR-0008 ' + + 'optimistic-concurrency chain runs on: send it back as the `If-Match` ' + + 'request header on the next write to that item and a concurrent edit is ' + + 'reported as 409 `metadata_conflict` instead of silently overwritten. ' + + 'Opaque to callers — echo it verbatim, never parse it. Currently emitted ' + + 'as `sha256:<64 hex chars>`, but the format is not part of this contract.', + ), + seq: z.number().int().describe( + 'Monotonic sequence number of the metadata event this write appended to ' + + 'the item history (sys_metadata_history.event_seq). Orders writes; unlike ' + + '`version` it is not an OCC token.', + ), + state: z.enum(['draft', 'active']).describe( + 'Lifecycle the body was written into: "draft" when the request asked for ' + + 'draft mode (`?mode=draft`), otherwise "active" (published and live). A ' + + 'draft is staged only — it is not served to the runtime until published.', + ), + projectionApplied: z.object({ + success: z.boolean().describe('False when the projector threw; the metadata write itself still succeeded.'), + error: z.string().optional().describe('Projector failure message, present only when `success` is false.'), + }).optional().describe( + 'Outcome of the awaited ADR-0094 mutation projector — the post-persist step ' + + 'that materializes this metadata into its derived data-plane read model ' + + '(e.g. `permission` → `sys_permission_set`). Present ONLY when a projector ' + + 'is registered for this metadata type, which is why it is optional: its ' + + 'absence means "no projector ran", never "the projection failed". ' + + 'Best-effort by design — a projector failure is reported here and logged, ' + + 'never thrown, so a caller that needs the read model to be live must check ' + + '`projectionApplied.success` rather than rely on the 200.', + ), message: z.string().optional(), }));