From a928e8817e15ac6504f9252ddc5d0192edd67118 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 11:45:18 +0000 Subject: [PATCH] fix(metadata-protocol): write saved overlays through the SchemaRegistry so they are dispatchable immediately (#4521) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A just-saved overlay was listed but not dispatchable for a short window: saveMetaItem only wrote the registry through for `object`, so every other overlay type reached it solely via the READ-side hydration in getMetaItems — the listing call is what repaired the dispatch path. resolveRouteActionDeclaration reads the registry, so `PUT /meta/action/x` followed by `POST /actions//x` answered the ADR-0110 "has no declaration" 404 until someone listed the type. - Extract the read-side hydration rule (ADR-0010 §3.3 protection graft, ADR-0048 package-scoped artifact lookup) into hydrateOverlayIntoRegistry and share it between getMetaItems and the new applyRegistryWriteThrough. - Call the write-through from saveMetaItem (publish mode), runPublishSideEffects (draft promotion), and rollbackMetaItem — for EVERY overlay type, with the same environmentId scoping gate the read carries. - Boundaries pinned by tests: drafts never leak into the live registry, ADR-0110's 404 for a genuinely absent declaration stands, and DELETE still restores the packaged artifact (the overlay is a plain-key shadow). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 --- .../meta-overlay-write-through-dispatch.md | 35 +++ packages/metadata-protocol/src/protocol.ts | 143 ++++++++++--- packages/objectql/src/protocol-meta.test.ts | 27 ++- .../src/meta-overlay-read-your-writes.test.ts | 202 ++++++++++++++++++ 4 files changed, 373 insertions(+), 34 deletions(-) create mode 100644 .changeset/meta-overlay-write-through-dispatch.md create mode 100644 packages/runtime/src/meta-overlay-read-your-writes.test.ts diff --git a/.changeset/meta-overlay-write-through-dispatch.md b/.changeset/meta-overlay-write-through-dispatch.md new file mode 100644 index 0000000000..1560c00aff --- /dev/null +++ b/.changeset/meta-overlay-write-through-dispatch.md @@ -0,0 +1,35 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): a just-saved overlay is dispatchable immediately, not after the next listing (#4521) + +The #4432 F1 verification found that immediately after a successful +`PUT /api/v1/meta/action/`, `GET /api/v1/meta/action` already listed the +overlay while `POST /api/v1/actions//` answered the ADR-0110 +"has no declaration" 404 — and a later POST succeeded. Nothing expired in +between: the *listing* is what repaired it. + +The lagging cache was the engine's `SchemaRegistry`. The runtime dispatch path +(`resolveRouteActionDeclaration`) reads it as the live view of metadata, but +`saveMetaItem` only wrote through it for `object` — every other overlay type +reached the registry solely via the READ-side hydration in `getMetaItems`, so +"has anyone listed this type yet?" silently decided whether a saved action +could be invoked. + +The fix is at the producer, per Prime Directive #12 — no retry, sleep, or +fallback was added at the dispatch site: + +- `saveMetaItem` (publish mode), draft publishing (`runPublishSideEffects`), + and `rollbackMetaItem` now write EVERY overlay type through the registry via + a shared `applyRegistryWriteThrough`, so an item that is listable is + dispatchable in the same breath. +- The write-through and the read-side hydration share one implementation + (`hydrateOverlayIntoRegistry`), including the ADR-0010 §3.3 protection-envelope + graft and the ADR-0048 package-scoped artifact lookup — a read and a write + can no longer leave the registry in two different states for the same row. +- Unchanged boundaries: drafts still never leak into the live registry, the + `environmentId` scoping gate matches the read side, ADR-0110's 404 for a + genuinely absent declaration stands, and DELETE ("reset to artifact default") + still restores the packaged artifact — the overlay is a plain-key shadow, not + an in-place overwrite. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index f564e36adf..26eeb81beb 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -2627,25 +2627,14 @@ export class ObjectStackProtocolImplementation implements // Only hydrate the global registry for unscoped (control-plane) // calls — scoped project entries must not leak process-wide. - // Graft the artifact's protection envelope onto the overlay body - // BEFORE registering: the plain-key entry written here shadows - // the packaged artifact on `registry.getItem`, and a bare - // overlay body would strip `_lock`/`_packageId`/`_provenance` - // from every registry-direct reader (ADR-0010 §3.3 — an overlay - // must never loosen a packaged lock). ADR-0048 (#1828) — scope - // the artifact lookup to the row's OWN package so a colliding - // overlay no longer grafts the first-registered package's - // provenance/lock onto another package's row. + // #4521 — this loop is no longer the ONLY way an overlay reaches + // the registry (the write writes through as well), so it is the + // shared {@link hydrateOverlayIntoRegistry} that both callers + // use: a read and a write that register differently would put + // the registry in two different states for the same row. if (this.environmentId === undefined) { for (const { data, packageId: recPkg } of overlays) { - if (data && typeof data === 'object' && 'name' in data) { - const artifact = this.lookupArtifactItem(request.type, (data as any).name, recPkg); - this.engine.registry.registerItem( - request.type, - mergeArtifactProtection(data, artifact), - 'name' as any, - ); - } + this.hydrateOverlayIntoRegistry(request.type, data, recPkg); } } } @@ -5970,6 +5959,89 @@ export class ObjectStackProtocolImplementation implements } } + /** + * Register ONE active overlay body into the engine's SchemaRegistry. + * + * The single implementation shared by the READ-side hydration + * (`getMetaItems`) and the WRITE-side write-through + * ({@link applyRegistryWriteThrough}) — #4521. Two copies of this rule + * would let a read and a write leave the registry in two different + * states for the same row, which is the class of bug the write-through + * exists to close. + * + * Graft the artifact's protection envelope onto the overlay body BEFORE + * registering: the plain-key entry written here shadows the packaged + * artifact on `registry.getItem`, and a bare overlay body would strip + * `_lock`/`_packageId`/`_provenance` from every registry-direct reader + * (ADR-0010 §3.3 — an overlay must never loosen a packaged lock). + * ADR-0048 (#1828) — scope the artifact lookup to the row's OWN package + * so a colliding overlay no longer grafts the first-registered package's + * provenance/lock onto another package's row. + * + * Returns whether anything was registered (bodies without a `name`, and + * registry doubles without `registerItem`, are no-ops). + */ + private hydrateOverlayIntoRegistry(type: string, data: unknown, packageId?: string | null): boolean { + if (!data || typeof data !== 'object' || !('name' in data)) return false; + const registry: any = (this.engine as any)?.registry; + if (!registry || typeof registry.registerItem !== 'function') return false; + const artifact = this.lookupArtifactItem(type, (data as any).name, packageId ?? undefined); + registry.registerItem(type, mergeArtifactProtection(data, artifact), 'name' as any); + return true; + } + + /** + * [#4521] Write-through the SchemaRegistry after a mutation goes LIVE, so + * a just-saved item is dispatchable — not merely listable. + * + * `resolveRouteActionDeclaration` (and every other runtime consumer that + * reads `engine.registry` directly) treats the registry as the live view + * of metadata. Before this method the write only wrote through it for + * `object` ({@link applyObjectRegistryMutation} returns early otherwise); + * every other overlay type arrived in the registry solely via the + * READ-side hydration in `getMetaItems` / `loadMetaFromDb`. That made a + * *read* the thing that repaired the registry: a `PUT /meta/action/x` + * followed immediately by `POST /actions//x` answered the + * ADR-0110 "has no declaration" 404, and the very next listing call made + * the same POST succeed (#4432 F1, split out as #4521). Read-your-writes + * between the meta list and the dispatch path was decided by whether + * anyone had listed yet. + * + * The fix is at the producer, not the consumer: no retry, no sleep and no + * tolerance was added at the dispatch site, and ADR-0110's 404 for a + * genuinely absent declaration is untouched — an item nobody wrote still + * has nothing in the registry to find. + * + * Call ONLY after the write has landed and is live: + * • `saveMetaItem` repo path — post-`put()`, `mode === 'publish'` only + * (drafts are a staging buffer and must never leak into the runtime); + * • `runPublishSideEffects` — the draft→active promotion; + * • `rollbackMetaItem` — the restored body is the live one. + * + * The non-object branch carries the same `environmentId === undefined` + * gate the read-side hydration carries: a project-scoped row must not be + * registered into a registry that unscoped (control-plane) callers share. + * The write must not be more permissive about that than the read is. + */ + private applyRegistryWriteThrough(request: { type: string; name: string; item?: any; packageId?: string | null }): void { + if (request.type === 'object' || request.type === 'objects') { + this.applyObjectRegistryMutation(request); + return; + } + if (this.environmentId !== undefined) return; + try { + this.hydrateOverlayIntoRegistry(request.type, request.item, request.packageId ?? undefined); + } catch (err: any) { + // Best-effort, exactly like the object branch: the row is already + // persisted, so a registry hiccup must not fail the write that + // succeeded. It degrades to the pre-#4521 behaviour (the next + // listing hydrates it), never to a lost write. + console.warn( + `[Protocol] registry write-through failed for ${request.type}/${request.name}: ${err?.message ?? err}`, + ); + } + } + /** * Heal the in-memory registry after a metadata reset (overlay-row * delete) on control-plane kernels. Two layers: @@ -6493,12 +6565,21 @@ export class ObjectStackProtocolImplementation implements ...(request.packageId !== undefined ? { packageId: request.packageId } : {}), }); // Persistence succeeded — NOW it's safe to mutate the - // in-memory object registry. If put() had thrown, the - // registry would still reflect the prior state. Drafts - // are NOT live: don't propagate them into the runtime - // object registry (would defeat the staging buffer). + // in-memory registry. If put() had thrown, the registry + // would still reflect the prior state. Drafts are NOT + // live: don't propagate them into the runtime registry + // (would defeat the staging buffer). + // #4521 — write through for EVERY overlay type, not just + // `object`: the runtime dispatch path reads this registry, + // so an item that is already listable must be dispatchable + // in the same breath. See {@link applyRegistryWriteThrough}. if (mode === 'publish') { - this.applyObjectRegistryMutation(request); + this.applyRegistryWriteThrough({ + type: singularTypeForRepo, + name: request.name, + item: request.item, + packageId: request.packageId ?? null, + }); await this.ensureObjectStorage(request.type, request.name); } // ADR-0010 — success audit (best-effort). @@ -7147,12 +7228,15 @@ export class ObjectStackProtocolImplementation implements projectionApplied?: MutationProjectionOutcome; } = {}; // Drafts skipped the registry mutation; on publish we now refresh the - // runtime object registry so live behaviour catches up immediately - // (matches saveMetaItem's post-persistence registry update path). - this.applyObjectRegistryMutation({ - type: args.requestType, + // runtime registry so live behaviour catches up immediately (matches + // saveMetaItem's post-persistence registry update path — #4521 makes + // that path cover every overlay type, so promoting a drafted action + // makes it dispatchable at once instead of at the next listing). + this.applyRegistryWriteThrough({ + type: args.singularType, name: args.name, item: args.body, + packageId: args.packageId, }); // Create the object's table now so it's CRUD-able without a restart. await this.ensureObjectStorage(args.requestType, args.name); @@ -8417,8 +8501,11 @@ export class ObjectStackProtocolImplementation implements ...(request.message ? { message: request.message } : {}), intent, }); - this.applyObjectRegistryMutation({ - type: request.type, + // #4521 — a rollback is a live write like any other: the restored + // body must be the one the runtime dispatches on immediately, not + // after someone lists the type. + this.applyRegistryWriteThrough({ + type: singularType, name: request.name, item: result.item.body, }); diff --git a/packages/objectql/src/protocol-meta.test.ts b/packages/objectql/src/protocol-meta.test.ts index 480ea7c92e..3535fb2c02 100644 --- a/packages/objectql/src/protocol-meta.test.ts +++ b/packages/objectql/src/protocol-meta.test.ts @@ -214,15 +214,30 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { ).rejects.toThrow('Item data is required'); }); - it('should NOT mutate the SchemaRegistry for non-object types (ADR-0005)', async () => { - // ADR-0005: sys_metadata is the authoritative overlay store. - // saveMetaItem must not pollute the artifact-loaded registry for - // overlay-eligible types (view/dashboard/etc.) — getMetaItem reads - // sys_metadata first, so the registry stays at the artifact value. + it('writes the saved body through to the SchemaRegistry for non-object types (#4521)', async () => { + // INVERTED from "should NOT mutate the SchemaRegistry for + // non-object types (ADR-0005)". That assertion was written when + // mutating the registry here meant OVERWRITING the artifact in + // place, so `deleteMetaItem` ("reset to artifact default") would + // have had nothing left to restore. That is no longer how the + // registry stores the two: an artifact lives under the composite + // `:` key, an overlay is a plain-key SHADOW, and + // `restoreArtifactRegistryView` drops the shadow on delete. The + // read side (`getMetaItems` hydration) has been writing that + // shadow for a long time. + // + // Leaving the write alone therefore did not protect the artifact — + // it only made a READ the thing that repaired the registry: a + // just-saved overlay was listed but NOT dispatchable until someone + // listed the type, because `resolveRouteActionDeclaration` reads + // the registry (#4432 F1 → #4521). The rest of ADR-0005 is intact: + // `sys_metadata` is still the authoritative store and `getMetaItem` + // still consults it first. await protocol.saveMetaItem({ type: 'app', name: 'test_app', item: sampleApp }); const stored = registry.getItem('app', 'test_app'); - expect(stored).toBeUndefined(); + expect(stored).toBeDefined(); + expect((stored as any).name).toBe('test_app'); }); it('should register `object` type items in SchemaRegistry (engine schema-sync needs it)', async () => { diff --git a/packages/runtime/src/meta-overlay-read-your-writes.test.ts b/packages/runtime/src/meta-overlay-read-your-writes.test.ts new file mode 100644 index 0000000000..f44ed539c0 --- /dev/null +++ b/packages/runtime/src/meta-overlay-read-your-writes.test.ts @@ -0,0 +1,202 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4521 — a just-saved overlay must be DISPATCHABLE, not just listable. + * + * The v17 verification (#4432, F1) reported that immediately after a + * successful `PUT /api/v1/meta/action/`, `GET /api/v1/meta/action` + * already listed the overlay while `POST /api/v1/actions//` + * answered the ADR-0110 "has no declaration" 404 — and a later POST + * succeeded. Nothing expired in between: the *listing* is what repaired it. + * + * The lagging cache is the engine's `SchemaRegistry`. `resolveRouteActionDeclaration` + * reads it as source 2, but the WRITE only wrote through it for `object` + * (`applyObjectRegistryMutation` returns early for every other type). Every + * other overlay type reached the registry solely via the READ-side hydration + * inside `getMetaItems` / `loadMetaFromDb` — so "has this been listed yet?" + * silently decided whether a saved action could be invoked. + * + * These tests drive the exact repro through the real seam: the real + * `ObjectStackProtocolImplementation.saveMetaItem`, the real `SchemaRegistry`, + * and the real `resolveRouteActionDeclaration` — with NO listing call in + * between. They fail if the write-through is removed, and they pin the two + * boundaries the fix must not cross: a genuinely absent declaration still + * resolves to nothing (ADR-0110's 404 stands), and a `draft` save is still + * not live. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SchemaRegistry } from '@objectstack/objectql'; +import { resolveRouteActionDeclaration, type ActionExecutionDeps } from './action-execution.js'; + +/** + * A `sys_metadata`-shaped fake engine. Rows live in a plain array so the + * repository write path (insert/update) and every read see one store — the + * same double #4432's canonicalization test uses. + */ +function makeEngine(registry: SchemaRegistry) { + let rows: any[] = []; + let nextId = 1; + const matches = (r: any, w: Record): boolean => + Object.entries(w).every(([k, v]) => { + if (v === undefined) return true; + if (v !== null && typeof v === 'object') return true; // operator clause — not exercised here + return r[k] === v; + }); + const engine: any = { + registry, + find: vi.fn(async (_table: string, opts: any) => rows.filter((r) => matches(r, opts?.where ?? {}))), + findOne: vi.fn(async (table: string, opts: any) => (await engine.find(table, opts))[0] ?? null), + insert: vi.fn(async (_table: string, data: any) => { + const row = { id: data.id ?? `row_${nextId++}`, ...data }; + rows.push(row); + return row; + }), + update: vi.fn(async (_table: string, data: any, opts: any) => { + const target = rows.find((r) => matches(r, opts?.where ?? {})); + if (target) Object.assign(target, data); + return target ?? null; + }), + delete: vi.fn(async (_table: string, opts: any) => { + const before = rows.length; + rows = rows.filter((r) => !matches(r, opts?.where ?? {})); + return { deleted: before - rows.length }; + }), + count: vi.fn(async (_table: string, opts: any) => rows.filter((r) => matches(r, opts?.where ?? {})).length), + aggregate: vi.fn(async () => []), + execute: vi.fn(async () => undefined), + getRows: () => rows, + }; + return engine; +} + +/** The routed object the showcase repro dispatched against. */ +const OBJECT_DEF = { name: 'showcase_task', label: 'Task', fields: {}, actions: [] }; + +describe('#4521 — read-your-writes between saveMeta and the dispatch path', () => { + let registry: SchemaRegistry; + let engine: any; + let protocol: ObjectStackProtocolImplementation; + let ql: any; + let deps: ActionExecutionDeps; + + beforeEach(() => { + registry = new SchemaRegistry({ multiTenant: false }); + registry.registerObject(OBJECT_DEF as any, 'showcase'); + engine = makeEngine(registry); + protocol = new ObjectStackProtocolImplementation(engine); + ql = { + registry, + getSchema: (name: string) => (name === OBJECT_DEF.name ? OBJECT_DEF : undefined), + }; + // No metadata service at all: the declaration must be resolvable from + // what the WRITE left behind, not from a second store that happens to + // be reachable. `resolveService` answering `undefined` is the ordinary + // "no metadata plane bound" case, not a degraded one. + deps = { + resolveService: (async () => undefined) as any, + getObjectQL: async () => ql, + } as ActionExecutionDeps; + }); + + const saveAction = (item: any, mode?: 'draft' | 'publish') => + protocol.saveMetaItem({ + type: 'action', + name: item.name, + item, + ...(mode ? { mode } : {}), + }); + + const resolve = (actionName: string) => + resolveRouteActionDeclaration(deps, { + ql, + objectName: OBJECT_DEF.name, + actionName, + }); + + it('dispatches an overlay saved in the SAME request sequence — no listing call in between', async () => { + // The exact #4432 F1 repro: PUT, then POST. Nothing reads the list. + const saved = await saveAction({ + name: 'rc1_probe', + label: 'Probe', + objectName: 'showcase_task', + type: 'script', + target: 'showcase.probe', + }); + expect(saved.success).toBe(true); + + const { action, degraded } = await resolve('rc1_probe'); + + // Pre-fix this is `undefined` → the route answers the ADR-0110 + // "has no declaration" 404 for a just-saved, already-listed overlay. + expect(action).toBeDefined(); + expect(action?.name).toBe('rc1_probe'); + expect(action?.type).toBe('script'); + // A miss and an outage are different facts (ADR-0110 D3): this is a hit. + expect(degraded).toBeFalsy(); + }); + + it('object-less overlays are dispatchable immediately too', async () => { + await saveAction({ name: 'global_probe', label: 'Global', type: 'script', target: 'showcase.probe' }); + const { action } = await resolve('global_probe'); + expect(action?.name).toBe('global_probe'); + }); + + it('listing FIRST is not what makes it dispatchable (the pre-fix crutch is gone)', async () => { + // Revert-proof: pre-fix, this test passed ONLY because `getMetaItems` + // hydrated the registry. Assert the write already did it — the list + // read must be an observation, not a repair. + await saveAction({ name: 'order_probe', label: 'Order', objectName: 'showcase_task', type: 'script', target: 'showcase.probe' }); + const hydratedByWrite = registry.getItem('action', 'order_probe'); + expect(hydratedByWrite).toBeDefined(); + expect((hydratedByWrite as any)?.label).toBe('Order'); + }); + + it('a second save is visible immediately (the overlay UPDATE path, not just insert)', async () => { + await saveAction({ name: 'rc1_probe', label: 'V1', objectName: 'showcase_task', type: 'script', target: 'showcase.probe' }); + await saveAction({ name: 'rc1_probe', label: 'V2', objectName: 'showcase_task', type: 'flow', target: 'wf' }); + const { action } = await resolve('rc1_probe'); + expect(action?.label).toBe('V2'); + // #3915 — the declared TYPE is what the route dispatches on, so a + // stale registry entry would send the call to the wrong executor. + expect(action?.type).toBe('flow'); + }); + + it('a name that was never declared still resolves to NOTHING (ADR-0110 404 preserved)', async () => { + await saveAction({ name: 'rc1_probe', label: 'Probe', objectName: 'showcase_task', type: 'script', target: 'showcase.probe' }); + const { action, degraded } = await resolve('never_declared'); + expect(action).toBeUndefined(); + expect(degraded).toBeFalsy(); + }); + + it('the write-through SHADOWS the packaged artifact — DELETE still restores it', async () => { + // The hazard the pre-#4521 "saveMetaItem must not touch the registry" + // rule was guarding: if the write overwrote the artifact IN PLACE, + // `deleteMetaItem` ("reset to artifact default") would have nothing to + // restore. It does not — the artifact lives under the composite + // `:` key and the overlay is a plain-key shadow, which + // `restoreArtifactRegistryView` removes on delete. Pinned here because + // the write-through is what makes that separation load-bearing. + registry.registerItem('action', { name: 'shipped_probe', label: 'Shipped', type: 'script', target: 'showcase.shipped' }, 'name', 'showcase'); + expect((await resolve('shipped_probe')).action?.label).toBe('Shipped'); + + await saveAction({ name: 'shipped_probe', label: 'Customized', objectName: 'showcase_task', type: 'script', target: 'showcase.probe' }); + expect((await resolve('shipped_probe')).action?.label).toBe('Customized'); + + await protocol.deleteMetaItem({ type: 'action', name: 'shipped_probe' }); + expect((await resolve('shipped_probe')).action?.label).toBe('Shipped'); + }); + + it('a DRAFT save is not dispatchable — drafts never leak into the live registry', async () => { + const saved = await saveAction( + { name: 'draft_probe', label: 'Draft', objectName: 'showcase_task', type: 'script', target: 'showcase.probe' }, + 'draft', + ); + expect(saved.success).toBe(true); + expect(saved.state).toBe('draft'); + const { action } = await resolve('draft_probe'); + expect(action).toBeUndefined(); + expect(registry.getItem('action', 'draft_probe')).toBeUndefined(); + }); +});