fix(automation): create_record surfaces the engine's DUPLICATE_RECORD code - #14948

Fixes #14419

What

create_record collapsed every data.insert() failure into one opaque string
(literally create_record(OBJECT_NAME) failed: MESSAGE_TEXT), so a flow's
only two error-handling primitives — try_catch and a fault edge — could
not tell "the row is already there" from "the store is down". engine.insert
(#14095) already raises DuplicateRecordError — code: 'DUPLICATE_RECORD'
(ADR-0112), driver-independent — but the executor's catch threw it away.

Per the ruling of record (issue comments 5505584657 / 5506297499):

• Carry the engine's code on the node result, beside the existing errorClass.
• No new NodeFailureClass member — that vocabulary is authoring-surface, not this repair's call.
• create_record only — update_record / delete_record collapse identically, but engine.update still leaks the raw driver error (engine.update still leaks the raw driver unique-violation error — the same defect one verb over from the insert door, with the whole UPDATE statement as its message #14390, not yet fixed); a code those verbs never set would be worse than the string they have now.
• Pin the discrimination, not the field.

Changes

• NodeExecutionResult gains an optional code field (string, beside errorClass) — packages/services/service-automation/src/engine.ts.
• create_record's catch sets it when the caught error carries the platform's classified DUPLICATE_RECORD code — packages/services/service-automation/src/builtin/crud-nodes.ts. This reads code duck-typed off the thrown value (matching DuplicateRecordError's own documented contract — branch on the code string, not on a driver dialect or the message), never importing @objectstack/objectql's DuplicateRecordError class: objectql is this package's devDependency only (check:undeclared-dep-imports catches the alternative), because this executor runs against any IDataEngine, not a concrete engine. Forwarded narrowly — only when it equals DUPLICATE_RECORD, not any code an as-yet-unaudited driver error might someday carry.
• AutomationEngine.executeNode copies code onto the $error run variable beside message, so a directly-declared fault edge handler can read it off $error.
• try_catch's executor now preserves that code across its own errorVariable binding — previously it reconstructed a plain node-id-plus-message object from the caught exception's message alone and silently dropped whatever the engine had already written to $error, which is what made the catch region unable to discriminate at all. Patch round 1 adds an identity guard here — see below.
• One doc line-rot repair (content/docs/permissions/system-context.mdx), mechanical: node scripts/check-system-context-census.mjs --fix re-anchored a line-number citation my new import line shifted by one.
• Patch round 1: the hand-written content/docs/automation/flows.mdx $error table row now mentions {$error.code} alongside {$error.nodeId} / {$error.message}, with the "absent, not universal" caveat. content/docs/references/automation/control-flow.mdx is untouched — it is generated from the spec describe() and moves only when spec does (see Patch round 1 below).

Why try-catch-node.ts and engine.ts, not just crud-nodes.ts

The dispatch named crud-nodes.ts as where the collapse lives, and that is
still the only place a code gets SET. The other two files are read paths
for that same value, and both are necessary for the fix to be anything more
than a field nobody can reach:

• engine.ts, NodeExecutionResult: this is the declaration site the
  ruling means by "the node result" — code has to be declared somewhere,
  and this interface (already carrying errorClass) is it. No behavioural
  change here, a type addition only.
• engine.ts, executeNode's $error write: separate from the
  declaration, this is the ONE place the engine turns a failing node's
  result into something a flow can read ($error, used by a directly
  attached fault edge handler, and read by try_catch before it forms its
  own binding — see below). Before this change it copied nodeId and
  message only; code was set on the result but never reached $error,
  so nothing downstream could have seen it regardless of crud-nodes.ts.
  This line already existed and already builds this object — the diff adds
  one field to it.
• try-catch-node.ts: this file already existed and already
  OVERWRITES $error (or a custom errorVariable) with its own
  reconstruction — { nodeId, message } — built from the caught
  exception's .message string alone, discarding whatever the engine had
  just written above. Left unpatched, this is the exact mechanism that
  would have made the fix inert for the one primitive the ruling names
  explicitly as the discrimination target: "a flow with a try_catch that
  swallows the duplicate and re-raises a store failure". Without this
  change, code would reach $error at the failing node's own level and
  then be destroyed the moment try_catch's catch region tries to read it.
  The change is a preservation, not new logic: read the code already
  sitting on $error (from the write above) before it gets overwritten,
  carry it into the new object. Patch round 1 tightened this same
  preservation — read on.

On the fence — ⛔ do not add a NodeFailureClass member: neither change
touches it. NodeFailureClass ('runtime' | 'guard') is untouched, still
exactly two members, and the routing decision that reads it
(result.errorClass === 'guard' ? undefined : …) is byte-identical to
before this PR. code is a new, independent field of a different type
(string, not the NodeFailureClass union) — it is not read by, does not
feed, and does not widen that vocabulary anywhere in this diff. A flow
branches on code the same way it already branches on any other flow
variable value ($record.status, $error.message, …) — nothing here adds
a new thing an author matches on in the sense the ruling was fencing off
(no new label a decision node's conditions can name that didn't already
exist, no new edge type, no new node-level routing primitive).

Why this earns the card, not just the field

The ruling is explicit that asserting result.code equals DUPLICATE_RECORD
is not the bar — that passes with a fault edge that still cannot branch. The
new test file adds a real flow: try_catch wraps create_record; the catch
region reads the bound error's code and either swallows (duplicate) or
re-raises (anything else); the outer flow has a plain edge AND a fault edge
off the try_catch node.

try:   create_record(lead, { email })
catch: code equals DUPLICATE_RECORD → swallow, continue (plain edge)
       otherwise                   → re-raise (fault edge routes)

Two tests run the same flow shape with two different data.insert failures
and assert on which nodes actually ran:

• DuplicateRecordError thrown → swallowed, then after (plain edge, try_catch succeeds)
• generic Error thrown → reraised, then escalate (fault edge, try_catch fails)

An ablation (predicted, then measured, then restored — see Tests) confirms
this is load-bearing: stripping the code field collapses BOTH cases onto the
same path.

Patch round 1 (tier contract review — three required patches)

P1, a correctness defect the review reproduced. try_catch reads code
off the run-wide $error, but the engine only rewrites $error when a
failing node returns a failure, or throws through a node with its own
fault edge (engine.ts's executeNode, the throw arm) — and a node inside
a try_catch's try region never has a fault edge of its own, because the
region's synthetic sub-flow carries only the region's own edges. So a node
that fails by throwing (a timeoutMs firing, a dying nested container, a
thrown guard) used to leave $error exactly as an earlier, unrelated
failure left it — its code included. Two flows reproduce it: a loop
sweeping two rows where row 1 is a genuine duplicate (swallowed correctly)
and row 2's store hangs into a timeoutMs (previously bound as
{ code: 'DUPLICATE_RECORD', message: "…timed out…" } and swallowed too —
a store failure misread as a duplicate, this card's own failure mode through
a different door); and a plain flow where an earlier fault-routed duplicate
on node A leaked into a completely unrelated later try_catch around node B.

Fixed with a minimal identity guard: capture $error at the start of
each try-region attempt, and only trust the post-catch $error as this
attempt's failure if it changed (!==) from that snapshot — not merely if it
is still present. Both repro flows are now pinned (create-record-duplicate-code.test.ts,
third describe block) and were verified red on the pre-guard code, green
after (reproduced locally: reverting the guard's comparison back to the old
form fails exactly those two new tests and none of the other four; restoring
it turns all six green again).

P2, the packages/spec gap — deferred, not fixed here. TryCatchErrorValueSchema
declares the errorVariable binding shape (nodeId, message, iteration,
item) as one shape shared by author, engine and run log, and does not
declare code yet — a strict parse strips it. packages/spec is single-owner
(domain:spec); this lane does not touch it. Filed as #14954, named in
the changeset. The hand-written content/docs/automation/flows.mdx row is
updated to mention {$error.code} (see Changes); the generated
content/docs/references/automation/control-flow.mdx is deliberately
untouched — it moves only when spec's own describe() does.

P3, a test that asserted nothing. The original "sets code: DUPLICATE_RECORD"
regression pin checked success, step status and error text — never code
itself, and its comment claiming the step log was where a node-level code
"actually lands" was wrong (StepLogEntry has no code; its error.code is
the constant 'NODE_FAILURE'). Both regression-pin tests (first describe
block) are rewritten to register a fault-edge handler that reads $error
directly and assert code on it — which also exercises the direct-fault-edge
read path (engine.ts's executeNode, outside any try_catch) that this PR
body claims and that nothing previously tested.

Fold-in, non-blocking: the JSDoc on NodeExecutionResult.code now says
plainly that create_record narrows deliberately to DUPLICATE_RECORD rather
than promising "any classified code" (see Changes); and the changeset notes
that a custom IDataEngine throwing { code: 'DUPLICATE_RECORD' } directly
(without being an instance of DuplicateRecordError) is duck-typed as a
duplicate too — correct under ADR-0112, since code is the envelope's public
contract, not the concrete class. The review's note 2 (a try_catch whose
catch itself re-raises returns without forwarding a code) is out of this
round's scope, as directed.

Contract review (Clause-②)

NodeExecutionResult is exported at @objectstack/service-automation's
package root (src/index.ts), so the new code field is a new payload key on
a published surface — measured on the built dist/index.d.ts, not asserted:
grepping the built declaration file for the NodeExecutionResult interface
shows code optional-string alongside success, output and error. And
confirmed as newly added against origin/main:

git diff origin/main -- packages/services/service-automation/src/engine.ts | grep 'code?:'

returns exactly one added line declaring the new optional field. Additive-only
(optional field, no existing field or behaviour changed) — flagging for the
seat's needs:contract-review call, not asserting the outcome.

Tests

• packages/services/service-automation/src/builtin/create-record-duplicate-code.test.ts — now 6 tests across 3 describe blocks:
  • two $error-level regression pins, each registering a fault-edge handler and asserting code on the $error it receives (DUPLICATE_RECORD set / unset) — corrected in patch round 1, see above;
  • two discrimination tests (the try_catch flow above) — the pair that earns the card;
  • two patch-round-1 stale-code repro tests (a loop over two rows, and a plain flow with an earlier unrelated fault-routed failure) — reproduced red on the pre-guard code, green with the identity guard.
• Full package suite: pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 — 102 files / 1209 tests passed on the final head commit (after patch round 1; 1207 before it, +2 for the new repro tests).
• tsc --noEmit -p packages/services/service-automation/tsconfig.json — only the 3 pre-existing DEBT-ledgered errors in nested-region-parity.test.ts (unrelated; matches scripts/check-type-check-coverage.mjs's ledger entry verbatim), zero new errors, reconfirmed after patch round 1.
• Ablation (original submission): stripped the code assignment in crud-nodes.ts (literal-text replace, occurrence count confirmed 1 to 0 before running, marker confirmed present) — re-ran the discrimination test file — the "swallows DUPLICATE_RECORD" test failed as predicted (took the reraised/escalate path instead of swallowed/after); the other tests stayed green. Restored via a git checkout from HEAD (file was committed first) — blob hash matched HEAD's exactly, the diff against HEAD was empty — re-ran the test file — all green again.
• Red→green (patch round 1): reverted the identity guard's !== comparison back to the pre-patch form (marker comment confirmed present, then removed) — the two new stale-code repro tests failed exactly as the review predicted, the other four in the file stayed green — restored the guard (marker confirmed gone) — all six green.
• Gate family derived via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (re-derived after the final commit) — every matched gate run; all green except:
  • check:dual-build-cjs-loads — exit 3, NOT MEASURED (needs a full pnpm build, out of local-verification scope per this repo's own convention; CI runs it).
  • check:type-check-debt — exit 3, NOT MEASURED (its own re-measure is a maintainer-only act).
  • check:test-completeness — exit 3, NOT MEASURED (grades a saved turbo run test log; CI supplies one, there is none locally).
• check:system-context-census line-rot repaired via its own --fix (see Changes and the follow-up regen-discharge commit after merging origin/main, which shifted a different anchor); re-verified clean.
• check:undeclared-dep-imports caught the first draft's @objectstack/objectql runtime import (devDependency only) — fixed by switching to the duck-typed StandardErrorCode check described above; re-verified clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8