From 263a835083ac07eddcc8cd83a703b33cc48c23de Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:31:33 +0000 Subject: [PATCH 1/2] fix(automation): close the default-routable footgun on refuse-to-execute guards (#3863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3881 stopped a `fault` edge swallowing a guard refusal, keyed on `errorClass`. That field defaults to 'runtime' — right for compatibility, since every executor written before the split keeps its routing — but it leaves the footgun pointing the other way: a NEW guard is routable unless its author remembers to classify it, and forgetting is silent. `refuseNode(reason)` returns a guard-class failure, so writing a guard and marking it un-routable become one act. Its doc states the test for using it (re-running unchanged can never succeed AND the fix is to edit metadata) and the inverse, because over-marking turns a recoverable integration into a dead run. Five guards that were never marked are now un-routable — all missing required config or a defective graph, none able to succeed on a retry: - `http` with no url - `subflow` with no flowName, and subflow past max nesting depth (a recursive graph nests exactly as deep next run) - `map` with no flowName - `connector_action` with no connectorId/actionId The seven crud-nodes guards from #3881 move to the helper — same behaviour, one spelling. A behavioural inventory test drives every known guard through the engine with a fault edge attached and asserts it stays fatal, matching the refusal text so a guard failing for a different reason cannot pass vacuously. Verified to have teeth: un-marking one fails its row immediately. The negative half is pinned too — a plain failure and a thrown error must still route. Deliberately not marked: a degraded connector (#3017 recovers automatically), a collection that did not resolve to an array, a collection over the iteration cap, a subflow that failed on its own. The world caused those. Considered and rejected: making `errorClass` required on the result type — compile-time enforcement, but it breaks 281 call sites plus third-party executors for a type-only gain over the helper. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EdCvGtER9SzJopS4Yh3Pa1 --- .changeset/guard-refusal-chokepoint.md | 51 ++++ .../src/builtin/connector-nodes.ts | 8 +- .../src/builtin/crud-nodes.ts | 15 +- .../src/builtin/http-nodes.ts | 3 +- .../src/builtin/map-node.ts | 3 +- .../src/builtin/subflow-node.ts | 12 +- .../src/guard-refusal-inventory.test.ts | 240 ++++++++++++++++++ .../service-automation/src/guard-refusal.ts | 29 +++ 8 files changed, 343 insertions(+), 18 deletions(-) create mode 100644 .changeset/guard-refusal-chokepoint.md create mode 100644 packages/services/service-automation/src/guard-refusal-inventory.test.ts diff --git a/.changeset/guard-refusal-chokepoint.md b/.changeset/guard-refusal-chokepoint.md new file mode 100644 index 0000000000..a5d2af1ce2 --- /dev/null +++ b/.changeset/guard-refusal-chokepoint.md @@ -0,0 +1,51 @@ +--- +"@objectstack/service-automation": patch +--- + +fix(automation): close the default-routable footgun on refuse-to-execute guards (#3863) + +#3881 stopped a `fault` edge from swallowing a guard refusal, keyed on +`NodeExecutionResult.errorClass`. That field defaults to `'runtime'`, which was +right for compatibility — every executor written before the split keeps its +routing — but it leaves the footgun pointing the other way: **a new guard is +routable unless its author remembers to classify it**, and forgetting is silent. +Nothing in the type system catches it. + +Three changes close that for the guards that exist and make the next one hard to +get wrong. + +**`refuseNode(reason)`** — one call that returns a guard-class failure, so +"write a guard" and "mark it un-routable" become the same act. Its doc states +the test for using it: re-running unchanged can never succeed AND the fix is to +edit metadata. It also states the inverse, because over-marking is not the safe +direction — classifying a handleable condition as `guard` turns a recoverable +integration into a dead run. + +**Five guards that were never marked** are now un-routable. All are missing +required config or a defective graph, none can succeed on a retry: + +- `http` with no `url` +- `subflow` with no `config.flowName`, and `subflow` exceeding max nesting depth + (a recursive graph nests exactly as deep next run) +- `map` with no `config.flowName` +- `connector_action` with no `connectorId` / `actionId` + +The seven `crud-nodes` guards from #3881 move to the helper — same behaviour, +one spelling. + +**A behavioural inventory test** drives every known guard through the engine +with a fault edge attached and asserts it is still fatal, matching on the +refusal text so a guard failing for a different reason cannot pass vacuously. +Verified to have teeth: un-marking one guard fails its row immediately. The +negative half is pinned too — a plain node failure and a thrown error must still +route, since that is what fault edges are for. + +Deliberately **not** marked, and why: a degraded connector (#3017 says recovery +is automatic), a collection that did not resolve to an array, a collection over +the iteration cap, and a subflow that failed on its own. Those are conditions +the world caused, and an author must be able to handle them. + +Considered and rejected: making `errorClass` required on the result type. It +would enforce classification at compile time, but it breaks every node executor +returning a failure — 281 call sites across the repo plus third-party +executors — for a type-only gain over the helper. diff --git a/packages/services/service-automation/src/builtin/connector-nodes.ts b/packages/services/service-automation/src/builtin/connector-nodes.ts index 4e9d3e66f9..57664c0ad4 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.ts @@ -3,6 +3,7 @@ import type { PluginContext } from '@objectstack/core'; import { defineActionDescriptor } from '@objectstack/spec/automation'; import type { AutomationEngine, ConnectorActionContext } from '../engine.js'; +import { refuseNode } from '../guard-refusal.js'; /** * Connector built-in node — `connector_action` (generic integration dispatch). @@ -49,10 +50,9 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont async execute(node, variables, context) { const cfg = node.connectorConfig; if (!cfg?.connectorId || !cfg?.actionId) { - return { - success: false, - error: `connector_action '${node.id}': connectorConfig.connectorId and .actionId are required`, - }; + return refuseNode( + `connector_action '${node.id}': connectorConfig.connectorId and .actionId are required`, + ); } const handler = engine.resolveConnectorAction(cfg.connectorId, cfg.actionId); diff --git a/packages/services/service-automation/src/builtin/crud-nodes.ts b/packages/services/service-automation/src/builtin/crud-nodes.ts index fc54604ec3..8abc09eb5d 100644 --- a/packages/services/service-automation/src/builtin/crud-nodes.ts +++ b/packages/services/service-automation/src/builtin/crud-nodes.ts @@ -7,6 +7,7 @@ import type { DroppedFieldsEvent } from '@objectstack/spec/data'; import type { AutomationEngine } from '../engine.js'; import { interpolate, interpolateFilter, type VariableMap } from './template.js'; import { readAliasedConfig } from './config-aliases.js'; +import { refuseNode } from '../guard-refusal.js'; import { resolveRunDataContext } from '../runtime-identity.js'; /** @@ -161,7 +162,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', errorClass: 'guard' }; + if (!objectName) return refuseNode('get_record: objectName required'); // `filters` → `filter` is now handled at load by the ADR-0087 D2 // conversion layer ('flow-node-crud-filter-alias'), so the executor @@ -170,7 +171,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, errorClass: 'guard' }; + if ('error' in filterResult) return refuseNode(filterResult.error); const filter = filterResult.filter; const fields = cfg.fields as string[] | undefined; const limit = typeof cfg.limit === 'number' ? cfg.limit : undefined; @@ -222,7 +223,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', errorClass: 'guard' }; + if (!objectName) return refuseNode('create_record: objectName required'); const fields = interpolate(cfg.fields ?? {}, variables, context) as Record; const outputVariable = cfg.outputVariable as string | undefined; @@ -303,14 +304,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', errorClass: 'guard' }; + if (!objectName) return refuseNode('update_record: objectName required'); // `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, errorClass: 'guard' }; + if ('error' in filterResult) return refuseNode(filterResult.error); 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 +377,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', errorClass: 'guard' }; + if (!objectName) return refuseNode('delete_record: objectName required'); // `filters` → `filter` converted at load (ADR-0087 D2); read canonical. // The highest-stakes of the three: an erased condition here is the @@ -385,7 +386,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, errorClass: 'guard' }; + if ('error' in filterResult) return refuseNode(filterResult.error); const filter = filterResult.filter; const data = getData(); diff --git a/packages/services/service-automation/src/builtin/http-nodes.ts b/packages/services/service-automation/src/builtin/http-nodes.ts index 3aa9540dc3..b6aa7f1f40 100644 --- a/packages/services/service-automation/src/builtin/http-nodes.ts +++ b/packages/services/service-automation/src/builtin/http-nodes.ts @@ -4,6 +4,7 @@ import type { PluginContext } from '@objectstack/core'; import { defineActionDescriptor } from '@objectstack/spec/automation'; import { randomUUID } from 'node:crypto'; import type { AutomationEngine } from '../engine.js'; +import { refuseNode } from '../guard-refusal.js'; import { interpolate } from './template.js'; /** @@ -95,7 +96,7 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext): const cfg = interpolate(raw, variables, context) as Record; const url = cfg.url as string | undefined; - if (!url) return { success: false, error: 'http: url is required' }; + if (!url) return refuseNode('http: url is required'); const durable = cfg.durable === true; const headers = cfg.headers as Record | undefined; diff --git a/packages/services/service-automation/src/builtin/map-node.ts b/packages/services/service-automation/src/builtin/map-node.ts index 9f92e52555..2857470879 100644 --- a/packages/services/service-automation/src/builtin/map-node.ts +++ b/packages/services/service-automation/src/builtin/map-node.ts @@ -4,6 +4,7 @@ import type { PluginContext } from '@objectstack/core'; import { defineActionDescriptor } from '@objectstack/spec/automation'; import type { AutomationContext } from '@objectstack/spec/contracts'; import type { AutomationEngine } from '../engine.js'; +import { refuseNode } from '../guard-refusal.js'; import { interpolate } from './template.js'; /** Hard cap on map fan-out — turns a runaway collection into a clean error. */ @@ -74,7 +75,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v const flowName = typeof cfg.flowName === 'string' ? cfg.flowName : typeof cfg.flow === 'string' ? cfg.flow : undefined; if (!flowName) { - return { success: false, error: `map '${node.id}': config.flowName (the per-item subflow) is required` }; + return refuseNode(`map '${node.id}': config.flowName (the per-item subflow) is required`); } const iteratorVariable = diff --git a/packages/services/service-automation/src/builtin/subflow-node.ts b/packages/services/service-automation/src/builtin/subflow-node.ts index 49efc99bf0..6e797ee11f 100644 --- a/packages/services/service-automation/src/builtin/subflow-node.ts +++ b/packages/services/service-automation/src/builtin/subflow-node.ts @@ -5,6 +5,7 @@ import { defineActionDescriptor } from '@objectstack/spec/automation'; import type { AutomationContext } from '@objectstack/spec/contracts'; import type { AutomationEngine } from '../engine.js'; import { interpolate } from './template.js'; +import { refuseNode } from '../guard-refusal.js'; /** Hard cap on subflow nesting — turns an accidental cycle into a clean error. */ const MAX_SUBFLOW_DEPTH = 16; @@ -55,16 +56,17 @@ export function registerSubflowNode(engine: AutomationEngine, ctx: PluginContext const flowName = typeof cfg.flowName === 'string' ? cfg.flowName : typeof cfg.flow === 'string' ? cfg.flow : undefined; if (!flowName) { - return { success: false, error: `subflow '${node.id}': config.flowName is required` }; + return refuseNode(`subflow '${node.id}': config.flowName is required`); } // Cycle guard: depth rides on the context so it accumulates across nesting. const depth = Number((context as { $subflowDepth?: number } | undefined)?.$subflowDepth ?? 0); if (depth >= MAX_SUBFLOW_DEPTH) { - return { - success: false, - error: `subflow '${flowName}': max nesting depth (${MAX_SUBFLOW_DEPTH}) exceeded — recursive subflow?`, - }; + // A graph that recurses past the ceiling is a metadata defect, not a + // transient condition — the next run nests exactly as deep (#3863). + return refuseNode( + `subflow '${flowName}': max nesting depth (${MAX_SUBFLOW_DEPTH}) exceeded — recursive subflow?`, + ); } // Map inputs (resolve `{var}` against the parent's variables/context). diff --git a/packages/services/service-automation/src/guard-refusal-inventory.test.ts b/packages/services/service-automation/src/guard-refusal-inventory.test.ts new file mode 100644 index 0000000000..51089d7aea --- /dev/null +++ b/packages/services/service-automation/src/guard-refusal-inventory.test.ts @@ -0,0 +1,240 @@ +// 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'; +import { registerHttpNodes } from './builtin/http-nodes.js'; +import { registerSubflowNode } from './builtin/subflow-node.js'; +import { registerMapNode } from './builtin/map-node.js'; +import { registerConnectorNodes } from './builtin/connector-nodes.js'; +import { refuseNode } from './guard-refusal.js'; + +/** + * #3863 follow-up — the INVENTORY of refuse-to-execute guards, pinned by + * behaviour rather than by reading the source. + * + * `errorClass` defaults to `'runtime'`, which was the right call for + * compatibility (every executor written before the split keeps its routing) but + * leaves the footgun pointing the other way: a guard that forgets to classify + * itself is silently routable, and a `fault` edge then swallows it. Nothing in + * the type system catches that. + * + * So each known guard is driven through the engine WITH a fault edge attached + * and asserted still fatal. Unmarking any of them — or writing the next one + * without {@link refuseNode} and adding it here — fails loudly instead of + * quietly re-opening the hole. + * + * The negative half matters just as much: `runtime` failures listed at the + * bottom MUST stay routable. Over-marking is not the safe direction — it turns + * a condition the author can legitimately handle into a dead run. + */ + +function createTestLogger(): any { + return { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => createTestLogger() }; +} + +const dataStub = { + find: async () => [], + findOne: async () => null, + create: async () => ({ id: 'x' }), + update: async () => ({ modified: 0 }), + delete: async () => ({ deleted: 0 }), + getObject: () => ({ name: 'deal', fields: {} }), +}; + +function ctxWith(data: unknown): any { + return { + logger: createTestLogger(), + getService(name: string) { + return name === 'data' ? data : undefined; + }, + }; +} + +/** A flow whose single operative node has BOTH a normal and a fault out-edge. */ +function flowWithHandler(name: string, node: Record) { + return { + name, + label: name, + type: 'autolaunched' as const, + runAs: 'system' as const, + nodes: [ + { id: 'start', type: 'start' as const, label: 'Start' }, + { id: 'op', label: 'Op', ...node }, + { 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' }, + ], + }; +} + +/** + * Every guard that must survive a declared fault edge. `node` is the operative + * node; `expect` is a fragment of the refusal the run must fail with, so a guard + * that starts failing for a DIFFERENT reason does not pass vacuously. + */ +const GUARDS: Array<{ name: string; why: string; node: Record; expect: string }> = [ + { + name: 'get_record without objectName', + why: 'a required config key — no run can supply it', + node: { type: 'get_record', config: { filter: { id: 'x' } } }, + expect: 'objectName required', + }, + { + name: 'create_record without objectName', + why: 'a required config key', + node: { type: 'create_record', config: { fields: { a: 1 } } }, + expect: 'objectName required', + }, + { + name: 'update_record without objectName', + why: 'a required config key', + node: { type: 'update_record', config: { fields: { a: 1 } } }, + expect: 'objectName required', + }, + { + name: 'delete_record without objectName', + why: 'a required config key', + node: { type: 'delete_record', config: { filter: { id: 'x' } } }, + expect: 'objectName required', + }, + { + name: 'get_record whose filter lost a condition (#3810)', + why: 'an erased condition WIDENS the query — the reason #3810 exists', + node: { type: 'get_record', config: { objectName: 'deal', filter: { owner: '{record.ownr}' } } }, + expect: 'refusing to run', + }, + { + name: 'update_record whose filter lost a condition (#3810)', + why: 'the widened query would overwrite rows the author excluded', + node: { + type: 'update_record', + config: { objectName: 'deal', filter: { owner: '{record.ownr}' }, fields: { stage: 'x' } }, + }, + expect: 'refusing to run', + }, + { + name: 'delete_record whose filter lost a condition (#3810)', + why: 'the highest-stakes case — a collapsed filter empties the object', + node: { type: 'delete_record', config: { objectName: 'deal', filter: { owner: '{record.ownr}' } } }, + expect: 'refusing to run', + }, + { + name: 'http without url', + why: 'a required config key', + node: { type: 'http', config: { method: 'GET' } }, + expect: 'url is required', + }, + { + name: 'subflow without flowName', + why: 'a required config key', + node: { type: 'subflow', config: {} }, + expect: 'flowName is required', + }, + { + name: 'map without flowName', + why: 'a required config key — the per-item subflow', + node: { type: 'map', config: { collection: [] } }, + expect: 'flowName', + }, + { + name: 'connector_action without connectorId/actionId', + why: 'required config keys', + node: { type: 'connector_action', config: {} }, + expect: 'are required', + }, +]; + +describe('#3863 — the guard inventory stays un-routable', () => { + let engine: AutomationEngine; + + beforeEach(() => { + engine = new AutomationEngine(createTestLogger()); + const ctx = ctxWith(dataStub); + registerCrudNodes(engine, ctx); + registerHttpNodes(engine, ctx); + registerSubflowNode(engine, ctx); + registerMapNode(engine, ctx); + registerConnectorNodes(engine, ctx); + }); + + it.each(GUARDS.map((g, i) => ({ ...g, i })))( + '$name stays fatal with a fault edge ($why)', + async ({ node, expect: fragment, i }) => { + let handlerRan = false; + engine.registerNodeExecutor({ + type: 'script', + async execute() { + handlerRan = true; + return { success: true }; + }, + }); + const flowName = `guard_case_${i}`; + engine.registerFlow(flowName, flowWithHandler(flowName, node) as any); + + const result = await engine.execute(flowName, { record: { id: 'r1', owner: 'usr_7' } } as any); + + expect(result.success).toBe(false); + expect(result.error ?? '').toContain(fragment); + expect(handlerRan).toBe(false); + }, + ); +}); + +describe('#3863 — runtime failures stay routable', () => { + let engine: AutomationEngine; + + beforeEach(() => { + engine = new AutomationEngine(createTestLogger()); + }); + + /** + * The negative half of the contract. These are conditions the world caused, + * and an author must be able to handle them — marking one `guard` would turn + * a recoverable integration into a dead run. + */ + it('a plain node failure still routes to the handler', async () => { + let handlerRan = false; + engine.registerNodeExecutor({ + type: 'script', + async execute(node) { + if (node.id === 'op') return { success: false, error: 'upstream 503' }; + handlerRan = true; + return { success: true }; + }, + }); + engine.registerFlow('runtime_ok', flowWithHandler('runtime_ok', { type: 'script' }) as any); + + const result = await engine.execute('runtime_ok'); + expect(result.success).toBe(true); + expect(handlerRan).toBe(true); + }); + + it('a thrown node error still routes to the handler', async () => { + let handlerRan = false; + engine.registerNodeExecutor({ + type: 'script', + async execute(node) { + if (node.id === 'op') throw new Error('socket hang up'); + handlerRan = true; + return { success: true }; + }, + }); + engine.registerFlow('throw_ok', flowWithHandler('throw_ok', { type: 'script' }) as any); + + const result = await engine.execute('throw_ok'); + expect(result.success).toBe(true); + expect(handlerRan).toBe(true); + }); +}); + +describe('refuseNode', () => { + it('produces a guard-class failure so writing one and marking it are one act', () => { + expect(refuseNode('nope')).toEqual({ success: false, error: 'nope', errorClass: 'guard' }); + }); +}); diff --git a/packages/services/service-automation/src/guard-refusal.ts b/packages/services/service-automation/src/guard-refusal.ts index daef88709e..2ff3a61051 100644 --- a/packages/services/service-automation/src/guard-refusal.ts +++ b/packages/services/service-automation/src/guard-refusal.ts @@ -29,6 +29,35 @@ export const GUARD_REFUSAL: unique symbol = Symbol.for( 'objectstack.automation.guardRefusal', ) as never; +/** + * Refuse a node because its METADATA is wrong — the one call an executor should + * make instead of hand-writing `{ success: false, … }` for that case. + * + * `errorClass` defaults to `'runtime'` so executors written before #3863 keep + * their routing, which leaves a footgun pointing the other way: a NEW + * refuse-to-execute guard is routable unless its author remembers to classify + * it, and forgetting is silent. This helper closes that by making "write a + * guard" and "mark it un-routable" the same act — there is nothing to remember + * and nothing extra to type. + * + * Use it when BOTH hold: + * 1. re-running the flow unchanged can never succeed, and + * 2. the fix is to edit metadata (a missing required config key, a graph that + * recurses, a filter whose condition interpolation erased). + * + * If either is false the failure is a `runtime` one — the world did not + * cooperate — and it must stay routable so authors can handle it. A connector + * that is degraded, a collection that arrived the wrong shape, a subflow that + * failed on its own: return those as plain `{ success: false }`. Over-marking + * is not the safe direction; it turns a handleable condition into a dead run. + * + * @example + * if (!url) return refuseNode('http: url is required'); + */ +export function refuseNode(reason: string): { success: false; error: string; errorClass: 'guard' } { + return { success: false, error: reason, errorClass: 'guard' }; +} + /** 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, { From eb14ab37fd92e30b76218764a1f3037f3ddbe61b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:33:54 +0000 Subject: [PATCH 2/2] docs: state the guard/runtime test, not a closed list of guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #3881 callout in flows.mdx and the automation skill enumerated the guards that existed at the time. This PR adds five more, which makes an enumeration the wrong shape — it was already stale by the end of its own branch. Both now state the TEST instead: a failure is a guard when re-running unchanged could never succeed AND the fix is to edit metadata. The examples stay, as examples rather than as the definition, and the runtime side gains the cases this PR deliberately left routable (a degraded connector, a subflow that failed on its own) so the two lists read as one contract. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EdCvGtER9SzJopS4Yh3Pa1 --- content/docs/automation/flows.mdx | 21 ++++++++++++++------- skills/objectstack-automation/SKILL.md | 19 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 4ce9b725d5..e9a6e68919 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -587,15 +587,22 @@ 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 +- **Runtime** — the world did not cooperate, and a later run could succeed: an + `http` node got a 404, a connector rate-limited or is degraded, the data engine + rejected a write, a collection arrived the wrong shape, a subflow failed on its + own. Routed to the fault edge. +- **Guard** — a refuse-to-execute check found the *metadata* wrong. **Never + routed:** these stay fatal whether or not a `fault` edge exists, and the run fails with the guard's own message. +A failure is a guard when both hold: re-running the flow unchanged could never +succeed, **and** the fix is to edit metadata. In practice that is a missing +required config key (a data node with no `objectName`, an `http` node with no +`url`, a `subflow` or `map` with no `flowName`, a `connector_action` with no +`connectorId`/`actionId`), a filter token that resolved to nothing so the +condition was dropped from the query, a graph that recurses past the nesting +ceiling, or a run that would execute unscoped. + 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 diff --git a/skills/objectstack-automation/SKILL.md b/skills/objectstack-automation/SKILL.md index bb12b20139..b68cd8b8de 100644 --- a/skills/objectstack-automation/SKILL.md +++ b/skills/objectstack-automation/SKILL.md @@ -144,13 +144,18 @@ variables: [ > 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. +> rate-limit, a rejected write, a subflow that failed on its own. A *guard* +> refusal stays fatal with or without a fault edge. A failure is a guard when +> re-running unchanged could never succeed **and** the fix is to edit metadata: +> a missing required config key (`objectName`, `url`, `flowName`, +> `connectorId`/`actionId`), a filter token that resolved to nothing so the +> condition was dropped, a graph that recurses past the nesting ceiling, or a run +> that would execute unscoped. +> +> Never add a fault edge 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