diff --git a/.changeset/20862-automation-doors-persist.md b/.changeset/20862-automation-doors-persist.md new file mode 100644 index 00000000000..65325839616 --- /dev/null +++ b/.changeset/20862-automation-doors-persist.md @@ -0,0 +1,25 @@ +--- +'@objectstack/runtime': minor +--- + +fix(runtime)!: the /automation create and update doors save the flow as a tenant row, so what they answer 200 for survives a restart, and the removal door deletes that row too (#20862) + +Clause-②: no (narrowing) + + + +**BREAKING**: shipped as `minor` under the launch-window convention. `POST /api/v1/automation` and `PUT /api/v1/automation/:name` now refuse a definition the metadata store refuses, which they used to register and answer `200` for. `DELETE /api/v1/automation/:name` now relays a store's refusal to delete the flow's row. + +**What changed.** The create and update doors registered a flow in the automation engine and wrote no metadata row. The next boot binds flows from the stored metadata, so a flow created through `POST /automation` was gone after a restart, and an update through `PUT /automation/:name` to a flow stored through `/meta` lost to the stored definition. Both doors now save the definition through the metadata protocol's own `saveMetaItem`: the save `PUT /api/v1/meta/flow/:name` uses, and the one the clone door (`POST /automation/:name/clone`) already used. The row is env-wide and live (`active`), so the flow reads back on `/meta` and survives a cold boot. The three doors share one path. + +- **Engine first, store second.** The engine's registration is still the first check. Its refusal is answered as before (`400 VALIDATION_FAILED`), and nothing is saved. +- **A save the store refuses is relayed with its own code and status, and leaves no registration behind.** A create is withdrawn from the engine. An update puts back the definition the engine held, so a refused update does not take the flow down. +- **`DELETE /automation/:name` deletes the tenant row too**, through `deleteMetaItem`, so a flow created through the door does not come back at the next boot. The engine's own removal refusal (`DELETE_RESTRICTED` / `409`) is still raised before the store is touched. A name with no stored row is removed as before. A delete the store refuses puts the definition back and relays the refusal. +- **Unchanged:** the locked-base refusal on a packaged flow's name (`403 NOT_OVERRIDABLE`) and the refusal of a definition claiming a package's provenance (`422 INVALID_METADATA`) still answer first. A composition with no metadata protocol keeps the engine-only registration and removal it always had. + +**Newly refused, because the metadata store refuses them** (measured on the showcase): + +- A flow name with a leading underscore. `FlowSchema` admits it and the metadata item-name grammar does not, so the door answers `400 INVALID_REQUEST`. Rename the flow to a name that starts with a letter. +- A definition a gating runtime publish rule refuses, such as a default edge that also carries a condition (`flow-default-edge-with-condition`). The door answers `422 INVALID_METADATA` with the rule's finding. Fix the definition as the finding says. + +Such a flow could never be stored, so before this change it ran only until the next restart. diff --git a/packages/qa/dogfood/test/automation-authoring-doors-durable.dogfood.test.ts b/packages/qa/dogfood/test/automation-authoring-doors-durable.dogfood.test.ts new file mode 100644 index 00000000000..e7d2b82ce5c --- /dev/null +++ b/packages/qa/dogfood/test/automation-authoring-doors-durable.dogfood.test.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#20862, ADR-0126 §2, ADR-0131 D6] The `/automation` definition doors keep +// what they answer `200` for — over the real showcase composition, across a +// cold boot on one database file. +// +// ## What was broken +// +// `POST /automation` and `PUT /automation/:name` registered the definition in +// the engine and wrote no metadata row. The next boot binds flows from the +// stored metadata, so a created flow was gone after a restart, and an update +// to a flow stored through `/meta` was overwritten by the stored definition. +// Measured on `origin/main` before the fix, through this file's own cases: +// the created flow answered `404` after the cold boot, and the updated flow +// served its stored label again. +// +// ## Why a booted stack +// +// The door-level pins sit beside the domain +// (`packages/runtime/src/domains/automation-authoring-doors-durable.test.ts`). +// What they cannot answer is whether, on the stack an operator runs, the door +// reaches a protocol whose save lands a row the next boot binds. Only a +// restart on the same file shows that. +// +// ## What each case pins +// +// - a created flow reads back on the metadata door and survives a cold boot; +// - an update to a `/meta`-stored flow reads back there and survives it; +// - a save the store refuses leaves no engine registration: a create is +// withdrawn, and an update puts back the definition the engine held; +// - a flow created and then removed through the door stays removed after the +// cold boot (the removal door keeps pace with the create door); +// - a packaged flow's name is still refused as a locked base (the control). + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { ConnectorRestPlugin } from '@objectstack/connector-rest'; +import { ConnectorOpenApiPlugin } from '@objectstack/connector-openapi'; +import { ConnectorMcpPlugin } from '@objectstack/connector-mcp'; +import { fileURLToPath } from 'node:url'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +/** Package-relative connector refs resolve against the cwd — see the sibling boots. */ +const SHOWCASE_DIR = fileURLToPath(new URL('../../../../examples/app-showcase/', import.meta.url)); + +/** A flow the showcase package ships (`com.example.showcase`). */ +const SHIPPED = 'showcase_urgent_task_alert'; +/** Names no package ships, each used by one case. */ +const CREATED = 'dogfood_created_flow_20862'; +const META_STORED = 'dogfood_meta_stored_flow_20862'; +const REMOVED = 'dogfood_removed_flow_20862'; +/** + * A name the engine registers and the store refuses: `FlowSchema.name` admits a + * leading underscore, the metadata item-name grammar does not. So the engine's + * registration succeeds and the save fails — a real store refusal, reached + * through the public door. + */ +const UNSAVABLE = '_dogfood_unsavable_flow_20862'; + +const UPDATED_LABEL = 'Updated through the automation door'; + +const definition = (name: string, label = `Dogfood ${name}`) => ({ + name, + label, + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: {} }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], +}); + +/** + * A definition the engine registers and the store's publish gate refuses: a + * default edge carrying a condition (`flow-default-edge-with-condition`, a + * gating authoring rule the engine does not run). + */ +const gateRefused = (name: string) => ({ + ...definition(name, 'Refused by the publish gate'), + edges: [{ id: 'e1', source: 'start', target: 'end', isDefault: true, condition: 'true' }], +}); + +interface DispatcherEnvelope { + success?: boolean; + data?: Record; + error?: { code?: string; message?: string }; +} +const dataOf = (json: unknown) => (json as DispatcherEnvelope).data; +const dispatcherCode = (json: unknown) => (json as DispatcherEnvelope).error?.code; + +const plugins = () => [ + new ConnectorRestPlugin(), + new ConnectorOpenApiPlugin(), + new ConnectorMcpPlugin({ declarativeStdio: ['node'] }), +]; + +async function boot(databaseFile: string): Promise { + return bootStack(showcaseStack, { automation: true, databaseFile, extraPlugins: plugins() }); +} + +describe('the /automation definition doors persist what they register (showcase, cold boot)', () => { + let stack: VerifyStack; + let token: string; + let prevCwd: string; + let dir: string; + let dbFile: string; + + const call = async (method: string, path: string, body?: unknown) => { + const res = await stack.apiAs(token, method, path, body); + const json: unknown = await res.json().catch(() => ({})); + return { status: res.status, json }; + }; + /** `GET /meta/flow/:name` — the served document, unwrapped from whichever envelope carries it. */ + const metaRead = async (name: string) => { + const read = await call('GET', `/meta/flow/${name}`); + const json = read.json as Record; + const doc = (json?.data ?? json?.item ?? json) as Record; + return { status: read.status, doc: (doc?.item ?? doc) as Record }; + }; + const engineLabel = async (name: string) => { + const read = await call('GET', `/automation/${name}`); + return { status: read.status, label: dataOf(read.json)?.label }; + }; + + beforeAll(async () => { + prevCwd = process.cwd(); + process.chdir(SHOWCASE_DIR); + dir = mkdtempSync(join(tmpdir(), 'dogfood-20862-')); + dbFile = join(dir, 'showcase.db'); + stack = await boot(dbFile); + token = await stack.signIn(); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + if (prevCwd) process.chdir(prevCwd); + if (dir) rmSync(dir, { recursive: true, force: true }); + }); + + it('a flow created through the automation door reads back on the metadata door', async () => { + const created = await call('POST', '/automation', definition(CREATED)); + expect(created.status, JSON.stringify(created.json)).toBe(200); + + const served = await metaRead(CREATED); + expect(served.status, JSON.stringify(served.doc)).toBe(200); + expect(served.doc.label).toBe(`Dogfood ${CREATED}`); + }); + + it('an update through the automation door to a /meta-stored flow reads back on the metadata door', async () => { + const stored = await call('PUT', `/meta/flow/${META_STORED}`, definition(META_STORED, 'Stored through /meta')); + expect(stored.status, JSON.stringify(stored.json)).toBe(200); + + const updated = await call('PUT', `/automation/${META_STORED}`, definition(META_STORED, UPDATED_LABEL)); + expect(updated.status, JSON.stringify(updated.json)).toBe(200); + + expect((await metaRead(META_STORED)).doc.label).toBe(UPDATED_LABEL); + expect((await engineLabel(META_STORED)).label).toBe(UPDATED_LABEL); + }); + + it('a create whose save the store refuses answers the store\'s refusal and leaves no engine registration', async () => { + const created = await call('POST', '/automation', definition(UNSAVABLE)); + + expect(created.status, JSON.stringify(created.json)).toBe(400); + expect(dispatcherCode(created.json)).toBe('INVALID_REQUEST'); + expect((await call('GET', `/automation/${UNSAVABLE}`)).status).toBe(404); + }); + + it('an update whose save the store refuses answers the refusal and the engine keeps the definition it held', async () => { + const updated = await call('PUT', `/automation/${META_STORED}`, gateRefused(META_STORED)); + + expect(updated.status, JSON.stringify(updated.json)).toBe(422); + expect(dispatcherCode(updated.json)).toBe('INVALID_METADATA'); + expect(await engineLabel(META_STORED)).toEqual({ status: 200, label: UPDATED_LABEL }); + expect((await metaRead(META_STORED)).doc.label).toBe(UPDATED_LABEL); + }); + + it('a flow created and then removed through the automation door is gone from the metadata door too', async () => { + const created = await call('POST', '/automation', definition(REMOVED)); + expect(created.status, JSON.stringify(created.json)).toBe(200); + + const removed = await call('DELETE', `/automation/${REMOVED}`); + expect(removed.status, JSON.stringify(removed.json)).toBe(200); + + expect((await call('GET', `/automation/${REMOVED}`)).status).toBe(404); + expect((await metaRead(REMOVED)).status).toBe(404); + }); + + it('control: a packaged flow\'s name is still refused as a locked base, on the update door and on a create onto it', async () => { + const before = dataOf((await call('GET', `/automation/${SHIPPED}`)).json)!; + + const put = await call('PUT', `/automation/${SHIPPED}`, { ...before, label: 'Edited in place' }); + expect(put.status, JSON.stringify(put.json)).toBe(403); + expect(dispatcherCode(put.json)).toBe('NOT_OVERRIDABLE'); + + const post = await call('POST', '/automation', { ...before, label: 'Overwritten via create' }); + expect(post.status, JSON.stringify(post.json)).toBe(403); + expect(dispatcherCode(post.json)).toBe('NOT_OVERRIDABLE'); + + expect(dataOf((await call('GET', `/automation/${SHIPPED}`)).json)).toEqual(before); + }); + + it('after a cold boot on the same database file, every door\'s outcome is what it answered', async () => { + await stack.stop(); + stack = await boot(dbFile); + token = await stack.signIn(); + + // Independent facts about one restart, so each is reported on its own. + // The created flow survives. + expect.soft(await engineLabel(CREATED)).toEqual({ status: 200, label: `Dogfood ${CREATED}` }); + // The update survives: the stored definition no longer wins over it. + expect.soft(await engineLabel(META_STORED)).toEqual({ status: 200, label: UPDATED_LABEL }); + // The refused create left nothing to bind, and the removed flow stays removed. + expect.soft((await call('GET', `/automation/${UNSAVABLE}`)).status).toBe(404); + expect.soft((await call('GET', `/automation/${REMOVED}`)).status).toBe(404); + // The packaged flow is the one its package ships. + expect.soft((await engineLabel(SHIPPED)).status).toBe(200); + }, 180_000); +}); diff --git a/packages/runtime/src/domains/automation-authoring-doors-durable.test.ts b/packages/runtime/src/domains/automation-authoring-doors-durable.test.ts new file mode 100644 index 00000000000..56a45f5b7ae --- /dev/null +++ b/packages/runtime/src/domains/automation-authoring-doors-durable.test.ts @@ -0,0 +1,241 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20862, ADR-0126 §2, ADR-0131 D6] The `/automation` definition doors persist + * what they register — through the ONE path the clone door uses + * (`registerAndSaveFlow`): the engine registers the definition, then the + * metadata protocol's own `saveMetaItem` saves it as a tenant row, env-wide. + * + * ## What this pins, at door level + * + * - `POST /` and `PUT /:name` register the definition AND save that same + * definition through the protocol — type `flow`, no organization, no + * package, the default (live) save mode — engine first, store second; + * - a save that fails relays the store's own refusal and leaves no + * registration of its own: a create is withdrawn, and an update puts back + * the definition the engine held (it does not take the flow down with it); + * - an engine refusal is answered as before and nothing is saved; + * - the refusals in front stay in front: a packaged name is refused as a + * locked base and a definition claiming a package is refused, and neither + * reaches the engine or the store; + * - `DELETE /:name` unregisters AND deletes the tenant row, so a flow the + * create door saved does not come back at the next boot; a delete that + * fails puts the definition back, and the engine's own removal refusal is + * raised before the store is touched; + * - a composition with no metadata protocol keeps the engine-only + * registration and removal it always had — there is no store to persist + * into (the clone door's posture). + * + * Harness as in `automation-tenant-authored-write.test.ts`: a REAL protocol + * over a REAL `SchemaRegistry` answers every verdict, with the packaged flow + * registered the way an artifact loader registers it; the automation service + * is a spy, and the store behind the protocol's save and delete is spied — + * whether the saved row survives a cold boot is the showcase pin's subject + * (`packages/qa/dogfood/test/automation-authoring-doors-durable.dogfood.test.ts`). + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SchemaRegistry } from '@objectstack/objectql'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import type { HttpProtocolContext } from '../http-dispatcher.js'; + +const PACKAGED = 'pkg_alert_flow'; +const PACKAGE_ID = 'com.example.pkg'; +const CUSTOMER = 'customer_flow'; +const NEW = 'new_flow'; + +const definitionOf = (name: string, label = 'Original') => + ({ name, label, type: 'autolaunched', nodes: [], edges: [] }); + +const AUTHOR = (): HttpProtocolContext => ({ + request: {}, + executionContext: { userId: 'u_admin', systemPermissions: ['manage_metadata'] }, +} as HttpProtocolContext); + +/** A refusal as the store raises one — an ADR-0112 envelope the door must relay as is. */ +const storeRefusal = () => Object.assign(new Error('flow/x failed spec validation'), { code: 'INVALID_METADATA', status: 422 }); + +function boot({ protocol: mode = 'real' }: { protocol?: 'real' | 'absent' } = {}) { + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + registry.registerItem('flow', definitionOf(PACKAGED), 'name', PACKAGE_ID); + + const flows = new Map([ + [PACKAGED, definitionOf(PACKAGED)], + [CUSTOMER, definitionOf(CUSTOMER)], + ]); + const registerFlow = vi.fn((name: string, definition: unknown) => { + flows.set(name, definition); + return definition; + }); + const unregisterFlow = vi.fn((name: string) => { flows.delete(name); }); + const getFlow = vi.fn(async (name: string) => flows.get(name) ?? null); + + const protocol = new ObjectStackProtocolImplementation( + { registry, findOne: async () => null } as never, + () => new Map(), + undefined, + ); + const saveMetaItem = vi.spyOn(protocol, 'saveMetaItem').mockResolvedValue({ success: true } as never); + const deleteMetaItem = vi.spyOn(protocol, 'deleteMetaItem').mockResolvedValue({ success: true } as never); + + const services: Record = { + automation: { handlerReady: true, registerFlow, unregisterFlow, getFlow }, + }; + if (mode === 'real') services.protocol = protocol; + const resolve = (name: string): unknown => services[name]; + const kernel = { getService: resolve, getServiceAsync: async (name: string) => resolve(name), context: { getService: resolve } }; + + return { + dispatcher: new HttpDispatcher(kernel as never), + registerFlow, unregisterFlow, saveMetaItem, deleteMetaItem, + held: (name: string) => flows.get(name), + }; +} + +const statusOf = (response: any): unknown => response?.status; +const errorOf = (response: any): { code?: unknown } => response?.body?.error ?? {}; +const dataOf = (response: any): any => response?.body?.data; + +describe('[#20862] the create and update doors save what they register', () => { + it('POST / registers the definition and saves the same definition through the protocol, env-wide, engine first', async () => { + const h = boot(); + const body = definitionOf(NEW, 'Created'); + const { response } = await h.dispatcher.handleAutomation('', 'POST', body, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect(dataOf(response)?.name).toBe(NEW); + expect(h.registerFlow).toHaveBeenCalledWith(NEW, body); + expect(h.saveMetaItem).toHaveBeenCalledTimes(1); + expect(h.saveMetaItem).toHaveBeenCalledWith({ type: 'flow', name: NEW, item: body }); + expect(h.registerFlow.mock.invocationCallOrder[0]).toBeLessThan(h.saveMetaItem.mock.invocationCallOrder[0]!); + }); + + it('PUT /:name registers the full definition and saves it, the `{ definition }` envelope unwrapped', async () => { + const h = boot(); + const definition = definitionOf(CUSTOMER, 'Updated'); + const { response } = await h.dispatcher.handleAutomation(`/${CUSTOMER}`, 'PUT', { definition }, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect(h.registerFlow).toHaveBeenCalledWith(CUSTOMER, definition); + expect(h.saveMetaItem).toHaveBeenCalledWith({ type: 'flow', name: CUSTOMER, item: definition }); + expect(h.held(CUSTOMER)).toEqual(definition); + }); + + it('a create whose save fails relays the store\'s refusal and is withdrawn from the engine', async () => { + const h = boot(); + h.saveMetaItem.mockRejectedValueOnce(storeRefusal()); + + const { response } = await h.dispatcher.handleAutomation('', 'POST', definitionOf(NEW), AUTHOR(), undefined); + + expect(statusOf(response)).toBe(422); + expect(errorOf(response).code).toBe('INVALID_METADATA'); + expect(h.unregisterFlow).toHaveBeenCalledWith(NEW); + expect(h.held(NEW)).toBeUndefined(); + }); + + it('an update whose save fails relays the refusal and the engine gets back the definition it held', async () => { + const h = boot(); + const before = h.held(CUSTOMER); + h.saveMetaItem.mockRejectedValueOnce(storeRefusal()); + + const { response } = await h.dispatcher.handleAutomation( + `/${CUSTOMER}`, 'PUT', definitionOf(CUSTOMER, 'Not kept'), AUTHOR(), undefined, + ); + + expect(statusOf(response)).toBe(422); + expect(errorOf(response).code).toBe('INVALID_METADATA'); + // Put back, not withdrawn: a refused update must not take the flow down. + expect(h.unregisterFlow).not.toHaveBeenCalled(); + expect(h.registerFlow).toHaveBeenLastCalledWith(CUSTOMER, before); + expect(h.held(CUSTOMER)).toBe(before); + }); + + it('an engine refusal is answered as before, and nothing is saved', async () => { + const h = boot(); + h.registerFlow.mockImplementationOnce(() => { throw new Error("node 'n' (notify): unknown config key `x`"); }); + + const { response } = await h.dispatcher.handleAutomation('', 'POST', definitionOf(NEW), AUTHOR(), undefined); + + expect(statusOf(response)).toBe(400); + expect(errorOf(response).code).toBe('VALIDATION_FAILED'); + expect(h.saveMetaItem).not.toHaveBeenCalled(); + }); + + it('the refusals in front stay in front: a packaged name and a claimed package reach neither the engine nor the store', async () => { + const h = boot(); + + const locked = await h.dispatcher.handleAutomation(`/${PACKAGED}`, 'PUT', definitionOf(PACKAGED, 'Edited'), AUTHOR(), undefined); + expect(statusOf(locked.response)).toBe(403); + expect(errorOf(locked.response).code).toBe('NOT_OVERRIDABLE'); + + const claimed = await h.dispatcher.handleAutomation( + '', 'POST', { ...definitionOf(NEW), _packageId: PACKAGE_ID, _provenance: 'package' }, AUTHOR(), undefined, + ); + expect(statusOf(claimed.response)).toBe(422); + expect(errorOf(claimed.response).code).toBe('INVALID_METADATA'); + + expect(h.registerFlow).not.toHaveBeenCalled(); + expect(h.saveMetaItem).not.toHaveBeenCalled(); + }); + + it('no metadata protocol in the composition: the definition is registered in the engine only, as before', async () => { + const h = boot({ protocol: 'absent' }); + + const { response } = await h.dispatcher.handleAutomation('', 'POST', definitionOf(NEW), AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect(h.held(NEW)).toEqual(definitionOf(NEW)); + expect(h.saveMetaItem).not.toHaveBeenCalled(); + }); +}); + +describe('[#20862] the removal door keeps pace: DELETE removes the tenant row too', () => { + it('DELETE /:name unregisters the flow, then deletes its row through the protocol, env-wide', async () => { + const h = boot(); + + const { response } = await h.dispatcher.handleAutomation(`/${CUSTOMER}`, 'DELETE', undefined, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect(dataOf(response)).toEqual({ name: CUSTOMER, deleted: true }); + expect(h.unregisterFlow).toHaveBeenCalledWith(CUSTOMER); + expect(h.deleteMetaItem).toHaveBeenCalledWith({ type: 'flow', name: CUSTOMER }); + expect(h.unregisterFlow.mock.invocationCallOrder[0]).toBeLessThan(h.deleteMetaItem.mock.invocationCallOrder[0]!); + }); + + it('a delete that fails relays the refusal and the engine gets the definition back', async () => { + const h = boot(); + const before = h.held(CUSTOMER); + h.deleteMetaItem.mockRejectedValueOnce(Object.assign(new Error('locked'), { code: 'ITEM_LOCKED', status: 403 })); + + const { response } = await h.dispatcher.handleAutomation(`/${CUSTOMER}`, 'DELETE', undefined, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(403); + expect(errorOf(response).code).toBe('ITEM_LOCKED'); + expect(h.registerFlow).toHaveBeenCalledWith(CUSTOMER, before); + expect(h.held(CUSTOMER)).toBe(before); + }); + + it('the engine\'s own removal refusal is raised before the store is touched', async () => { + const h = boot(); + h.unregisterFlow.mockImplementationOnce(() => { + throw Object.assign(new Error('a packaged caller still reaches this subflow'), { code: 'DELETE_RESTRICTED', status: 409 }); + }); + + await expect( + h.dispatcher.handleAutomation(`/${CUSTOMER}`, 'DELETE', undefined, AUTHOR(), undefined), + ).rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409 }); + expect(h.deleteMetaItem).not.toHaveBeenCalled(); + }); + + it('no metadata protocol in the composition: the flow is removed from the engine only, as before', async () => { + const h = boot({ protocol: 'absent' }); + + const { response } = await h.dispatcher.handleAutomation(`/${CUSTOMER}`, 'DELETE', undefined, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect(h.held(CUSTOMER)).toBeUndefined(); + expect(h.deleteMetaItem).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/runtime/src/domains/automation-packaged-base-lock.test.ts b/packages/runtime/src/domains/automation-packaged-base-lock.test.ts index d78d45047f7..65aeffe7f20 100644 --- a/packages/runtime/src/domains/automation-packaged-base-lock.test.ts +++ b/packages/runtime/src/domains/automation-packaged-base-lock.test.ts @@ -89,6 +89,14 @@ interface Harness { toggleFlow: ReturnType; /** The definition the ENGINE holds — read from the store, never a response. */ held: (name: string) => unknown; + /** + * [#20862] Stand in the store behind the protocol's save and delete. The + * definition doors now save through this protocol (and the removal door + * deletes through it), and this harness's engine has no store; what the + * store does is pinned in `automation-authoring-doors-durable.test.ts`, so + * a case that reaches it here stands it in, as the clone case below does. + */ + standInStore: () => void; } /** @@ -138,6 +146,10 @@ function boot( protocol, registerFlow, unregisterFlow, toggleFlow, held: (name: string) => flows.get(name), + standInStore: () => { + vi.spyOn(protocol, 'saveMetaItem').mockResolvedValue({ success: true } as never); + vi.spyOn(protocol, 'deleteMetaItem').mockResolvedValue({ success: true } as never); + }, }; } @@ -235,6 +247,7 @@ describe('a PACKAGED flow — the base is locked at both /automation definition expect(h.held(PACKAGED)).toBe(before); // Control: a name no code package ships is created exactly as before. + h.standInStore(); const created = await h.dispatcher.handleAutomation( '', 'POST', definitionOf('brand_new_flow', 'New'), AUTHOR(), undefined, ); @@ -287,6 +300,7 @@ describe('a PACKAGED flow — the base is locked at both /automation definition describe('what the lock leaves open', () => { it('the customer\'s own flow (no code package ships it) is updated and removed as before', async () => { const h = boot(); + h.standInStore(); const put = await h.dispatcher.handleAutomation( `/${CUSTOMER}`, 'PUT', definitionOf(CUSTOMER, 'Edited'), AUTHOR(), undefined, @@ -301,6 +315,7 @@ describe('what the lock leaves open', () => { it('a registry item with no package provenance, and a tenant-authored one bound to a package, are not locked', async () => { const h = boot({ environmentId: 'env_1' }); + h.standInStore(); for (const name of [RUNTIME_ROW, TENANT_BOUND]) { const put = await h.dispatcher.handleAutomation( `/${name}`, 'PUT', definitionOf(name, 'Edited'), AUTHOR(), undefined, @@ -341,6 +356,7 @@ describe('what the lock leaves open', () => { process.env.OS_METADATA_WRITABLE = 'flow'; ObjectStackProtocolImplementation.resetEnvWritableCache(); const h = boot(); + h.standInStore(); const { response } = await h.dispatcher.handleAutomation( `/${PACKAGED}`, 'PUT', definitionOf(PACKAGED, 'Operator edit'), AUTHOR(), undefined, diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index e3260fd3b49..264e10f6e89 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -1550,6 +1550,137 @@ async function refuseUnauthoredFlowWrite( return { handled: true, response: deps.errorFromThrown(refusal) }; } +/** The engine methods {@link registerAndSaveFlow} drives. */ +type FlowRegistrationService = + Required> & Pick; + +/** + * Put the engine back where it was before this request touched `name`: the + * definition it held re-registered, or — when it held none — the name + * withdrawn. The one undo every definition door here shares, so a write the + * store refused leaves no registration of its own behind, and a refused update + * does not take the flow it was updating down with it. + */ +function restoreFlowRegistration(automationService: FlowRegistrationService, name: string, held: unknown): void { + if (held) automationService.registerFlow(name, held); + else automationService.unregisterFlow?.(name); +} + +/** + * [#20862, #20761, ADR-0126 §2, ADR-0131 D6] THE ONE PERSISTENCE PATH for this + * domain's definition writes — `POST /`, `PUT /:name` and the copy + * `POST /:name/clone` writes: register the definition in the engine, then save + * it as a tenant row through the metadata protocol's own `saveMetaItem`. + * + * Registered in the engine alone, a definition was a process-local fact: the + * next boot binds flows from the stored metadata, so a flow created here was + * gone after a restart, and an update to a flow stored through `/meta` was + * overwritten by the stored definition — a `200` for a change the platform + * did not keep, while the same act through `/meta` was durable. + * + * - **One save, the metadata door's.** `saveMetaItem` env-wide — no + * organization (a flow has no per-org channel) and no package — so the row + * is an ordinary tenant row the boot binds, `/meta` reads back, and the + * same authoring rule and publish gates judge. No second persistence path. + * The default save mode is the live one (`active`), which is what the + * engine registration already is: a draft row would leave the engine + * running a flow the next boot does not bind. + * - **Engine first, store second.** The engine's registration is the + * stricter validation (strict parse, cycles, regions, node config keys, + * predicates), so a definition it refuses is never stored; that refusal is + * answered as a 400 by {@link flowDefinitionRefusal}. + * - **A save that fails undoes the registration** and relays the store's own + * failure, so no answer reports a write that will not survive: a new name + * is withdrawn, and a name the engine already held gets that definition + * back ({@link restoreFlowRegistration}). + * - **The protocol is resolved before anything is registered**, loudly, so a + * slot that fails to resolve cannot leave a registered-but-unsaved flow. + * A composition with no metadata store to save into keeps the engine-only + * registration it always had — there is nothing to persist into. + * + * What is saved is the definition as the caller's door built it — never the + * engine's parsed copy, whose materialized schema defaults would freeze on + * disk (the save canonicalizes it the way it does every `/meta` flow write). + */ +async function registerAndSaveFlow( + deps: DomainHandlerDeps, + context: HttpProtocolContext, + automationService: FlowRegistrationService, + name: string, + definition: unknown, +): Promise<{ registered: unknown } | { refused: HttpDispatcherResult }> { + const store: Partial> | undefined = + await deps.resolveServiceOrLoud(context, 'protocol'); + const held = typeof automationService.getFlow === 'function' ? await automationService.getFlow(name) : null; + let registered: unknown; + try { + registered = automationService.registerFlow(name, definition); + } catch (e) { + return { + refused: { handled: true, response: deps.errorFromThrown(flowDefinitionRefusal(e), VALIDATION_FAILED_STATUS) }, + }; + } + if (typeof store?.saveMetaItem === 'function') { + try { + await store.saveMetaItem({ type: FLOW_METADATA_TYPE, name, item: definition }); + } catch (e) { + restoreFlowRegistration(automationService, name, held); + return { refused: { handled: true, response: deps.errorFromThrown(e) } }; + } + } + return { registered }; +} + +/** The engine methods {@link unregisterAndDeleteFlow} drives. */ +type FlowRemovalService = + Required> & Pick; + +/** + * [#20862] The removal half of {@link registerAndSaveFlow}, for + * `DELETE /:name`: unregister the flow in the engine, then delete its tenant + * row through the metadata protocol's own `deleteMetaItem`, env-wide. + * + * Needed because the create door now saves: a flow created and then removed + * here would otherwise keep its row, and the next boot would bind it again — + * a `200 {deleted: true}` for a removal the platform does not keep, the same + * defect in the other direction. The same order and the same undo as the + * write path: + * + * - **Engine first.** The engine's own refusal (ADR-0126 §7.3's + * `DELETE_RESTRICTED`, a packaged subflow a packaged caller still reaches) + * is raised before the store is touched, so a refused removal deletes + * nothing. + * - **A name with no row is not a failure** — `deleteMetaItem` answers it as + * "nothing to delete" (a flow registered before its door saved, or in a + * composition with no store). + * - **A delete that fails puts the definition back** in the engine and + * relays the store's own failure, so no answer reports a removal that will + * not survive. A composition with no metadata store keeps the engine-only + * removal it always had. + * + * Returns the failure to answer, or nothing when the removal landed. + */ +async function unregisterAndDeleteFlow( + deps: DomainHandlerDeps, + context: HttpProtocolContext, + automationService: FlowRemovalService, + name: string, +): Promise { + const store: Partial> | undefined = + await deps.resolveServiceOrLoud(context, 'protocol'); + const held = typeof automationService.getFlow === 'function' ? await automationService.getFlow(name) : null; + automationService.unregisterFlow(name); + if (typeof store?.deleteMetaItem === 'function') { + try { + await store.deleteMetaItem({ type: FLOW_METADATA_TYPE, name }); + } catch (e) { + if (held) automationService.registerFlow?.(name, held); + return { handled: true, response: deps.errorFromThrown(e) }; + } + } + return undefined; +} + /** * [#9378] The ONE mapper both trigger doors answer through — `POST * /:name/trigger` and the legacy `POST /trigger/:name`, which @@ -2298,16 +2429,13 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // [#20552] Creating onto a name the engine already holds is an // overwrite, so the round-trip rule applies here as on `PUT /:name`. const definition = await keepStoredFlowCredentials(automationService, body.name, body); - let registered; - try { - registered = automationService.registerFlow(body.name, definition); - } catch (e) { - return { - handled: true, - response: deps.errorFromThrown(flowDefinitionRefusal(e), VALIDATION_FAILED_STATUS), - }; - } - return { handled: true, response: deps.success(servedFlowDefinition(registered)) }; + // [#20862] Registered AND saved as a tenant row, so the flow this + // answers `200` for survives a restart — see `registerAndSaveFlow`. + const written = await registerAndSaveFlow( + deps, context, automationService as FlowRegistrationService, body.name, definition, + ); + if ('refused' in written) return written.refused; + return { handled: true, response: deps.success(servedFlowDefinition(written.registered)) }; } } @@ -2591,49 +2719,28 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // asks admits it — and would refuse it, loudly, if the base's // provenance ever rode across. Asked before the engine is // called, like every other definition write here. The - // metadata protocol is resolved HERE, before anything is - // registered, so a slot that fails to resolve cannot leave a + // metadata protocol is resolved before anything is registered + // (by the rule here, and again by `registerAndSaveFlow` below), + // so a slot that fails to resolve cannot leave a // registered-but-unsaved clone behind. const authored = await refuseUnauthoredFlowWrite(deps, context, targetName, clone); if (authored) return authored; - const store: Partial> | undefined = - await deps.resolveServiceOrLoud(context, 'protocol'); - // Engine verdicts are answered as a 400, not rethrown — the - // create arm's reasoning (`flowDefinitionRefusal`) applies - // verbatim, and it matters more here: a clone that the engine - // refuses must not leave a half-registered flow behind, and it - // must not read as a server fault when the source definition is - // simply one this deployment can no longer register. - try { - automationService.registerFlow(targetName, clone); - } catch (e) { - return { - handled: true, - response: deps.errorFromThrown(flowDefinitionRefusal(e), VALIDATION_FAILED_STATUS), - }; - } // [#20761, ADR-0126 §7.1] …and SAVED as a tenant row. Registered // in the engine alone the clone was a process-local fact: the - // metadata door read its name as absent, and the next boot — - // which binds flows from the stored metadata — did not know it, - // so the copy an administrator made to customize a locked flow - // vanished at the first restart. It is written through the - // metadata protocol's own save, env-wide (a flow has no per-org - // channel), so it lands as an ordinary `sys_metadata` row the - // boot hydrates, `/meta` reads back, and the same authoring rule - // judges. Engine first, store second: the engine's registration - // is the stricter validation, and a save that then fails - // withdraws the registration, so the answer never reports a - // clone that will not survive. A composition with no metadata - // store to save into keeps the engine-only clone it always had. - if (typeof store?.saveMetaItem === 'function') { - try { - await store.saveMetaItem({ type: FLOW_METADATA_TYPE, name: targetName, item: clone }); - } catch (e) { - automationService.unregisterFlow?.(targetName); - return { handled: true, response: deps.errorFromThrown(e) }; - } - } + // metadata door read its name as absent, and the next boot did + // not know it, so the copy an administrator made to customize a + // locked flow vanished at the first restart. [#20862] Through the + // one persistence path the create and update doors share — + // `registerAndSaveFlow` — so the three cannot drift. Engine + // verdicts are answered as a 400 there, not rethrown, which + // matters most here: a clone the engine refuses must not read as + // a server fault when the source definition is simply one this + // deployment can no longer register. The target name is new (the + // collision probe above), so a save that fails withdraws it. + const written = await registerAndSaveFlow( + deps, context, automationService as FlowRegistrationService, targetName, clone, + ); + if ('refused' in written) return written.refused; // ⛔ NO ANCESTRY on the way out either (ADR-0126 amendment // ruling 2, §9): the response names the flow that was created // and says nothing about what it was copied from. There is no @@ -3237,16 +3344,15 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // [#20552] A body that round-trips the served (projected) form // keeps the secret the engine holds; an explicit one replaces it. const toRegister = await keepStoredFlowCredentials(automationService, name, definition); - let registered; - try { - registered = automationService.registerFlow(name, toRegister); - } catch (e) { - return { - handled: true, - response: deps.errorFromThrown(flowDefinitionRefusal(e), VALIDATION_FAILED_STATUS), - }; - } - return { handled: true, response: deps.success(servedFlowDefinition(registered)) }; + // [#20862] Registered AND saved as a tenant row, so the update + // survives a restart instead of losing to the stored + // definition; a save that fails puts back the definition the + // engine held — see `registerAndSaveFlow`. + const written = await registerAndSaveFlow( + deps, context, automationService as FlowRegistrationService, name, toRegister, + ); + if ('refused' in written) return written.refused; + return { handled: true, response: deps.success(servedFlowDefinition(written.registered)) }; } } @@ -3257,7 +3363,13 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // packaged base — refused before the engine is asked. const locked = await refusePackagedFlowBaseChange(deps, context, name, 'delete'); if (locked) return locked; - automationService.unregisterFlow(name); + // [#20862] …and removed from the store as well as the engine, + // so a flow the create door now saves does not come back at + // the next boot — see `unregisterAndDeleteFlow`. + const failed = await unregisterAndDeleteFlow( + deps, context, automationService as FlowRemovalService, name, + ); + if (failed) return failed; return { handled: true, response: deps.success({ name, deleted: true }) }; } }