diff --git a/.changeset/fault-edge-guard-containment.md b/.changeset/fault-edge-guard-containment.md new file mode 100644 index 0000000000..9b81159930 --- /dev/null +++ b/.changeset/fault-edge-guard-containment.md @@ -0,0 +1,51 @@ +--- +"@objectstack/service-automation": minor +--- + +fix(automation): a `fault` edge must not switch off a guardrail (#3863) + +A `fault` edge routes a failed node to a handler instead of aborting the run. +That is the right primitive for the world not cooperating — an `http` node that +404s, a connector that rate-limited, a rejected write. + +It was also, until now, routing the **refuse-to-execute** family. Those guards +report that the METADATA is wrong, not that an operation failed: #3810 +(interpolation erased a filter condition), ADR-0049/#1888 (the run would execute +unscoped), a data node naming no object. Because they surfaced as ordinary node +failures, one declared edge silently disabled them. + +**The live consequence, reproduced in a test before the fix:** attach a `fault` +edge to a `delete_record` whose filter has a typo (`{record.ownr}`), and #3810's +protection against emptying the object was gone — the guard fired, the handler +swallowed it, and the run reported `success: true`. That is the exact fail-open +direction #3810 was opened to close, reachable from a single edge, and it is the +kind of suppression an AI authoring loop reaches for first when trying to make a +diagnostic go away. + +**Failures now carry a class.** `NodeExecutionResult.errorClass` is `'runtime'` +(default — every existing executor keeps its current routing) or `'guard'`. +Guard-class failures are never routed: they stay fatal with or without a `fault` +edge, and the run fails with the guard's own message. Thrown guards are covered +too — `UnscopedRunDataAccessError` is branded via a shared `guard-refusal` +module, so the engine's catch path cannot become the bypass the return path no +longer is. + +Marked as guard-class: the three `resolveNodeFilter` refusals (#3810), the four +`objectName required` refusals, and `UnscopedRunDataAccessError` (ADR-0049). +Genuine engine failures (`get_record(x) failed: …`) stay runtime-class and keep +routing. + +**Also in this change** + +- `{.error}` now carries a failed node's message alongside the run-wide + `{$error}`. `$error` names only the most recent failure, so a handler shared by + two fault edges could not tell which node it was handling; `{charge_card.error}` + is addressable from any downstream template. Additive — `$error` is unchanged. +- Fault edges are **documented** for the first time (`content/docs/automation/flows.mdx` + and the automation skill), including the routable/not-routable split. The skill + entry says plainly not to add a fault edge to silence a guard error, since that + is the misuse the class split now makes impossible. + +A run that takes a fault branch still reports success, and the failed step still +carries `status: 'failure'` and its message in the trace — recovery does not +erase the record of what failed (#3356/#3407). diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 2d9f53de66..4ce9b725d5 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -560,6 +560,49 @@ Edges connect nodes and define the execution path: | `condition` | `string` | optional | Boolean expression for branching | | `label` | `string` | optional | Label displayed on the connector | +### Fault edges — handling a failed node + +A `fault` edge routes a **failed** node to a handler instead of aborting the +run. Without one, any node failure ends the run. + +```typescript +edges: [ + { id: 'e_ok', source: 'charge_card', target: 'mark_paid' }, + { id: 'e_fault', source: 'charge_card', target: 'flag_for_review', type: 'fault' }, +] +``` + +The handler reads what went wrong from two variables: + +| Variable | Scope | Use | +| :--- | :--- | :--- | +| `{.error}` | the failing node | `{charge_card.error}` — addressable by name, so one handler shared by several fault edges can tell which node it is handling | +| `{$error}` | run-wide | `{$error.nodeId}` / `{$error.message}` — the most recent failure only | + +A run that takes a fault branch and completes reports **success**, but the +failed step stays in the run trace with `status: 'failure'` and its message — +recovery never erases the record of what failed. + + +**A fault edge does not disable a guardrail.** Failures come in two classes, and +only one is routable. + +- **Runtime** — the world did not cooperate: an `http` node got a 404, a + connector rate-limited, the data engine rejected a write. Routed to the fault + edge. +- **Guard** — the *metadata* is wrong, and a refuse-to-execute check said so: a + filter token resolved to nothing so the condition was dropped from the query, + a data node names no object, or the run would execute unscoped. **Never + routed.** These stay fatal whether or not a `fault` edge exists, and the run + fails with the guard's own message. + +The split is deliberate. A dropped filter condition does not narrow a query, it +widens it — so if guards were routable, one `fault` edge on a `delete_record` +would turn off the protection against emptying the object while the run still +reported success. Re-running changes nothing either: the fix is to correct the +metadata, which `objectstack validate` will point at. + + ## Variables Flows use variables to pass data between nodes and to/from callers: diff --git a/packages/services/service-automation/src/builtin/crud-nodes.ts b/packages/services/service-automation/src/builtin/crud-nodes.ts index dad7e02628..fc54604ec3 100644 --- a/packages/services/service-automation/src/builtin/crud-nodes.ts +++ b/packages/services/service-automation/src/builtin/crud-nodes.ts @@ -161,7 +161,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; const objectName = String(readAliasedConfig(cfg, 'get_record', 'objectName', ['object'], ctx.logger) ?? ''); - if (!objectName) return { success: false, error: 'get_record: objectName required' }; + if (!objectName) return { success: false, error: 'get_record: objectName required', errorClass: 'guard' }; // `filters` → `filter` is now handled at load by the ADR-0087 D2 // conversion layer ('flow-node-crud-filter-alias'), so the executor @@ -170,7 +170,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): cfg.filter, variables, context, 'get_record', 'would have read rows the filter was written to exclude', ); - if ('error' in filterResult) return { success: false, error: filterResult.error }; + if ('error' in filterResult) return { success: false, error: filterResult.error, errorClass: 'guard' }; const filter = filterResult.filter; const fields = cfg.fields as string[] | undefined; const limit = typeof cfg.limit === 'number' ? cfg.limit : undefined; @@ -222,7 +222,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; const objectName = String(readAliasedConfig(cfg, 'create_record', 'objectName', ['object'], ctx.logger) ?? ''); - if (!objectName) return { success: false, error: 'create_record: objectName required' }; + if (!objectName) return { success: false, error: 'create_record: objectName required', errorClass: 'guard' }; const fields = interpolate(cfg.fields ?? {}, variables, context) as Record; const outputVariable = cfg.outputVariable as string | undefined; @@ -303,14 +303,14 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; const objectName = String(readAliasedConfig(cfg, 'update_record', 'objectName', ['object'], ctx.logger) ?? ''); - if (!objectName) return { success: false, error: 'update_record: objectName required' }; + if (!objectName) return { success: false, error: 'update_record: objectName required', errorClass: 'guard' }; // `filters` → `filter` converted at load (ADR-0087 D2); read canonical. const filterResult = resolveNodeFilter( cfg.filter, variables, context, 'update_record', 'would have matched — and overwritten — rows the filter was written to exclude', ); - if ('error' in filterResult) return { success: false, error: filterResult.error }; + if ('error' in filterResult) return { success: false, error: filterResult.error, errorClass: 'guard' }; const filter = filterResult.filter; // `fields` is the single canonical write-map key — no alias (the wrong key // `fieldValues` is corrected at the authoring source + rejected by graph-lint). @@ -376,7 +376,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): async execute(node, variables, context) { const cfg = (node.config ?? {}) as Record; const objectName = String(readAliasedConfig(cfg, 'delete_record', 'objectName', ['object'], ctx.logger) ?? ''); - if (!objectName) return { success: false, error: 'delete_record: objectName required' }; + if (!objectName) return { success: false, error: 'delete_record: objectName required', errorClass: 'guard' }; // `filters` → `filter` converted at load (ADR-0087 D2); read canonical. // The highest-stakes of the three: an erased condition here is the @@ -385,7 +385,7 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): cfg.filter, variables, context, 'delete_record', 'would have matched every remaining row and deleted it', ); - if ('error' in filterResult) return { success: false, error: filterResult.error }; + if ('error' in filterResult) return { success: false, error: filterResult.error, errorClass: 'guard' }; const filter = filterResult.filter; const data = getData(); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 12f0c6ad1a..3f254db916 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -18,6 +18,7 @@ import { ConnectorSchema } from '@objectstack/spec/integration'; // engine at module load in both ESM and CJS builds. import { ExpressionEngine, validateExpression } from '@objectstack/formula'; import { runIsUnscopedUserMode, flowTouchesData } from './runtime-identity.js'; +import { isGuardRefusal } from './guard-refusal.js'; // ─── Node Executor Interface (Plugin Extension Point) ─────────────── @@ -52,10 +53,37 @@ export interface NodeExecutor { ): Promise; } +/** + * Why a node failed — the question a `fault` edge's routing decision turns on + * (#3863). + * + * - `runtime` — the world did not cooperate: an `http` node got a 404, a + * connector rate-limited, the data engine rejected a write. The metadata is + * fine and a later run could succeed. A declared `fault` edge routes it. + * - `guard` — the METADATA is wrong, and a refuse-to-execute guard said so: + * interpolation erased a filter condition (#3810), a data node names no + * object, a run would execute unscoped (ADR-0049/#1888). Re-running changes + * nothing, and the refusal IS the safety property. Not routable — it stays + * fatal whether or not a `fault` edge exists. + * + * The split exists because without it a `fault` edge is a one-edge switch that + * turns off the platform's data-safety guarantees: attach one to a + * `delete_record` and #3810's protection against emptying the object is gone, + * while the run still reports success. Absent, this defaults to `runtime`, so + * every executor written before the field keeps its current routing behaviour. + */ +export type NodeFailureClass = 'runtime' | 'guard'; + export interface NodeExecutionResult { success: boolean; output?: Record; error?: string; + /** + * #3863 — why this failed, which decides whether a `fault` edge may route it. + * Only meaningful when `success` is false; defaults to `runtime`. + * See {@link NodeFailureClass}. + */ + errorClass?: NodeFailureClass; /** * #3407: advisory warnings surfaced on the step's log entry. The step still * SUCCEEDS — a warning flags a legal-but-surprising outcome (e.g. an @@ -2777,6 +2805,24 @@ export class AutomationEngine implements IAutomationService { } } + /** + * #3863 — publish a failed node's message under `.error` alongside + * the run-wide `$error`. + * + * `$error` names only the most recent failure, so a flow with more than one + * `fault` edge converging on a shared handler cannot tell which node it is + * handling. Keying by node id makes `{cleanup.error}` addressable from any + * downstream template, which is what a handler needs to branch or report. + * Merges into an existing entry so a node's earlier `output` survives. + */ + private setNodeError(variables: Map, nodeId: string, message: string): void { + const prior = variables.get(nodeId); + const base = prior && typeof prior === 'object' && !Array.isArray(prior) + ? (prior as Record) + : {}; + variables.set(nodeId, { ...base, error: message }); + } + /** * Execute a node with timeout support, fault edge handling, and step logging. */ @@ -2859,10 +2905,16 @@ export class AutomationEngine implements IAutomationService { error: { code: 'EXECUTION_ERROR', message: errMsg }, }); - // Check for fault edges - const faultEdge = flow.edges.find(e => e.source === node.id && e.type === 'fault'); + // #3863 — a guard that THROWS is as un-routable as one that + // returns: `UnscopedRunDataAccessError` (ADR-0049/#1888) reports + // that the metadata would run unscoped, and rerouting it would + // let a `fault` edge disable the elevation check. + const faultEdge = isGuardRefusal(execErr) + ? undefined + : flow.edges.find(e => e.source === node.id && e.type === 'fault'); if (faultEdge) { variables.set('$error', { nodeId: node.id, message: errMsg }); + this.setNodeError(variables, node.id, errMsg); const faultTarget = flow.nodes.find(n => n.id === faultEdge.target); if (faultTarget) { await this.executeNode(faultTarget, flow, variables, context, steps); @@ -2887,9 +2939,16 @@ export class AutomationEngine implements IAutomationService { // Write error output to variable context for downstream nodes variables.set('$error', { nodeId: node.id, message: errMsg, output: result.output }); - - // Check for fault edges - const faultEdge = flow.edges.find(e => e.source === node.id && e.type === 'fault'); + this.setNodeError(variables, node.id, errMsg); + + // #3863 — only a `runtime` failure may be routed. A `guard` + // refusal says the METADATA is wrong (#3810 erased a filter + // condition, a data node names no object); rerouting it would + // make a single `fault` edge a switch that turns the guard off + // while the run still reports success. + const faultEdge = result.errorClass === 'guard' + ? undefined + : flow.edges.find(e => e.source === node.id && e.type === 'fault'); if (faultEdge) { const faultTarget = flow.nodes.find(n => n.id === faultEdge.target); if (faultTarget) { diff --git a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts new file mode 100644 index 0000000000..a77b334004 --- /dev/null +++ b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts @@ -0,0 +1,289 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { AutomationEngine } from './engine.js'; +import { registerCrudNodes } from './builtin/crud-nodes.js'; + +/** + * #3863 — a `fault` edge routes a node failure to a handler instead of aborting + * the run. That is the right primitive for the world not cooperating: an `http` + * node that 404s, a query that matched nothing, a connector that rate-limited. + * + * It must NOT be the escape hatch for a GUARD. The refuse-to-execute family — + * #3810 (interpolation erased a filter condition), #3425 (a readonly write is a + * certain no-op), ADR-0049/#1888 (a scheduled run is unscoped) — reports "the + * metadata is wrong", not "the operation failed". Routing those would hand every + * author (and every AI authoring loop) a one-edge switch that silently disables + * the platform's data-safety guarantees: attach a fault edge to a + * `delete_record`, and #3810's protection against emptying the object is gone + * while the run still reports success. + * + * So failures carry a CLASS. `runtime` failures are routable; `guard` failures + * stay fatal, fault edge or not, and the run fails with the guard's own message. + */ + +function createTestLogger(): any { + return { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => createTestLogger() }; +} + +/** A data engine stub that records whether a destructive verb was ever reached. */ +function fakeData() { + const seen: { deleted: boolean; where?: unknown } = { deleted: false }; + const service = { + find: async () => [], + findOne: async () => null, + update: async () => ({ modified: 0 }), + delete: async (_o: string, o: any) => { + seen.deleted = true; + seen.where = o?.where; + return { deleted: 1 }; + }, + getObject: () => ({ name: 'deal', fields: {} }), + }; + return { service, seen }; +} + +function ctxWith(data: any): any { + return { + logger: createTestLogger(), + getService(name: string) { + return name === 'data' ? data : undefined; + }, + }; +} + +describe('#3863 — a fault edge must not swallow a guard refusal', () => { + let engine: AutomationEngine; + let data: ReturnType; + + beforeEach(() => { + engine = new AutomationEngine(createTestLogger()); + data = fakeData(); + registerCrudNodes(engine, ctxWith(data.service)); + }); + + /** + * The exact shape #3810 exists to stop, with a fault edge bolted on: + * `{record.ownr}` is a typo, so the only condition is erased and the filter + * would collapse to `{}` — every row in the object. + */ + function guardedDeleteWithHandler() { + return { + name: 'guarded_delete', + label: 'Guarded Delete', + type: 'autolaunched' as const, + runAs: 'system' as const, + nodes: [ + { id: 'start', type: 'start' as const, label: 'Start' }, + { + id: 'op', + type: 'delete_record' as any, + label: 'Delete', + config: { objectName: 'deal', filter: { owner: '{record.ownr}' } }, + }, + { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'end', type: 'end' as const, label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'op' }, + { id: 'e2', source: 'op', target: 'end' }, + { id: 'e_fault', source: 'op', target: 'handler', type: 'fault' as const }, + { id: 'e3', source: 'handler', target: 'end' }, + ], + }; + } + + it('a guard refusal stays fatal even when a fault edge is declared', async () => { + let handlerRan = false; + engine.registerNodeExecutor({ + type: 'script', + async execute() { + handlerRan = true; + return { success: true }; + }, + }); + engine.registerFlow('guarded_delete', guardedDeleteWithHandler()); + + const result = await engine.execute('guarded_delete', { + record: { id: 'r1', owner: 'usr_7' }, + } as any); + + // The run must FAIL, and it must fail with the guard's own diagnosis — + // not be quietly rerouted into the handler. + expect(result.success).toBe(false); + expect(result.error).toContain('delete_record'); + expect(handlerRan).toBe(false); + // And above all: nothing was deleted. + expect(data.seen.deleted).toBe(false); + }); + + it('a runtime failure on the same node still routes to the handler', async () => { + // Same flow shape, but the node fails for a reason the world caused + // rather than a defect in the metadata. This is what fault edges are for, + // and it must keep working — the fix is a split, not a removal. + let handlerRan = false; + engine.registerNodeExecutor({ + type: 'script', + async execute(node) { + if (node.id === 'risky') return { success: false, error: 'upstream 503' }; + handlerRan = true; + return { success: true }; + }, + }); + engine.registerFlow('runtime_fail', { + name: 'runtime_fail', + label: 'Runtime Fail', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'risky', type: 'script' as any, label: 'Risky' }, + { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'risky' }, + { id: 'e2', source: 'risky', target: 'end' }, + { id: 'e_fault', source: 'risky', target: 'handler', type: 'fault' }, + { id: 'e3', source: 'handler', target: 'end' }, + ], + }); + + const result = await engine.execute('runtime_fail'); + expect(result.success).toBe(true); + expect(handlerRan).toBe(true); + }); + + it('a THROWN guard refusal is un-routable too (ADR-0049 unscoped run)', async () => { + // The #3810 guard RETURNS its refusal; the ADR-0049/#1888 scoping guard + // THROWS one. Both must be contained, or the catch path becomes the + // bypass the return path no longer is. `runAs` is left at the default + // 'user' with no trigger user, which is exactly the unscoped case. + let handlerRan = false; + engine.registerNodeExecutor({ + type: 'script', + async execute() { + handlerRan = true; + return { success: true }; + }, + }); + engine.registerFlow('unscoped_delete', { + name: 'unscoped_delete', + label: 'Unscoped Delete', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'op', + type: 'delete_record' as any, + label: 'Delete', + config: { objectName: 'deal', filter: { status: 'closed' } }, + }, + { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'op' }, + { id: 'e2', source: 'op', target: 'end' }, + { id: 'e_fault', source: 'op', target: 'handler', type: 'fault' }, + { id: 'e3', source: 'handler', target: 'end' }, + ], + }); + + const result = await engine.execute('unscoped_delete'); + + expect(result.success).toBe(false); + expect(result.error).toContain('runAs'); + expect(handlerRan).toBe(false); + expect(data.seen.deleted).toBe(false); + }); + + it('publishes the failing node id as {.error} for the handler', async () => { + // `$error` names only the LAST failure, so a handler shared by two fault + // edges cannot tell which node it is handling. Keying by node id makes + // that addressable. + let seenNodeScoped: unknown; + let seenGlobal: unknown; + engine.registerNodeExecutor({ + type: 'script', + async execute(node, variables) { + if (node.id === 'risky') return { success: false, error: 'upstream 503' }; + seenNodeScoped = variables.get('risky'); + seenGlobal = variables.get('$error'); + return { success: true }; + }, + }); + engine.registerFlow('node_scoped_error', { + name: 'node_scoped_error', + label: 'Node Scoped Error', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'risky', type: 'script' as any, label: 'Risky' }, + { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'risky' }, + { id: 'e2', source: 'risky', target: 'end' }, + { id: 'e_fault', source: 'risky', target: 'handler', type: 'fault' }, + { id: 'e3', source: 'handler', target: 'end' }, + ], + }); + + await engine.execute('node_scoped_error'); + expect((seenNodeScoped as any)?.error).toBe('upstream 503'); + // The pre-existing run-wide variable is unchanged — additive, not a swap. + expect((seenGlobal as any)?.message).toBe('upstream 503'); + }); + + it('records the failure on the step even when the fault branch completes', async () => { + // #3356/#3407 lesson: a run that recovered must not read as if nothing + // went wrong. The run succeeds, but the failed step stays in the trace. + engine.registerNodeExecutor({ + type: 'script', + async execute(node) { + if (node.id === 'risky') return { success: false, error: 'upstream 503' }; + return { success: true }; + }, + }); + engine.registerFlow('traced', { + name: 'traced', + label: 'Traced', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'risky', type: 'script' as any, label: 'Risky' }, + { id: 'handler', type: 'script' as any, label: 'Handler' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'risky' }, + { id: 'e2', source: 'risky', target: 'end' }, + { id: 'e_fault', source: 'risky', target: 'handler', type: 'fault' }, + { id: 'e3', source: 'handler', target: 'end' }, + ], + }); + + const result = await engine.execute('traced'); + expect(result.success).toBe(true); + + const runs = await engine.listRuns('traced'); + const riskyStep = runs[0]?.steps?.find((s: any) => s.nodeId === 'risky'); + expect(riskyStep?.status).toBe('failure'); + expect(riskyStep?.error?.message).toBe('upstream 503'); + }); + + it('a guard refusal without any fault edge is fatal, as before', async () => { + const flow = guardedDeleteWithHandler(); + flow.name = 'no_handler'; + flow.edges = flow.edges.filter((e) => e.type !== 'fault'); + engine.registerFlow('no_handler', flow); + + const result = await engine.execute('no_handler', { + record: { id: 'r1', owner: 'usr_7' }, + } as any); + + expect(result.success).toBe(false); + expect(data.seen.deleted).toBe(false); + }); +}); diff --git a/packages/services/service-automation/src/guard-refusal.ts b/packages/services/service-automation/src/guard-refusal.ts new file mode 100644 index 0000000000..daef88709e --- /dev/null +++ b/packages/services/service-automation/src/guard-refusal.ts @@ -0,0 +1,50 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The "this failure is a guard refusal" marker (#3863). + * + * Lives in its own module because both ends need it and they already form a + * chain: `engine.ts` imports `runtime-identity.ts`, so the guard that throws + * cannot import the engine back without a cycle. + * + * ## Why a refusal is marked at all + * + * A `fault` edge routes a failed node to a handler instead of aborting the run. + * That is the right primitive for the world not cooperating — a 404, a + * rate-limit, a query that matched nothing. + * + * It is the wrong primitive for the refuse-to-execute family (#3810 erased a + * filter condition, ADR-0049/#1888 a run would execute unscoped). Those report + * that the METADATA is wrong; re-running changes nothing, and the refusal is + * the safety property itself. If they routed, one `fault` edge on a + * `delete_record` would disable #3810's protection against emptying the object + * while the run still reported success — a one-edge switch for turning the + * platform's data-safety guarantees off, which is exactly the kind of + * suppression an AI authoring loop reaches for first. + * + * Returned failures carry `errorClass: 'guard'` on the node result. THROWN ones + * carry this symbol instead, and the engine's catch path honours both. + */ +export const GUARD_REFUSAL: unique symbol = Symbol.for( + 'objectstack.automation.guardRefusal', +) as never; + +/** Brand `err` as a guard refusal, so a `fault` edge will not route it. */ +export function markGuardRefusal(err: E): E { + Object.defineProperty(err, GUARD_REFUSAL, { + value: true, + enumerable: false, + configurable: false, + writable: false, + }); + return err; +} + +/** True when `err` was raised by a refuse-to-execute guard. */ +export function isGuardRefusal(err: unknown): boolean { + return ( + !!err + && typeof err === 'object' + && (err as Record)[GUARD_REFUSAL] === true + ); +} diff --git a/packages/services/service-automation/src/runtime-identity.ts b/packages/services/service-automation/src/runtime-identity.ts index 8d94ecb470..ca15dd50c5 100644 --- a/packages/services/service-automation/src/runtime-identity.ts +++ b/packages/services/service-automation/src/runtime-identity.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import type { AutomationContext } from '@objectstack/spec/contracts'; +import { markGuardRefusal } from './guard-refusal.js'; /** * The IDENTITY envelope a flow's data nodes pass to ObjectQL as @@ -90,6 +91,10 @@ export class UnscopedRunDataAccessError extends Error { `(a write made with a system context carries none). (ADR-0049, #1888, #3760)`, ); this.name = 'UnscopedRunDataAccessError'; + // #3863 — a guard refusal, so a `fault` edge must not route it. Elevation + // is the thing being refused; letting a handler swallow it would turn one + // declared edge into an opt-out from the ADR-0049 scoping check. + markGuardRefusal(this); } } diff --git a/skills/objectstack-automation/SKILL.md b/skills/objectstack-automation/SKILL.md index e8caf3a3e6..bb12b20139 100644 --- a/skills/objectstack-automation/SKILL.md +++ b/skills/objectstack-automation/SKILL.md @@ -138,6 +138,20 @@ variables: [ > (a single call updates *every* matching row — no per-row loop needed). > `label` is **required** on the flow and on every node. +> **Handling a failed node: a `fault` edge.** `{ source, target, type: 'fault' }` +> routes a failed node to a handler instead of ending the run; the handler reads +> `{.error}` (or run-wide `{$error}`). The run then reports success, and +> the failed step stays in the trace. +> +> **It is not a way past a guardrail.** Only *runtime* failures route — a 404, a +> rate-limit, a rejected write. A *guard* refusal means the metadata is wrong (a +> filter token resolved to nothing so the condition was dropped; a data node +> names no object; the run would execute unscoped) and stays fatal with or +> without a fault edge. Never add one to silence such an error: a dropped filter +> condition **widens** the query, so routing it would let a `delete_record` +> empty the object while the run reported success. Fix the metadata — +> `objectstack validate` names the offending template. + > **Writing a `readonly` field? Set `runAs: 'system'`.** `readonly: true` > governs the end-user surface: under the default `runAs: 'user'`, the engine > **silently strips** a `readonly` field from an `update_record` payload