From fec4827ec6d22025edc45eaf26179d13a53d32b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 00:58:10 +0000 Subject: [PATCH 1/9] fix(rest)!: an import undo runs under the object's declared controls for every caller but the system principal The undo door's restore and delete writes keep skipAutomations and skipStateMachine only for the system principal. A reversal a control refuses is reported per row in the undo answer and kept, with its refusal, in the job's undo log; reverted_at is stamped only when every row was reverted. Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- packages/rest/src/data-action-routes.ts | 119 ++++-- .../rest/src/import-undo-controls.test.ts | 388 ++++++++++++++++++ 2 files changed, 482 insertions(+), 25 deletions(-) create mode 100644 packages/rest/src/import-undo-controls.test.ts diff --git a/packages/rest/src/data-action-routes.ts b/packages/rest/src/data-action-routes.ts index e68effedee2..bf6a57a75bb 100644 --- a/packages/rest/src/data-action-routes.ts +++ b/packages/rest/src/data-action-routes.ts @@ -70,7 +70,7 @@ import { importControlOptOutRefusal, prepareImportRequest } from './import-prepa import { readTemplateMode, templateFieldDefs } from './import-template.js'; import { refuseRepeatedQueryParams } from './query-multiplicity.js'; import { refuseUnknownQueryParams } from './query-allowlist.js'; -import { handleRouteError } from './error-response.js'; +import { handleRouteError, logUnexpectedRouteError, mapDataError, nestedRefusalEnvelope } from './error-response.js'; // [#24348] The shared ADR-0112 nested envelope for the import doors' control // opt-out refusal — the flat sibling-`code` dialect the doors' older answers // use is ratcheted down by `check:route-envelope`. @@ -202,6 +202,68 @@ export interface DataActionRouteHost { answerImportTemplate: RestServer['answerImportTemplate']; } +/** + * [#24380] The write context an import UNDO's reversal writes run under — the + * deletes of the records the job created and the restores of the fields it + * updated. + * + * Those writes are ordinary data changes on a live app, so they run under the + * object's declared controls: its metadata-bound hooks (an approval gate, a + * closed-record freeze), its record-change triggers and its `state_machine` + * rule. Only the system principal, `ExecutionContext.isSystem === true` (an + * in-process caller such as a migration job), takes them off, with + * `skipAutomations` and `skipStateMachine`: the predicate the import doors ask + * before they honour a control opt-out (`importControlOptOutRefusal`, + * `import-prepare.ts`), and the same rule — only the system principal may take + * an app's declared controls off a write; the undo door is not an exception. + * An HTTP request is assembled with `isSystem: false`, so every signed-in + * caller, administrators included, undoes under the controls. ⛔ No + * permission, capability or administrator standing is read here. + * + * A historical job (`treat_as_historical`) keeps `preserveAudit` for every + * caller, mirroring the import's own write context (#3549): restoring the + * captured snapshot re-writes its `updated_at` / `updated_by` and business + * `readonly` values verbatim instead of stamping or stripping them. + */ +function importUndoWriteContext(context: unknown, job: { treat_as_historical?: unknown }): Record { + const systemPrincipal = (context as { isSystem?: unknown } | null | undefined)?.isSystem === true; + return { + ...((context ?? {}) as Record), + ...(systemPrincipal ? { skipAutomations: true, skipStateMachine: true } : {}), + ...(job.treat_as_historical ? { preserveAudit: true } : {}), + }; +} + +/** One reversal an import undo did not make: the row stays unreverted. */ +interface ImportUndoFailure { + /** The record the reversal targeted. */ + id: string; + /** `delete` for a record the job created, `restore` for one it updated. */ + action: 'delete' | 'restore'; + /** The ADR-0112 code the single-record write door answers the same failure with. */ + code: string; + /** That door's sentence for it. */ + error: string; +} + +/** + * [#24380] A reversal write that threw, as the undo reports it per row. The + * classification is the one `PATCH` / `DELETE /data/:object/:id` answer the + * same thrown error with ({@link mapDataError}, which withholds a fault's + * driver text), so a refusal by the object's hook, state machine or record + * lock reads in the words the app wrote for it. The code is read off that + * classification as the nested ADR-0112 envelope reads it + * ({@link nestedRefusalEnvelope}): an undeclared hook refusal carries none, + * and the catalog's floor for its status fills it. A fault that is not a + * refusal is logged once here, as those doors log it. + */ +function importUndoFailure(id: string, action: ImportUndoFailure['action'], error: unknown, objectName: string): ImportUndoFailure { + const mapped = mapDataError(error, objectName); + logUnexpectedRouteError(error, mapped); + const answer = nestedRefusalEnvelope(mapped); + return { id, action, code: answer.code, error: answer.message }; +} + /** * Register object-specific action endpoints that don't fit the generic CRUD * shape — `RestServer#registerDataActionEndpoints`' body, called by that method @@ -629,26 +691,18 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: } const objectName = String(row.object_name ?? ''); const log = parseUndoLog(row.undo_log)!; - // Undo automations too: reversing writes shouldn't re-fire triggers. - // Skip the state machine as well (#3479): restoring a prior snapshot - // re-writes the row's earlier state, which need not be a legal - // transition from where it is now — an undo reinstates an established - // fact, it does not walk the FSM. - // - // For a historical import (#3493/#3549), the undo write must mirror - // the import's own write context: carry `preserveAudit` so restoring - // `u.before` re-writes the captured `updated_at`/`updated_by` (and any - // business `readonly` fields in the snapshot) verbatim, rather than - // stamping-now / stripping them. Without this, undoing a historical - // import would silently rewrite the audit timeline the import took - // pains to preserve. A normal import keeps the default (stamp/strip). - const writeCtx = { - ...(context ?? {}), - skipAutomations: true, - skipStateMachine: true, - ...(row.treat_as_historical ? { preserveAudit: true } : {}), - }; - let deleted = 0, restored = 0, failed = 0; + // [#24380] The reversal writes run under the object's hooks, + // record-change triggers and state machine for every caller but + // the system principal, which keeps the skips; a historical job + // keeps `preserveAudit` (#3549). See `importUndoWriteContext`. + const writeCtx = importUndoWriteContext(context, row); + let deleted = 0, restored = 0; + // [#24380] Every reversal that did not happen, per row — a write + // a control refused (a hook, the state machine, a record lock) or + // one that failed otherwise. Never swallowed: each is reported in + // the answer and kept, with its row, in the job's undo log. + const failures: ImportUndoFailure[] = []; + const unreverted: { created: string[]; updated: typeof log.updated } = { created: [], updated: [] }; // Delete created records first (they didn't exist before). for (const id of log.created) { @@ -656,7 +710,10 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: const undoDeleteRequest: ServerScopedDataRequest = { object: objectName, id, context: writeCtx, ...(environmentId ? { environmentId } : {}) }; await p.deleteData(undoDeleteRequest); deleted++; - } catch { failed++; } + } catch (error: unknown) { + failures.push(importUndoFailure(id, 'delete', error, objectName)); + unreverted.created.push(id); + } } // Restore the touched fields on updated records. for (const u of log.updated) { @@ -664,17 +721,29 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: const undoRestoreRequest: ServerScopedDataRequest = { object: objectName, id: u.id, data: u.before, context: writeCtx, ...(environmentId ? { environmentId } : {}) }; await p.updateData(undoRestoreRequest); restored++; - } catch { failed++; } + } catch (error: unknown) { + failures.push(importUndoFailure(u.id, 'restore', error, objectName)); + unreverted.updated.push(u); + } } + // [#24380] What the job's history records. `reverted_at` means + // EVERY row was reverted, so it is stamped only then. An undo that + // left rows unreverted stamps nothing: it rewrites the undo log to + // exactly those rows, each refusal beside them under `failures`, + // so the job stays undoable and undoing it again retries only + // what remains — never a row already reverted. + const reverted = failures.length === 0; const undoStampRequest: ServerScopedDataRequest = { object: IMPORT_JOB_OBJECT, id: jobId, - data: { reverted_at: new Date().toISOString() }, + data: reverted + ? { reverted_at: new Date().toISOString() } + : { undo_log: { ...unreverted, failures } }, context: { ...(context as any), isSystem: true }, // [ADR-0103] engine-owned ...(environmentId ? { environmentId } : {}), }; await p.updateData(undoStampRequest); - res.json({ success: true, jobId, object: objectName, deleted, restored, failed }); + res.json({ success: reverted, jobId, object: objectName, deleted, restored, failed: failures.length, failures }); } catch (error: any) { handleRouteError(res, error, ''); } diff --git a/packages/rest/src/import-undo-controls.test.ts b/packages/rest/src/import-undo-controls.test.ts new file mode 100644 index 00000000000..2626ec3f3fb --- /dev/null +++ b/packages/rest/src/import-undo-controls.test.ts @@ -0,0 +1,388 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24380] AN IMPORT UNDO RUNS UNDER THE OBJECT'S DECLARED CONTROLS. + * + * `POST /data/import/jobs/:jobId/undo` reverses a finished import: it deletes + * the records the job created and restores the fields it updated. Those + * reversal writes are ordinary writes on a live app, so for every caller but + * the system principal (`ExecutionContext.isSystem`) they run under the + * object's metadata-bound hooks, its record-change triggers and its + * `state_machine` rule — the same rule the import doors keep: only the system + * principal may take an app's declared controls off a write. The system + * principal keeps `skipAutomations` / `skipStateMachine`. A historical job's + * undo keeps `preserveAudit` for every caller. + * + * A row a control refuses is reported per row, in the undo answer's `failures` + * and in the job's stored undo log, and is not counted as reverted: the job is + * stamped `reverted_at` only once every row is reverted, and until then it + * stays undoable for exactly the rows that remain. + * + * The fixtures are behaviour only, over the REAL routes, a REAL + * {@link ObjectQL} engine, the REAL protocol and a sqlite `:memory:` driver: + * + * - `ou_deal` carries an APPROVAL GATE: a metadata-bound `beforeUpdate` hook + * that re-opens approval (`approval_status: 'pending'`) whenever the amount + * changes; + * - `ou_case` carries a CLOSED-RECORD FREEZE: metadata-bound `beforeUpdate` + * and `beforeDelete` hooks that refuse any change to, or deletion of, a case + * whose stored `status` is `closed`; + * - `ou_ticket` carries a `state_machine` rule in which `closed` is terminal. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SysMetadataAuditObject, SysMetadataCommitObject, SysMetadataHistoryObject, SysMetadataObject } from '@objectstack/metadata-core'; +import { SysImportJob } from '@objectstack/platform-objects/audit'; +import { RestServer } from './rest-server'; + +function makeSqliteDriver() { + return new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); +} + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +const DEAL = { + name: 'ou_deal', label: 'Deal', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + amount: { name: 'amount', type: 'number' as const, label: 'Amount' }, + approval_status: { name: 'approval_status', type: 'text' as const, label: 'Approval status' }, + }, +}; + +const STATUS = { + name: 'status', type: 'select' as const, label: 'Status', + options: [{ label: 'Open', value: 'open' }, { label: 'Closed', value: 'closed' }], +}; + +const CASE = { + name: 'ou_case', label: 'Case', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + title: { name: 'title', type: 'text' as const, label: 'Title' }, + status: STATUS, + }, +}; + +const TICKET = { + name: 'ou_ticket', label: 'Ticket', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + title: { name: 'title', type: 'text' as const, label: 'Title' }, + status: STATUS, + }, + validations: [{ + type: 'state_machine', name: 'ticket_flow', message: 'A closed ticket stays closed.', + field: 'status', transitions: { open: ['closed'], closed: [] }, + }], +}; + +const FROZEN = 'A closed case is frozen.'; + +/** + * A metadata-bound hook body's refusal, in the shape the sandbox runner gives + * it (`.message` its debug wrapper, `.innerMessage` the author's sentence), as + * `rest-write-route-hook-refusal-sentence.ledger.test.ts` reproduces it: this + * package does not depend on `@objectstack/runtime`. + */ +function hookRefusal(hook: string, sentence: string): Error { + const err = new Error(`hook '${hook}' threw: Error: ${sentence}`); + err.name = 'SandboxError'; + return Object.assign(err, { innerMessage: sentence }); +} + +const MEMBER = { userId: 'usr_member' }; +const SYSTEM = { userId: 'usr_system_job', isSystem: true }; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(makeSqliteDriver(), true); + await engine.init(); + for (const o of [DEAL, CASE, TICKET, SysImportJob]) engine.registry.registerObject(o as any); + await engine.syncSchemas(); + for (const o of [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject]) { + if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as any); + } + + // The app's declared controls, bound from metadata exactly as a + // `defineStack({ hooks })` app binds them. + engine.bindHooks([ + { + name: 'ou_deal_approval_gate', + object: 'ou_deal', + events: ['beforeUpdate'], + handler: async (ctx: any) => { + const data = ctx.input?.data ?? {}; + if ('amount' in data && data.amount !== ctx.previous?.amount) data.approval_status = 'pending'; + }, + }, + { + name: 'ou_case_closed_freeze', + object: 'ou_case', + events: ['beforeUpdate', 'beforeDelete'], + handler: async (ctx: any) => { + if (ctx.previous?.status === 'closed') throw hookRefusal('ou_case_closed_freeze', FROZEN); + }, + }, + ] as any, { packageId: 'ou-app' }); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + let caller: Record = MEMBER; + (rest as any).resolveExecCtx = async () => caller; + rest.registerRoutes(); + const routes = rest.getRoutes(); + const find = (method: string, path: string): any => { + const route = routes.find((r: any) => r.method === method && r.path === path); + if (!route) throw new Error(`route ${method} ${path} is not registered`); + return route; + }; + const create = find('POST', '/api/v1/data/:object/import/jobs'); + const progressRoute = find('GET', '/api/v1/data/import/jobs/:jobId'); + const undoRoute = find('POST', '/api/v1/data/import/jobs/:jobId/undo'); + + const progress = async (jobId: string) => { + const res = makeRes(); + await progressRoute.handler({ params: { jobId }, query: {} } as any, res); + return res._json; + }; + + return { + engine, + protocol, + as(next: Record) { caller = next; }, + progress, + /** Run an async import job as the current caller and wait for it to finish. */ + async importJob(object: string, body: Record): Promise { + const res = makeRes(); + await create.handler({ params: { object }, body: { format: 'json', ...body } } as any, res); + expect(res._status).toBe(201); + const jobId = res._json.jobId as string; + for (let i = 0; i < 200; i++) { + const p = await progress(jobId); + if (['succeeded', 'failed', 'cancelled'].includes(p?.status)) { + expect(p.status).toBe('succeeded'); + return jobId; + } + await new Promise((r) => setTimeout(r, 5)); + } + throw new Error('import job did not finish'); + }, + async undo(jobId: string) { + const res = makeRes(); + await undoRoute.handler({ params: { jobId } } as any, res); + return res; + }, + /** The job row as stored, read the way the engine-owned writer reads it. */ + async jobRow(jobId: string): Promise { + return engine.findOne('sys_import_job', { where: { id: jobId }, context: { isSystem: true } } as any); + }, + async record(object: string, id: string): Promise { + return engine.findOne(object, { where: { id }, context: { isSystem: true } } as any); + }, + /** A later edit on the live app, by a signed-in user, between the import and its undo. */ + async edit(object: string, id: string, data: Record) { + await protocol.updateData({ object, id, data, context: MEMBER } as any); + }, + }; +} + +/** The stored undo log, whichever way the json column materialised. */ +function storedUndoLog(row: any): any { + const raw = row?.undo_log; + return typeof raw === 'string' ? JSON.parse(raw) : raw; +} + +describe('[#24380] an import undo runs under the object\'s declared controls for every caller but the system principal', () => { + let ctx: Awaited>; + beforeEach(async () => { ctx = await boot(); }); + + /** One deal the import updates (its amount) and one it creates. */ + async function dealJob(): Promise { + await ctx.engine.insert('ou_deal', { id: 'd_old', name: 'Acme renewal', amount: 100, approval_status: 'approved' }, { context: { isSystem: true } } as any); + const jobId = await ctx.importJob('ou_deal', { + writeMode: 'upsert', matchFields: ['id'], + rows: [{ id: 'd_old', amount: 250 }, { id: 'd_new', name: 'Globex', amount: 10 }], + }); + // The import re-opened approval on the changed amount; the app's approver + // then approves it again, before anyone undoes the import. + expect(await ctx.record('ou_deal', 'd_old')).toMatchObject({ amount: 250, approval_status: 'pending' }); + await ctx.edit('ou_deal', 'd_old', { approval_status: 'approved' }); + return jobId; + } + + it('a signed-in caller\'s undo runs the object\'s hooks on its reversal writes, and reverts every row', async () => { + const jobId = await dealJob(); + + const res = await ctx.undo(jobId); + expect(res._status ?? 200).toBe(200); + expect(res._json).toEqual({ + success: true, jobId, object: 'ou_deal', deleted: 1, restored: 1, failed: 0, failures: [], + }); + // The restore is a change of amount, so the approval gate re-opened approval. + expect(await ctx.record('ou_deal', 'd_old')).toMatchObject({ amount: 100, approval_status: 'pending' }); + expect(await ctx.record('ou_deal', 'd_new')).toBeNull(); + + const after = await ctx.progress(jobId); + expect(after.undoable).toBe(false); + expect(after.revertedAt).toBeTruthy(); + }); + + it('the system principal\'s undo keeps the skips: no metadata-bound hook runs on its reversal writes', async () => { + const jobId = await dealJob(); + + ctx.as(SYSTEM); + const res = await ctx.undo(jobId); + expect(res._json).toMatchObject({ success: true, deleted: 1, restored: 1, failed: 0, failures: [] }); + expect(await ctx.record('ou_deal', 'd_old')).toMatchObject({ amount: 100, approval_status: 'approved' }); + expect(await ctx.record('ou_deal', 'd_new')).toBeNull(); + }); + + it('a signed-in caller\'s undo walks the state machine: a restore it refuses is reported per row and is not counted as reverted', async () => { + await ctx.engine.insert('ou_ticket', [ + { id: 't_closed', title: 'printer', status: 'open' }, + { id: 't_renamed', title: 'old name', status: 'open' }, + ], { context: { isSystem: true } } as any); + const jobId = await ctx.importJob('ou_ticket', { + writeMode: 'update', matchFields: ['id'], + rows: [{ id: 't_closed', status: 'closed' }, { id: 't_renamed', title: 'new name' }], + }); + + const res = await ctx.undo(jobId); + expect(res._status ?? 200).toBe(200); + expect(res._json).toMatchObject({ success: false, jobId, object: 'ou_ticket', deleted: 0, restored: 1, failed: 1 }); + expect(res._json.failures).toEqual([ + { id: 't_closed', action: 'restore', code: 'VALIDATION_FAILED', error: 'A closed ticket stays closed.' }, + ]); + expect(await ctx.record('ou_ticket', 't_closed')).toMatchObject({ status: 'closed' }); + expect(await ctx.record('ou_ticket', 't_renamed')).toMatchObject({ title: 'old name' }); + + // The job's history: not stamped reverted, and the stored undo log keeps + // exactly the row that remains, with its refusal. + const row = await ctx.jobRow(jobId); + expect(row.reverted_at ?? null).toBeNull(); + const log = storedUndoLog(row); + expect(log.created).toEqual([]); + expect(log.updated).toEqual([{ id: 't_closed', before: { id: 't_closed', status: 'open' } }]); + expect(log.failures).toEqual(res._json.failures); + const after = await ctx.progress(jobId); + expect(after.undoable).toBe(true); + expect(after.revertedAt).toBeUndefined(); + + // The rest stays undoable, and a second undo retries only that row: the + // system principal's skips the state machine. + ctx.as(SYSTEM); + const retry = await ctx.undo(jobId); + expect(retry._json).toMatchObject({ success: true, deleted: 0, restored: 1, failed: 0, failures: [] }); + expect(await ctx.record('ou_ticket', 't_closed')).toMatchObject({ status: 'open' }); + const done = await ctx.progress(jobId); + expect(done.undoable).toBe(false); + expect(done.revertedAt).toBeTruthy(); + }); + + it('a delete and a restore a closed-record freeze refuses are each reported per row, and the job keeps both', async () => { + await ctx.engine.insert('ou_case', { id: 'c_old', title: 'before', status: 'open' }, { context: { isSystem: true } } as any); + const jobId = await ctx.importJob('ou_case', { + writeMode: 'upsert', matchFields: ['id'], + rows: [{ id: 'c_old', title: 'after' }, { id: 'c_new', title: 'fresh', status: 'open' }], + }); + // Both cases are closed on the live app after the import. + await ctx.edit('ou_case', 'c_old', { status: 'closed' }); + await ctx.edit('ou_case', 'c_new', { status: 'closed' }); + + const res = await ctx.undo(jobId); + expect(res._json).toMatchObject({ success: false, deleted: 0, restored: 0, failed: 2 }); + expect(res._json.failures).toEqual([ + { id: 'c_new', action: 'delete', code: 'VALIDATION_ERROR', error: FROZEN }, + { id: 'c_old', action: 'restore', code: 'VALIDATION_ERROR', error: FROZEN }, + ]); + expect(await ctx.record('ou_case', 'c_new')).toMatchObject({ title: 'fresh', status: 'closed' }); + expect(await ctx.record('ou_case', 'c_old')).toMatchObject({ title: 'after', status: 'closed' }); + + const row = await ctx.jobRow(jobId); + expect(row.reverted_at ?? null).toBeNull(); + const log = storedUndoLog(row); + expect(log.created).toEqual(['c_new']); + expect(log.updated.map((u: any) => u.id)).toEqual(['c_old']); + expect(log.failures).toEqual(res._json.failures); + }); + + describe('the write context the reversal writes carry', () => { + function spyWriteContexts() { + const seen: Array<{ op: string; object: string; context: any }> = []; + for (const op of ['updateData', 'deleteData'] as const) { + const orig = (ctx.protocol as any)[op].bind(ctx.protocol); + (ctx.protocol as any)[op] = async (args: any) => { + seen.push({ op, object: args.object, context: args.context }); + return orig(args); + }; + } + return () => seen.filter((s) => s.object === 'ou_deal'); + } + + /** A historical job, created by the system principal (the only caller the import doors let send it). */ + async function historicalJob(): Promise { + await ctx.engine.insert('ou_deal', { id: 'h_old', name: 'Initech', amount: 5, approval_status: 'approved' }, { context: { isSystem: true } } as any); + ctx.as(SYSTEM); + const jobId = await ctx.importJob('ou_deal', { + writeMode: 'upsert', matchFields: ['id'], treatAsHistorical: true, + rows: [{ id: 'h_old', amount: 7 }, { id: 'h_new', name: 'Umbrella', amount: 1 }], + }); + ctx.as(MEMBER); + return jobId; + } + + it('a historical job keeps preserveAudit on a signed-in caller\'s undo, which carries no control skip', async () => { + const jobId = await historicalJob(); + const writes = spyWriteContexts(); + const res = await ctx.undo(jobId); + expect(res._json).toMatchObject({ success: true, deleted: 1, restored: 1 }); + expect(writes().map((w) => w.op).sort()).toEqual(['deleteData', 'updateData']); + for (const w of writes()) { + expect(w.context?.preserveAudit).toBe(true); + expect(w.context?.skipAutomations).toBeUndefined(); + expect(w.context?.skipStateMachine).toBeUndefined(); + } + }); + + it('a historical job keeps preserveAudit on the system principal\'s undo, beside its skips', async () => { + const jobId = await historicalJob(); + ctx.as(SYSTEM); + const writes = spyWriteContexts(); + await ctx.undo(jobId); + expect(writes().length).toBe(2); + for (const w of writes()) { + expect(w.context).toMatchObject({ preserveAudit: true, skipAutomations: true, skipStateMachine: true }); + } + }); + }); +}); From 72382a087bf934de96d71c0baba5079243a042f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 00:59:39 +0000 Subject: [PATCH 2/9] fix(spec): the undo response reports per-row failures; skipAutomations names no undo exception Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- content/docs/permissions/system-context.mdx | 1 + .../docs/protocol/objectql/state-machine.mdx | 1 + ...route-hook-refusal-sentence.ledger.test.ts | 5 ++-- packages/spec/src/api/export.zod.ts | 27 ++++++++++++++++--- packages/spec/src/data/hook.zod.ts | 2 +- .../spec/src/kernel/execution-context.zod.ts | 13 ++++++--- 6 files changed, 38 insertions(+), 11 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index d5086daba4a..17d4e18307c 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -141,6 +141,7 @@ that silently does not happen. | 29c | A detail write's master-detail header read asks **no read door** | objectql | Get: the write binds the header a parent-scoped `requiredWhen` / `readonlyWhen` reads as `parent` from the header's stored row whoever can read it, as before. Every caller that is not system meets its own read door first: a header that read does not return answers as a header that exists nowhere — an unbound `parent`, and `reference_not_found` on the reference check (which a system caller's write skips altogether) | `packages/objectql/src/engine.ts#masterDetailHeadersCallerReads` | | 30 | **Bulk data event `organizationId` OMITTED** — the batch is published "not asserted" | plugin-security | Get: nothing — the `data.records.*` event still publishes. Lose: the per-organization attribution: this exit is taken before the security middleware composes any tenant wall, so it records no Layer 0 verdict on the operation (`OperationContext.tenantLayer0Verdict`, #15813), and the engine's bulk producer — which reads that recorded verdict and nothing else — omits the key rather than filling it from the caller's `tenantId`; a tenant-scoped consumer then does not deliver the event inside an organization wall (#15225) | `packages/plugins/plugin-security/src/security-plugin.ts#start` | | 30b | **The import doors admit a control opt-out** — `runAutomations: false` and `treatAsHistorical: true` on `POST /data/:object/import` and `/import/jobs` | rest | Get: an import whose rows are written with `skipAutomations` (no metadata-bound hook, no record-change trigger: an app's approvals and record freezes among them) or with `skipStateMachine` and `preserveAudit`. Every other caller, every signed-in user and administrator included, is refused `403 PERMISSION_DENIED` before the body is parsed, so nothing is written and no job is created. Inbound HTTP never carries the flag, so this is an in-process caller's door. Note: the flag still suppresses none of those controls by itself (see below); it only admits the opt-outs that do | `packages/rest/src/import-prepare.ts#importControlOptOutRefusal` | +| 30c | **The import undo keeps the control skips** — `POST /data/import/jobs/:jobId/undo` | rest | Get: an undo whose deletes and restores are written with `skipAutomations` and `skipStateMachine`, so no metadata-bound hook, record-change trigger or `state_machine` rule refuses a reversal. Every other caller, every signed-in user and administrator included, undoes under those controls: a reversal one of them refuses is reported per row in the undo answer's `failures` and kept in the job's undo log, and the job is stamped reverted only once every row is. A historical job's `preserveAudit` belongs to the job, not the caller, and rides every undo. Inbound HTTP never carries the flag, so this is an in-process caller's door | `packages/rest/src/data-action-routes.ts#importUndoWriteContext` | ### 3. Sharing (`plugin-sharing`) diff --git a/content/docs/protocol/objectql/state-machine.mdx b/content/docs/protocol/objectql/state-machine.mdx index 8ba76358685..3399baefe76 100644 --- a/content/docs/protocol/objectql/state-machine.mdx +++ b/content/docs/protocol/objectql/state-machine.mdx @@ -115,6 +115,7 @@ transitions: { - **`treatAsHistorical` also preserves the original audit timeline** (#3493) — **on the rows an import UPDATES** (#6640). Skipping the FSM is only half of migrating established facts; the other half is keeping *when* they happened and *who* did them. Under the same flag the write context also carries `preserveAudit`, which (1) makes `updated_at` / `updated_by` **client-preferred** — a supplied historical last-modified survives instead of being stamped with the import instant — and (2) admits a **whitelist** through the static-`readonly` write strip: the audit/timestamp family plus author-declared business `readonly` fields (`closed_at`, `resolved_by`, …) — but never the record's own primary key (`id`), which is the address of the write rather than a fact being restored (#8215). Platform-managed `system` columns outside that family (`organization_id` and other tenancy/generated columns) stay stripped — a historical import reinstates facts, it does not forge tenancy. Like the FSM exemption this is opt-in: a normal write still auto-stamps `updated_at`/`updated_by` and strips `readonly` exactly as before, and permissions / RLS / field-level security are unchanged. - **…but a historical `upsert` still drops those columns from the rows it CREATES** (#6640). `preserveAudit` is an **UPDATE-path exemption and nothing else reads it**: both write paths run the same in-engine strip (`stripReadonlyFields`, #14147), but only the UPDATE call site passes `preserveAudit` — the create side is given no such flag and keeps `context.isSystem` as its only exemption. So a single `treatAsHistorical` upsert keeps `closed_at` on the rows it **matches** and strips it — together with a supplied `created_at` / `updated_at`, which the injected audit columns also declare `readonly` — from the rows it **inserts**. The asymmetry is deliberate, not an oversight: `treatAsHistorical` arrives on an ordinary (non-system) import request, so honouring it on create would let any caller seed the approval/status columns that create-side strip exists to protect. The ignored request is at least no longer silent — the server logs a `WARN` naming the object, the stripped fields and this UPDATE-only rule — but the strip still applies. **To replay archival read-only facts on the rows an import creates, write from a system context** (`isSystem`). Over HTTP this case no longer arises: the import doors admit `treatAsHistorical: true` from the system principal alone, whose creates the strip exempts; it still holds for an in-process, non-system caller of the import runner. Full rule and rationale: [Security & Access Control](/docs/protocol/objectql/security). - **Undoing a historical import is symmetric** (#3549 / #3556). The import undo (`POST /api/v1/data/import/jobs/:jobId/undo`) logically rolls back a finished job — deleting the rows it created and restoring the captured pre-import snapshot on the rows it updated. That restore write now carries `preserveAudit` too, but **only** when the job was flagged `treatAsHistorical`, so the snapshotted `updated_at` / `updated_by` and business `readonly` fields (`closed_at`, …) are reinstated verbatim instead of being re-stamped to the undo instant. The undo is unaffected by the create-side carve-out above: it only ever *deletes* the rows the import created and *updates* the rows it touched, so every write it makes is on the path where the exemption is real. Without it the undo would silently overwrite the very timeline the historical import preserved; a normal (non-historical) import's undo keeps the default stamp/strip. +- **The undo walks the state machine for every caller but the system principal.** Restoring a snapshot is a transition like any other write, so a signed-in caller's undo, historical job or not, runs under the object's `state_machine` rule, its hooks and its record-change triggers; only the system principal's undo skips them, the same rule the import doors keep for `treatAsHistorical`. A restore the rule refuses is not counted as reverted: the undo answer lists it in `failures` with its code and message, the job keeps that row in its undo log and is not stamped `revertedAt`, and undoing the job again retries only the rows that remain. ### Conditional transitions diff --git a/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts b/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts index ee3d28c2633..d8458c7aaca 100644 --- a/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts +++ b/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts @@ -298,8 +298,9 @@ const ANSWERS_THE_SENTENCE: Record = { }, // The job lookup is what reaches these two answers: the cancel's own status // write swallows its failure (the in-memory flag still stops the worker), - // and the undo's writes run under `skipAutomations`, which the engine reads - // to skip every metadata-bound hook, and count their failures into `failed`. + // and a refusal of one of the undo's reversal writes is that row's entry in + // the answer's `failures`, in the hook's own sentence, never the route's + // answer (`import-undo-controls.test.ts` pins it per row). 'POST /api/v1/data/import/jobs/:jobId/cancel': { called: 'protocol.findData', protocol: (error) => ({ findData: async () => { throw error(); } }), diff --git a/packages/spec/src/api/export.zod.ts b/packages/spec/src/api/export.zod.ts index c613940d41d..c6730e00d13 100644 --- a/packages/spec/src/api/export.zod.ts +++ b/packages/spec/src/api/export.zod.ts @@ -470,7 +470,7 @@ export const ImportJobProgressSchema = lazySchema(() => z.object({ errors: z.number().int().describe('Rows that failed'), percentComplete: z.number().min(0).max(100).describe('processed / total as a percentage'), undoable: z.boolean().describe('Whether this job can still be logically rolled back (undo log captured, terminal state, not yet reverted)'), - revertedAt: z.string().optional().describe('When the job was undone / rolled back (ISO 8601)'), + revertedAt: z.string().optional().describe('When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows.'), error: z.string().optional().describe('Fatal error message (when status = failed)'), startedAt: z.string().optional().describe('Processing start timestamp (ISO 8601)'), completedAt: z.string().optional().describe('Completion timestamp (ISO 8601)'), @@ -524,7 +524,7 @@ export const ImportJobSummarySchema = lazySchema(() => z.object({ createdAt: z.string().describe('Job creation timestamp (ISO 8601)'), completedAt: z.string().optional().describe('Completion timestamp (ISO 8601)'), undoable: z.boolean().describe('Whether this job can still be logically rolled back'), - revertedAt: z.string().optional().describe('When the job was undone / rolled back (ISO 8601)'), + revertedAt: z.string().optional().describe('When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows.'), })); export type ImportJobSummary = z.input; @@ -537,14 +537,33 @@ export type ListImportJobsResponse = z.input z.object({ - success: z.boolean().describe('Whether the undo completed'), + success: z.boolean().describe('Whether every row was reverted. False when any row is listed in `failures`: the undo did not complete, and the job is not stamped reverted.'), jobId: z.string().describe('Import job id'), object: z.string().describe('Target object name'), deleted: z.number().int().describe('Created records deleted'), restored: z.number().int().describe('Updated records restored to pre-import values'), - failed: z.number().int().describe('Reversal operations that failed'), + failed: z.number().int().describe('Rows the undo did not revert: one per entry in `failures`'), + failures: z.array(z.object({ + id: z.string().describe('Id of the record the reversal targeted'), + action: z.enum(['delete', 'restore']).describe('The reversal that did not happen: `delete` for a record the import created, `restore` for one it updated'), + code: z.string().describe('Error code the write was refused or failed with, as the single-record write door answers it'), + error: z.string().describe('Human-readable message: a refusing hook or validation rule\'s own sentence, or the door\'s withheld-fault sentence'), + })).describe( + 'One entry per row the undo did not revert, in the order the undo tried them (deletes first). For every caller ' + + 'but the system principal the reversal writes run under the object\'s hooks, record-change triggers and ' + + '`state_machine` rule, so a row one of them refuses is listed here, as is a write that failed for any other ' + + 'reason. The job keeps these rows, each with its entry, in its undo log and stays undoable: undoing it again ' + + 'retries only them. Empty when every row was reverted.', + ), })); export type UndoImportJobResponse = z.input; diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index 65accbc72c3..d6e91053d76 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -1003,7 +1003,7 @@ export const HookContextSchema = lazySchema(() => z.object({ accessToken: z.string().optional(), isSystem: z.boolean().optional().describe('True when the call was made with an elevated system context (engine self-writes)'), skipTriggers: z.boolean().optional().describe('True when record-change automation (flow triggers) must be suppressed for this write — e.g. package seed replay. Lifecycle hooks still run.'), - skipAutomations: z.boolean().optional().describe('True when metadata-bound automation hooks must be suppressed for this write — e.g. data import with "run automations" unchecked, or import undo. Implies skipTriggers; code-registered system hooks (audit, security) still run.'), + skipAutomations: z.boolean().optional().describe('True when metadata-bound automation hooks must be suppressed for this write — e.g. a data import the system principal runs with "run automations" unchecked, or the system principal\'s import undo. Implies skipTriggers; code-registered system hooks (audit, security) still run. Only the system principal may take an app\'s declared controls off a write, and the import undo door is not an exception: an undo by any other caller runs under the object\'s hooks, record-change triggers and state machine.'), /** * Position names held by the caller (ADR-0090 D3 vocabulary — the schema * comment on `ExecutionContext.positions` spells it "Formerly `roles`"). diff --git a/packages/spec/src/kernel/execution-context.zod.ts b/packages/spec/src/kernel/execution-context.zod.ts index 886eb2b3e01..43a46fa66ad 100644 --- a/packages/spec/src/kernel/execution-context.zod.ts +++ b/packages/spec/src/kernel/execution-context.zod.ts @@ -431,9 +431,12 @@ export const ExecutionContextSchema = lazySchema(() => z.object({ * is suppressed too (implies {@link skipTriggers}). Hooks registered in * code by plugins — audit, capability gates, sharing projection — carry no * metadata binding and STILL run: this flag must never bypass security or - * audit. Set by the data-import runner when the user opts out of - * "run automations & triggers", and by import undo (reversing writes must - * not re-fire automation). + * audit. Set by the data-import runner when the system principal opts out + * of "run automations & triggers", and by the import undo when the system + * principal undoes a job. Only the system principal may take an app's + * declared controls off a write, and neither the import doors nor the + * import undo door is an exception: any other caller's import and undo run + * under the object's hooks, record-change triggers and state machine. */ skipAutomations: z.boolean().optional(), @@ -475,7 +478,9 @@ export const ExecutionContextSchema = lazySchema(() => z.object({ * skip-the-state-machine intent. The engine treats either flag identically. * Server-constructed only, never client-supplied — the REST import runner sets * it from the request's explicit `treatAsHistorical` option, gated so a normal - * import still walks the FSM. SCOPE is identical to `seedReplay`: only the + * import still walks the FSM, and the import undo sets it only when the + * system principal undoes a job: any other caller's undo walks the FSM, so + * a restore it refuses is reported per row. SCOPE is identical to `seedReplay`: only the * `state_machine` rule is skipped; every other validation still runs. */ skipStateMachine: z.boolean().optional(), From e2d71b64d7736870e8d00518d822e57c3175f594 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:04:22 +0000 Subject: [PATCH 3/9] chore(spec): regenerate reference docs, strictness ledger and authorable surface Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- content/docs/references/api/export.mdx | 22 ++++++++++++++----- content/docs/references/data/hook.mdx | 2 +- .../api.md | 2 +- packages/spec/authorable-surface/api.json | 1 + 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/content/docs/references/api/export.mdx b/content/docs/references/api/export.mdx index 2b879a1021f..6f176041c86 100644 --- a/content/docs/references/api/export.mdx +++ b/content/docs/references/api/export.mdx @@ -182,7 +182,7 @@ const result = CreateImportJobRequestSchema.parse(data); | **errors** | `integer` | ✅ | Rows that failed | | **percentComplete** | `number` | ✅ | processed / total as a percentage | | **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back (undo log captured, terminal state, not yet reverted) | -| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) | +| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. | | **error** | `string` | optional | Fatal error message (when status = failed) | | **startedAt** | `string` | optional | Processing start timestamp (ISO 8601) | | **completedAt** | `string` | optional | Completion timestamp (ISO 8601) | @@ -210,7 +210,7 @@ const result = CreateImportJobRequestSchema.parse(data); | **errors** | `integer` | ✅ | Rows that failed | | **percentComplete** | `number` | ✅ | processed / total as a percentage | | **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back (undo log captured, terminal state, not yet reverted) | -| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) | +| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. | | **error** | `string` | optional | Fatal error message (when status = failed) | | **startedAt** | `string` | optional | Processing start timestamp (ISO 8601) | | **completedAt** | `string` | optional | Completion timestamp (ISO 8601) | @@ -266,7 +266,7 @@ const result = CreateImportJobRequestSchema.parse(data); | **createdAt** | `string` | ✅ | Job creation timestamp (ISO 8601) | | **completedAt** | `string` | optional | Completion timestamp (ISO 8601) | | **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back | -| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) | +| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. | --- @@ -533,7 +533,7 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa | **createdAt** | `string` | ✅ | Job creation timestamp (ISO 8601) | | **completedAt** | `string` | optional | Completion timestamp (ISO 8601) | | **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back | -| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) | +| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. | --- @@ -544,12 +544,22 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **success** | `boolean` | ✅ | Whether the undo completed | +| **success** | `boolean` | ✅ | Whether every row was reverted. False when any row is listed in `failures`: the undo did not complete, and the job is not stamped reverted. | | **jobId** | `string` | ✅ | Import job id | | **object** | `string` | ✅ | Target object name | | **deleted** | `integer` | ✅ | Created records deleted | | **restored** | `integer` | ✅ | Updated records restored to pre-import values | -| **failed** | `integer` | ✅ | Reversal operations that failed | +| **failed** | `integer` | ✅ | Rows the undo did not revert: one per entry in `failures` | +| **failures** | `{ id: string; action: Enum<'delete' \| 'restore'>; code: string; error: string }[]` | ✅ | One entry per row the undo did not revert, in the order the undo tried them (deletes first). For every caller but the system principal the reversal writes run under the object's hooks, record-change triggers and `state_machine` rule, so a row one of them refuses is listed here, as is a write that failed for any other reason. The job keeps these rows, each with its entry, in its undo log and stays undoable: undoing it again retries only them. Empty when every row was reverted. | + +### Nested Shape: `UndoImportJobResponse.failures[number]` + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **id** | `string` | ✅ | Id of the record the reversal targeted | +| **action** | `Enum<'delete' \| 'restore'>` | ✅ | The reversal that did not happen: `delete` for a record the import created, `restore` for one it updated | +| **code** | `string` | ✅ | Error code the write was refused or failed with, as the single-record write door answers it | +| **error** | `string` | ✅ | Human-readable message: a refusing hook or validation rule's own sentence, or the door's withheld-fault sentence | --- diff --git a/content/docs/references/data/hook.mdx b/content/docs/references/data/hook.mdx index 0f1d01ca8a9..7c712afa1d5 100644 --- a/content/docs/references/data/hook.mdx +++ b/content/docs/references/data/hook.mdx @@ -123,7 +123,7 @@ L2 sandboxed JS body — runs inside an isolated VM with declared capabilities | **accessToken** | `string` | optional | | | **isSystem** | `boolean` | optional | True when the call was made with an elevated system context (engine self-writes) | | **skipTriggers** | `boolean` | optional | True when record-change automation (flow triggers) must be suppressed for this write — e.g. package seed replay. Lifecycle hooks still run. | -| **skipAutomations** | `boolean` | optional | True when metadata-bound automation hooks must be suppressed for this write — e.g. data import with "run automations" unchecked, or import undo. Implies skipTriggers; code-registered system hooks (audit, security) still run. | +| **skipAutomations** | `boolean` | optional | True when metadata-bound automation hooks must be suppressed for this write — e.g. a data import the system principal runs with "run automations" unchecked, or the system principal's import undo. Implies skipTriggers; code-registered system hooks (audit, security) still run. Only the system principal may take an app's declared controls off a write, and the import undo door is not an exception: an undo by any other caller runs under the object's hooks, record-change triggers and state machine. | | **positions** | `string[]` | optional | Position names held by the caller (ADR-0090 D3; formerly `roles`), copied from ExecutionContext.positions. For hook READS only — e.g. tailoring a message, or branching a business rule the hook runs through its own `ctx.api` channel. Authorization is decided by the security service on the ExecutionContext (permissions / positions / derived posture); this is NOT an authorization input and a hook must not gate a write by testing it. A hook context carries no `services` key, so the sharing service cannot be called from one either — the sharing gates already ran inside the engine before the hook chain. | | **preserveAudit** | `boolean` | optional | True when this write is a historical import that must KEEP its caller-supplied updated_at/updated_by (and the readonly audit family) instead of being stamped with the import instant. Server-set, opt-in, absent on normal writes; read by the built-in audit hook. A stamping policy, not an authorization input. | | **roles** | `never` | optional | [REMOVED] `HookContext.session.roles` was removed in @objectstack/spec 17.0.0 (ADR-0049 D2) — it was declared, read by two dead exemption branches, and never produced: ObjectQL's `buildSession()` builds the session field by field and has never written `roles`, so every read resolved `undefined` and a guard keyed on it was dead code that merely LOOKED like an authorization decision. Delete the key. To gate a hook on the caller, read `ctx.session.userId` / `ctx.session.isSystem`; to judge PRIVILEGE, ask the security service, which evaluates the ADR-0095 vocabulary on the execution context — capability grants (`permissions`), placements (`positions`) and the derived posture — never a role-name string comparison (ADR-0090 D3 bans the `role` spelling outright). Nothing to migrate: a HookContext is built per operation by the engine and never stored, so no metadata source carries this key. NOTE an ACTION body's `ctx.session` is a different object and still carries its own `roles` array today; that surface is tracked separately and is not what this key was. | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md b/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md index fc4533606bd..80c986c933e 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.counts/api.md @@ -19,4 +19,4 @@ hand-patch a number here** — fix the code or the verdict and regenerate. | Dir | Sites | |---|---| -| `api/` | 445 | +| `api/` | 446 | diff --git a/packages/spec/authorable-surface/api.json b/packages/spec/authorable-surface/api.json index 60fee7e2e9d..350090e90fc 100644 --- a/packages/spec/authorable-surface/api.json +++ b/packages/spec/authorable-surface/api.json @@ -1700,6 +1700,7 @@ "api/TriggerFlowResponse:success", "api/UndoImportJobResponse:deleted", "api/UndoImportJobResponse:failed", + "api/UndoImportJobResponse:failures", "api/UndoImportJobResponse:jobId", "api/UndoImportJobResponse:object", "api/UndoImportJobResponse:restored", From 629f02582c970caa4c415b536c268acbc8b84ab1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:05:39 +0000 Subject: [PATCH 4/9] chore(changeset): the undo's narrowing on rest and the response widening on spec Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- .changeset/24380-import-undo-controls.md | 20 +++++++++++++++++++ .changeset/24380-undo-response-failures.md | 9 +++++++++ .../rest/src/import-undo-controls.test.ts | 3 +++ 3 files changed, 32 insertions(+) create mode 100644 .changeset/24380-import-undo-controls.md create mode 100644 .changeset/24380-undo-response-failures.md diff --git a/.changeset/24380-import-undo-controls.md b/.changeset/24380-import-undo-controls.md new file mode 100644 index 00000000000..6bdff62ce22 --- /dev/null +++ b/.changeset/24380-import-undo-controls.md @@ -0,0 +1,20 @@ +--- +'@objectstack/rest': minor +--- + +fix(rest)!: an import undo runs under the object's hooks, record-change triggers and state machine for every caller but the system principal, and reports each row a control refuses instead of counting it as reverted + +Clause-②: yes (narrowing) + + + +**BREAKING** (the import undo door takes fewer controls off its writes, and claims less: `POST /api/v1/data/import/jobs/:jobId/undo` keeps `skipAutomations` and `skipStateMachine` on its reversal writes for the system principal alone, so every other caller's undo, administrators included, runs under the object's metadata-bound hooks, record-change triggers and `state_machine` rule; and an undo that leaves any row unreverted now answers `success: false` and leaves the job unstamped, where it answered `success: true` and stamped it reverted), shipped as `minor` under the launch-window convention for breaking changes. The cost is named, not hidden: undoing an ordinary import may fire record-change automation again (a notification, an approval re-opening on a restored amount), which is the app's declared behaviour for those writes. If a row now stays unreverted, read its entry in `failures`, resolve what refuses it on the app (complete the approval, reopen the record) and undo the job again, which retries only the rows that remain; a reversal that must skip the controls runs in-process as the system principal. + +The same rule now holds at every import door: only the system principal may take an app's declared controls off a write. The import doors already refused a caller-chosen opt-out from everyone else; the undo door chose its skips itself, for every caller, and no longer does. + +**What changes.** The undo's deletes (of the records the job created) and restores (of the fields it updated) are written as `ExecutionContext.isSystem` decides: the system principal keeps `skipAutomations` and `skipStateMachine`; every other caller writes with neither. A reversal that a hook, the state machine, a record lock or anything else refuses is caught for its row and never swallowed: + +- the answer lists it in a new `failures` array, one `{ id, action, code, error }` per row, where `action` is `delete` or `restore` and `code` / `error` are what the single-record write door answers the same error with (a hook's refusal reads in the hook's own sentence); `failed` is its length and `success` is `false`; +- the job's stored undo log is rewritten to exactly the unreverted rows, each refusal beside them under `failures`, and `reverted_at` is not stamped. The job stays `undoable`, and undoing it again retries only those rows, never one already reverted. `reverted_at` (`revertedAt` on the job's progress and history) now means every row was reverted. + +**What stays as it was.** An undo that no control refuses reverts every row and answers as before, with `failures: []`. A historical job (`treatAsHistorical`) keeps `preserveAudit` on its undo for every caller. The job-row write that records the outcome stays engine-owned (ADR-0103). The 404, 409 `ALREADY_REVERTED` and 422 `NOT_UNDOABLE` answers are unchanged, and the import doors and their opt-out gate are untouched. diff --git a/.changeset/24380-undo-response-failures.md b/.changeset/24380-undo-response-failures.md new file mode 100644 index 00000000000..d9b851c719d --- /dev/null +++ b/.changeset/24380-undo-response-failures.md @@ -0,0 +1,9 @@ +--- +'@objectstack/spec': minor +--- + +Clause-②: yes (widening) + +`UndoImportJobResponseSchema` gains `failures`: one `{ id, action, code, error }` entry per row an import undo did not revert, `action` being `delete` or `restore`. The `success` describe now says it is `false` when any row is listed there, and `failed` counts those entries. `revertedAt` on `ImportJobProgressSchema` and `ImportJobSummarySchema` now says it is set only once every row of the job is reverted; an undo that left rows unreverted sets nothing, and the job stays undoable for them. + +The `skipAutomations` describe on the hook context's `session` no longer names the import undo as a case that skips an app's controls for any caller. It names the system principal's import and undo, and says the import undo door is not an exception: only the system principal may take an app's declared controls off a write, and any other caller's undo runs under the object's hooks, record-change triggers and state machine. The `ExecutionContext` docs for `skipAutomations` and `skipStateMachine` say the same. diff --git a/packages/rest/src/import-undo-controls.test.ts b/packages/rest/src/import-undo-controls.test.ts index 2626ec3f3fb..31d113f2ac0 100644 --- a/packages/rest/src/import-undo-controls.test.ts +++ b/packages/rest/src/import-undo-controls.test.ts @@ -36,6 +36,7 @@ import { SqlDriver } from '@objectstack/driver-sql'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import { SysMetadataAuditObject, SysMetadataCommitObject, SysMetadataHistoryObject, SysMetadataObject } from '@objectstack/metadata-core'; import { SysImportJob } from '@objectstack/platform-objects/audit'; +import { UndoImportJobResponseSchema } from '@objectstack/spec/api'; import { RestServer } from './rest-server'; function makeSqliteDriver() { @@ -283,6 +284,8 @@ describe('[#24380] an import undo runs under the object\'s declared controls for expect(res._json.failures).toEqual([ { id: 't_closed', action: 'restore', code: 'VALIDATION_FAILED', error: 'A closed ticket stays closed.' }, ]); + // The answer is the published contract's, field for field. + expect(UndoImportJobResponseSchema.strict().safeParse(res._json).success).toBe(true); expect(await ctx.record('ou_ticket', 't_closed')).toMatchObject({ status: 'closed' }); expect(await ctx.record('ou_ticket', 't_renamed')).toMatchObject({ title: 'old name' }); From a23786da7de8d5cf45a6d4fcbf10f7b7e51cf625 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:43:37 +0000 Subject: [PATCH 5/9] test(rest): type the undo suite's engine query options Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- packages/rest/src/import-undo-controls.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/rest/src/import-undo-controls.test.ts b/packages/rest/src/import-undo-controls.test.ts index 31d113f2ac0..6fdb890c8a5 100644 --- a/packages/rest/src/import-undo-controls.test.ts +++ b/packages/rest/src/import-undo-controls.test.ts @@ -205,10 +205,10 @@ async function boot() { }, /** The job row as stored, read the way the engine-owned writer reads it. */ async jobRow(jobId: string): Promise { - return engine.findOne('sys_import_job', { where: { id: jobId }, context: { isSystem: true } } as any); + return engine.findOne('sys_import_job', { where: { id: jobId }, context: { isSystem: true } }); }, async record(object: string, id: string): Promise { - return engine.findOne(object, { where: { id }, context: { isSystem: true } } as any); + return engine.findOne(object, { where: { id }, context: { isSystem: true } }); }, /** A later edit on the live app, by a signed-in user, between the import and its undo. */ async edit(object: string, id: string, data: Record) { @@ -229,7 +229,7 @@ describe('[#24380] an import undo runs under the object\'s declared controls for /** One deal the import updates (its amount) and one it creates. */ async function dealJob(): Promise { - await ctx.engine.insert('ou_deal', { id: 'd_old', name: 'Acme renewal', amount: 100, approval_status: 'approved' }, { context: { isSystem: true } } as any); + await ctx.engine.insert('ou_deal', { id: 'd_old', name: 'Acme renewal', amount: 100, approval_status: 'approved' }, { context: { isSystem: true } }); const jobId = await ctx.importJob('ou_deal', { writeMode: 'upsert', matchFields: ['id'], rows: [{ id: 'd_old', amount: 250 }, { id: 'd_new', name: 'Globex', amount: 10 }], @@ -272,7 +272,7 @@ describe('[#24380] an import undo runs under the object\'s declared controls for await ctx.engine.insert('ou_ticket', [ { id: 't_closed', title: 'printer', status: 'open' }, { id: 't_renamed', title: 'old name', status: 'open' }, - ], { context: { isSystem: true } } as any); + ], { context: { isSystem: true } }); const jobId = await ctx.importJob('ou_ticket', { writeMode: 'update', matchFields: ['id'], rows: [{ id: 't_closed', status: 'closed' }, { id: 't_renamed', title: 'new name' }], @@ -313,7 +313,7 @@ describe('[#24380] an import undo runs under the object\'s declared controls for }); it('a delete and a restore a closed-record freeze refuses are each reported per row, and the job keeps both', async () => { - await ctx.engine.insert('ou_case', { id: 'c_old', title: 'before', status: 'open' }, { context: { isSystem: true } } as any); + await ctx.engine.insert('ou_case', { id: 'c_old', title: 'before', status: 'open' }, { context: { isSystem: true } }); const jobId = await ctx.importJob('ou_case', { writeMode: 'upsert', matchFields: ['id'], rows: [{ id: 'c_old', title: 'after' }, { id: 'c_new', title: 'fresh', status: 'open' }], @@ -354,7 +354,7 @@ describe('[#24380] an import undo runs under the object\'s declared controls for /** A historical job, created by the system principal (the only caller the import doors let send it). */ async function historicalJob(): Promise { - await ctx.engine.insert('ou_deal', { id: 'h_old', name: 'Initech', amount: 5, approval_status: 'approved' }, { context: { isSystem: true } } as any); + await ctx.engine.insert('ou_deal', { id: 'h_old', name: 'Initech', amount: 5, approval_status: 'approved' }, { context: { isSystem: true } }); ctx.as(SYSTEM); const jobId = await ctx.importJob('ou_deal', { writeMode: 'upsert', matchFields: ['id'], treatAsHistorical: true, From aabf9edb5becd3b8827fa9250fd8c94392540323 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 03:00:07 +0000 Subject: [PATCH 6/9] fix(spec)!: the undo response's required failures is declared a narrowing; ExecutionContext names runImport's in-process caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @objectstack/spec changeset now reads fix(spec)!:, Clause-②: yes (narrowing), one ADR-0087 marker and a BREAKING sentence saying who notices the required failures key. The skipAutomations TSDoc says the import doors admit the opt-out over HTTP from the system principal alone while an in-process caller of runImport passes its own setting. Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- .changeset/24380-undo-response-failures.md | 12 +++++++++--- .../spec/src/kernel/execution-context.zod.ts | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/.changeset/24380-undo-response-failures.md b/.changeset/24380-undo-response-failures.md index d9b851c719d..b6db044769a 100644 --- a/.changeset/24380-undo-response-failures.md +++ b/.changeset/24380-undo-response-failures.md @@ -2,8 +2,14 @@ '@objectstack/spec': minor --- -Clause-②: yes (widening) +fix(spec)!: `UndoImportJobResponseSchema` requires `failures`, one entry per row an import undo did not revert, and the `skipAutomations` describe names no import-undo exception -`UndoImportJobResponseSchema` gains `failures`: one `{ id, action, code, error }` entry per row an import undo did not revert, `action` being `delete` or `restore`. The `success` describe now says it is `false` when any row is listed there, and `failed` counts those entries. `revertedAt` on `ImportJobProgressSchema` and `ImportJobSummarySchema` now says it is set only once every row of the job is reverted; an undo that left rows unreverted sets nothing, and the job stays undoable for them. +Clause-②: yes (narrowing) -The `skipAutomations` describe on the hook context's `session` no longer names the import undo as a case that skips an app's controls for any caller. It names the system principal's import and undo, and says the import undo door is not an exception: only the system principal may take an app's declared controls off a write, and any other caller's undo runs under the object's hooks, record-change triggers and state machine. The `ExecutionContext` docs for `skipAutomations` and `skipStateMachine` say the same. + + +**BREAKING** (`UndoImportJobResponseSchema` accepts fewer answers: `failures` is required, so a response parsed with the schema is refused without it, and code that constructs an `UndoImportJobResponse` must set it, `[]` on a full revert; readers of `client.data.undoImportJob` are unchanged and gain the key), shipped as `minor` under the launch-window convention for breaking changes. + +`UndoImportJobResponseSchema` gains a required `failures`: one `{ id, action, code, error }` entry per row an import undo did not revert, `action` being `delete` or `restore`, and an empty array when every row was reverted. It is required rather than optional so a server that does not report its unreverted rows is off-contract instead of silently spec-valid. The `success` describe now says it is `false` when any row is listed there, and `failed` counts those entries. `revertedAt` on `ImportJobProgressSchema` and `ImportJobSummarySchema` now says it is set only once every row of the job is reverted; an undo that left rows unreverted sets nothing, and the job stays undoable for them. + +The `skipAutomations` describe on the hook context's `session` no longer names the import undo as a case that skips an app's controls for any caller. It names the system principal's import and undo, and says the import undo door is not an exception: only the system principal may take an app's declared controls off a write, and any other caller's undo runs under the object's hooks, record-change triggers and state machine. The `ExecutionContext` TSDoc for `skipAutomations` says the import runner sets it from its caller's `runAutomations: false`, which the import doors admit over HTTP from the system principal alone while an in-process caller of the runner passes its own setting, and that the import undo sets it only when the system principal undoes a job; the `skipStateMachine` TSDoc says the same of the undo. diff --git a/packages/spec/src/kernel/execution-context.zod.ts b/packages/spec/src/kernel/execution-context.zod.ts index 43a46fa66ad..6e95132c5b0 100644 --- a/packages/spec/src/kernel/execution-context.zod.ts +++ b/packages/spec/src/kernel/execution-context.zod.ts @@ -431,12 +431,14 @@ export const ExecutionContextSchema = lazySchema(() => z.object({ * is suppressed too (implies {@link skipTriggers}). Hooks registered in * code by plugins — audit, capability gates, sharing projection — carry no * metadata binding and STILL run: this flag must never bypass security or - * audit. Set by the data-import runner when the system principal opts out - * of "run automations & triggers", and by the import undo when the system - * principal undoes a job. Only the system principal may take an app's - * declared controls off a write, and neither the import doors nor the - * import undo door is an exception: any other caller's import and undo run - * under the object's hooks, record-change triggers and state machine. + * audit. Set by the data-import runner (`runImport`) when its caller opts + * out of "run automations & triggers" (`runAutomations: false`): over HTTP + * the import doors admit that opt-out from the system principal alone, and + * an in-process caller of the runner passes its own setting. Set by the + * import undo only when the system principal undoes a job: the undo door is + * not an exception to "only the system principal may take an app's declared + * controls off a write", so any other caller's undo runs under the object's + * hooks, record-change triggers and state machine. */ skipAutomations: z.boolean().optional(), @@ -480,8 +482,9 @@ export const ExecutionContextSchema = lazySchema(() => z.object({ * it from the request's explicit `treatAsHistorical` option, gated so a normal * import still walks the FSM, and the import undo sets it only when the * system principal undoes a job: any other caller's undo walks the FSM, so - * a restore it refuses is reported per row. SCOPE is identical to `seedReplay`: only the - * `state_machine` rule is skipped; every other validation still runs. + * a restore it refuses is reported per row. SCOPE is identical to + * `seedReplay`: only the `state_machine` rule is skipped; every other + * validation still runs. */ skipStateMachine: z.boolean().optional(), From 7c779806ff938b143c6fafcb1a17e88499867ded Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 03:00:07 +0000 Subject: [PATCH 7/9] test(rest): the write-response tripwire's undo row names the per-row failures it answers Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- .../src/rest-write-response-internal-fields.tripwire.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rest/src/rest-write-response-internal-fields.tripwire.test.ts b/packages/rest/src/rest-write-response-internal-fields.tripwire.test.ts index 208f86d5346..16d1e90b9d7 100644 --- a/packages/rest/src/rest-write-response-internal-fields.tripwire.test.ts +++ b/packages/rest/src/rest-write-response-internal-fields.tripwire.test.ts @@ -389,7 +389,7 @@ const DISPOSITIONS: Record = { [`POST ${D}/:object/import`]: { kind: 'no-record-echo', why: 'Import summary: counts + per-row receipts, never the written row.' }, [`POST ${D}/:object/import/jobs`]: { kind: 'no-record-echo', why: 'Job handle (id/status).' }, [`POST ${D}/import/jobs/:jobId/cancel`]: { kind: 'no-record-echo', why: 'Job state receipt.' }, - [`POST ${D}/import/jobs/:jobId/undo`]: { kind: 'no-record-echo', why: 'Undo summary (counts).' }, + [`POST ${D}/import/jobs/:jobId/undo`]: { kind: 'no-record-echo', why: 'Undo summary: counts + per-row `failures` (record id, action, code, refusal sentence), never a record body.' }, }; const keyOf = (r: { method: string; path: string }) => `${r.method} ${r.path}`; From 483069d1f44c8ce7fc65ce3cd1bb02949e9d1c0b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 03:00:07 +0000 Subject: [PATCH 8/9] fix(platform-objects): sys_import_job.undo_log says a partial undo keeps the unreverted rows with a failures list Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- packages/platform-objects/src/audit/sys-import-job.object.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/platform-objects/src/audit/sys-import-job.object.ts b/packages/platform-objects/src/audit/sys-import-job.object.ts index 3b85165f1a6..8ade239bef7 100644 --- a/packages/platform-objects/src/audit/sys-import-job.object.ts +++ b/packages/platform-objects/src/audit/sys-import-job.object.ts @@ -79,7 +79,7 @@ export const SysImportJob = ObjectSchema.create({ undo_log: Field.json({ label: 'Undo Log', required: false, - description: 'Reversal instructions ({created:[ids], updated:[{id,before}]}) captured for small non-dry-run jobs so the import can be undone', + description: 'Reversal instructions ({created:[ids], updated:[{id,before}]}) captured for small non-dry-run jobs so the import can be undone. A partial undo keeps only the unreverted rows here, with a failures list giving each one\'s id, action, code and refusal', group: 'Outcome', }), reverted_at: Field.datetime({ From 83425d5bc1cb39855f7f991131b7d0d8d3c60e69 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 03:07:21 +0000 Subject: [PATCH 9/9] chore(platform-objects): regenerate the en bundle for the undo_log description node scripts/check-i18n-bundles.mjs --filter=platform-objects --write; the translated locales keep their values (merge mode). Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude --- .../src/apps/translations/en.objects.generated.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/platform-objects/src/apps/translations/en.objects.generated.ts b/packages/platform-objects/src/apps/translations/en.objects.generated.ts index 785d046f9a3..b4567f67268 100644 --- a/packages/platform-objects/src/apps/translations/en.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/en.objects.generated.ts @@ -3007,7 +3007,7 @@ export const enObjects: NonNullable = { }, undo_log: { label: "Undo Log", - help: "Reversal instructions ({created:[ids], updated:[{id,before}]}) captured for small non-dry-run jobs so the import can be undone" + help: "Reversal instructions ({created:[ids], updated:[{id,before}]}) captured for small non-dry-run jobs so the import can be undone. A partial undo keeps only the unreverted rows here, with a failures list giving each one's id, action, code and refusal" }, reverted_at: { label: "Reverted At",