fix(trigger-record-change): decouple the flow-facing record from the batch payload - #15475
Conversation
… adopt #15356 harness Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…t-reach pins Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ontrol Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…couple-flow-record-from-batch-payload
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 1 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 94561225ff12def25dfb8e6803c34e292e27fac0 && git checkout 94561225ff12def25dfb8e6803c34e292e27fac0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e8c7956c46c0e8abda9d259ce07ba5f64e072c47 17172f1c5d3109c83cc4faa891bfb4f608ec9001 && git checkout -B drift-repro e8c7956c46c0e8abda9d259ce07ba5f64e072c47 && git merge --no-ff 17172f1c5d3109c83cc4faa891bfb4f608ec9001
node scripts/docs-audit/affected-docs.mjs --json e8c7956c46c0e8abda9d259ce07ba5f64e072c47
|
ACCEPT — and my dispatch instruction was the thing that was wrong. The ruling offered two fix shapes; this run measured one of them to be actively incorrect.
⭐ The freeze was not an equal alternative, and that is a falsification of the ruling's own menuBatch #38 ruled 「nested values deep-copied (or the overlay frozen)」 — two options, offered as equivalent. The dev measured that a freeze is wrong on two independent counts, and I verified the premise rather than taking it:
⇒ One of the ruling's two named shapes fails the ruling's own objective. That is worth more than a clean implementation of the other one. What a flow author now observes — the answer to the question I actually askedI warned that 「a silent no-op is a worse contract than a loud refusal」. The measured answer is neither: 「the old trick still takes effect on the snapshot the flow is holding — The S5 flip, done in the order that makes it evidenceThe adopted harness was run UNMODIFIED against the fix first: The changeset — the trap I flagged, navigated by measurement and dated
|
Landing — gate first, then ready, then armed.
|
Part of #14744 — the A fix ruled by the maintainer on 2026-09-04 (decision batch #38 item 2, verbatim 「同意」, recorded by
os-warrenin comment 5542623847).Part of, not a closing keyword. The ruling enumerates what closes that card: this fix merged with the harness adopted, the hotcrm reading posted (owed by the hotcrm seat under Q3), and the residue documented. Two of those are not in this PR, so merging it must not close the card.check:partof-closing-keywordis green against this body, and no commit on this branch carries a closing keyword for that number either.What this changes
buildContextbuilds the flow'srecordas a shallow overlay of the pre-image, the mutation payload and the after-row. The top-level object was new — so a flow assigning a top-level key reached nothing — but every nested value in it was the engine's own object, shared by reference. One of those isctx.input.data, and on amulti: trueupdate ADR-0058 Addendum II D3 hands every per-row context that same payload object, which is the SET clause of the singleupdateMany.So a
scriptnode whose registered function didrecord.tags.push(...)wrote that SET clause without assigning any key. Every dispatch's contribution landed on every matched row, including values derived from another row's pre-image, and #14099's key-set refusal could not see it because no key was assigned and both rows reported the empty key set. #15356 measured it end to end on the memory driver and on@objectstack/driver-sql.Both flow-facing roots are now decoupled from the engine's state before the flow runs, via
decoupleFromEngineState(new module, module scope only — not re-exported fromindex.ts, following thematerializeDeclaredFieldsprecedent in the same package).Fix shape: a COPY, not a freeze — and the measurement that chose
The ruling named deep-copy and freeze as alternatives. They are not equivalent, and freeze is not available here.
service-automation'sexpandDeclaredLookups(#3475) writesrecord[field] = expandedinto the contextbuildContextreturns; its own docblock says "Mutatesrecordin place (the same object the run's variable map already references)", andAutomationServicePluginbridges that expander in every deployment that has objectql.Measured, not argued. A new case in the harness — "a flow declaring
config.expandstill gets its lookup grafted onto the copy" — is green with the copy and red under a deep-freeze variant driven through the same seam:Under a freeze the graft throws,
expandDeclaredLookups' best-effortcatchswallows it, and every flow declaringconfig.expandsilently degrades to unexpanded scalar ids while logging "could not expand lookups". A freeze would also convert one unsupported write into a whole-flow outage, because the trigger's handler swallows flow failures by design (error isolation). And it would not even close the aliasing: under the freeze variant S5's reference-identity assertions stay red, because the flow still holds the payload's own array.What a flow author observes now. The mutation still takes effect on the snapshot the flow is holding —
{record.tags}later in the same run still sees it — it simply reaches nothing outside the run. It is not a no-op inside the flow; it is a snapshot that stopped being a handle on the engine's write. A flow that needs to write uses theupdate_recordnode (S6, measured: per-row values land correctly).previousis decoupled in the same strokectx.previousis the engine's single pre-image object and the sameHookContextreaches every other flow bound to the write — which is whybuildContextalready refused to materialise into it ({ ...priorBase }). That copy was shallow, so the stated rule held for top-level keys only. Same defect class, same function, same gate family, and the file's own comment is the authority for the intent; the comment is corrected in this diff to say which half a shallow copy buys. Pinned bydecouple-flow-record.test.ts.Cost, measured
multi: trueover 200 rowsBoth numbers come from the same process, so they are comparable without a cross-run baseline. A cross-run A/B (ablated 253.6 µs/dispatch vs 312.7 and 331.8 µs/dispatch on two identical fixed runs) is not separable from shared-box variance and is reported here only so nobody re-derives it as a clean signal: the with-fix number moved 6 % between two identical runs, the same order as the claimed delta. On a shared box the in-process ratio is the reading; the wall-clock absolutes are not.
The pin: #15356's harness adopted, S5 flipped
before-update-flow-payload-reach.test.tsis taken fromclaude/issue-15356-record-before-update-flow-payload-reach@e87000489— one file, unmodified except for the flip and one added case. Run unmodified against the fix first, it failed in exactly the predicted places, and those three failures are the evidence the door closed:S5, S5b and the SQL replica were then converted to cannot-reach pins — the opposite assertions, not weakened ones, keeping every observable they measured on (reference identity across the boundary, the per-row readings, the persisted rows) and keeping the S4/S5 discriminating pair intact. Both controls are untouched and both still fire.
What does NOT change
ADR-0058 Addendum II D3 stands untouched — the engine does not split its own write, one payload still serves N rows, and every per-row context is still handed that one object (asserted in S5). #14099's key-set criterion is untouched and not widened: a hook assigning the same key with per-row values still passes it, and divergent key sets are still refused whole (armed control, green). Pure flow metadata still reaches nothing.
Changeset — the derivation, dated
Level
patch, banner BREAKING, dispositionnot-required (no-migration-prescription).npm view @objectstack/trigger-record-change versionsends at 17.3.0, which is this repo's current version.npm pack @objectstack/trigger-record-change@17.3.0and grep of the publishedpackage/dist/index.jsshows the shallow overlay on lines 162–163:{ ...priorBase ?? {}, ...inputData ?? {}, ...after }and its two-arg sibling. ⇒ the reach is reachable by a released consumer today..github/workflows/pr-automation.yml(maintainer, 2026-09-04, decision batch [WIP] Add query enhancements and advanced validation features #35, on finding(changeset): two independent contract reviews read the repo's own history to opposite bumps for "add an exported symbol to a published index" #15294): afix(that changes no public surface stayspatch; breaking-ness is carried by the banner plus the ADR-0087 disposition, not by the level. This diff adds no exported symbol and no accepted key or value — the entry pointsrc/index.tsis byte-identical toorigin/main.objectstack migrate metahas nothing to reach. Registering a ledger entry would put data in it thatspec-changes.jsonand the upgrade guide cannot project — the failurecheck-adr-0087-registration.mjsnames in its owntype-surface-onlysection as "false data in the one ledger this gate keeps true". The gate accepts the claim:[BREAKING+bang] not-required (no-migration-prescription), exit 0.Clause ②:
noEvery published package this diff touches:
@objectstack/trigger-record-change, and nothing else.git diff origin/main...HEAD -- packages/triggers/trigger-record-change/src/index.tsis empty — no symbol added, moved or re-signed.buildContextisprivate. ⇒ noneeds:contract-reviewpre-hang.Verification
--commandsand asserted against the Reconciliation line's own total of 44, derived at the merged head17172f1c5. Exit codes captured by redirecting to a file before any pipe. 42 exit 0 on the first pass; two returned exit 3 = NOT MEASURED (check:dual-build-cjs-loads,check:type-check-debt) because each reads built output for the whole workspace. Both were then satisfied properly rather than reported as passes:turbo run buildacross the workspace (71/71 tasks), after whichcheck:dual-build-cjs-loadsexits 0 (103 require entry points across 66 packages, 619 CJS files parse) andcheck:type-check-debtexits 0 (14 ledger entries re-measured, 153 raw errors, none above its recorded number). 44/44 exit 0, all at17172f1c5.typecheckexit 0 for both programs, andtsc --listFilesconfirms both new test files are inside the test program (1 hit each) rather than excluded from it. Full-repoeslint . --no-inline-config: 5922 files, 0 errors, 0 warnings, exit 0, with all four changed files confirmed present in that population byfilePath— a whole-population run, not a narrowing.decoupleFromEngineStatecalls turns 6 cases red — the three seam-pin decoupling cases plus S5, S5b and the SQL replica — while both controls and the expand case stay green. Each leg proved the mutation landed on disk (injected marker counted twice, both removed call sites counted at zero) before its colour was read, and restore was proved withgit diff HEADempty plus agit hash-objectmatch against the HEAD blob.Generated by Claude Code