fix(lint): the flow-template rules read an http node's request payload - #16880
Conversation
`flow-double-brace-interpolation` and `flow-bare-dollar-reference` scan a
node's config recursively, over a region-STRIPPED view so a container is not
also credited with its descendants' findings. That view was built from the
FLAT UNION of every config key that holds a region on ANY node type (`body`,
`try`, `catch`, `branches`) instead of the slots the node in hand owns, because
this call site took `stripRegions`' default argument.
`body` is `loop`'s region slot AND the canonical request-payload key on an
`http` node, so `config.body` was deleted from every node's view before the
scan read it. That is the one key where an uninterpolated token has an outbound
consequence: `http-nodes.ts` interpolates the raw config wholesale, so a
double-brace `{{ }}` or a bare `$ref.field` written in a payload ships to the
endpoint as literal text — the exact failure both rules exist to catch, in the
exact place they could not see.
The remedy was already written in `stripRegions`' own docblock ("Pass the
OWNING node's slots, not the flat union") and the sibling call site in
`flow-walk.ts` already followed it. This one now does:
`stripRegions(node.config, ownRegionKeys(node.type))`.
`regionKeys` also becomes REQUIRED, and the flat-union default is deleted. The
union survived as a default only to bound the earlier change that introduced
the per-type argument, and this defect is what that cost: the shorter call
compiled and quietly asked a different question than its caller meant. With no
default, a caller that has not decided which set it means fails to compile.
`flow-walk.test.ts` pins that with a `@ts-expect-error` — evaluated, since
`tsconfig.test.json` compiles the test layer.
Both directions are pinned. Measured on the parent commit, every new case
returned zero findings for its rule: a `{{ }}` and a bare `$ref.field` in an
`http` payload, at top level and nested in a `try_catch` region, for both rule
ids. The over-correction direction — a repair that stripped nothing — is
pinned too: a payload token inside a `loop` body is still reported exactly
ONCE, against the node carrying it and not also against the container.
Blast radius measured across `examples/app-showcase`, `app-crm` and
`app-todo`: 34 flows, `http` payloads inside a `parallel` branch and a
`try_catch` try among them, zero findings before and zero after — those
payloads use correct single-brace tokens. A positive control, one `{{ }}`
injected into a real showcase `http` payload, reads 0 before and 1 after.
Fixes #16405
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 5 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 89ed5e18842bd1ca305a6a7aa87a353160ab3fa0 && git checkout 89ed5e18842bd1ca305a6a7aa87a353160ab3fa0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4cfc93b80270b12854fc31f7a4837f1f8bcf5513 837710e25b665fee1fb1f32809db52b210d5b59b && git checkout -B drift-repro 4cfc93b80270b12854fc31f7a4837f1f8bcf5513 && git merge --no-ff 837710e25b665fee1fb1f32809db52b210d5b59b
node scripts/docs-audit/affected-docs.mjs --json 4cfc93b80270b12854fc31f7a4837f1f8bcf5513 |
Fixes #16405
flow-double-brace-interpolationandflow-bare-dollar-referencescan each node's config recursively, over a region-STRIPPED view so a container is not also credited with its descendants' findings. That view was built from the FLAT UNION of every config key that holds a region on any node type (body,try,catch,branches) rather than the slots the node in hand owns — because the call site tookstripRegions' default argument.bodyisloop's region slot and the canonical request-payload key on anhttpnode, soconfig.bodywas deleted from every node's view before the scan read it. That is the one key where an uninterpolated token has an outbound consequence:http-nodes.tsinterpolates the raw config wholesale, so a{{ }}or a bare$ref.fieldin a payload ships to the endpoint as literal text — the exact failure both rules exist to catch, in the exact place they could not see.What changed
lint-flow-patterns.ts—stripRegions(node.config, ownRegionKeys(node.type)). The remedy was already written instripRegions' own docblock ("Pass the OWNING node's slots, not the flat union") and the sibling call site inflow-walk.tsalready followed it; this one did not.flow-walk.ts—regionKeysis now REQUIRED and the flat-union default is deleted. Acceptance criterion 1 of the triage comment: doing only the call site leaves the trap for the next caller. The union survived as a default only to bound the earlier change that introduced the per-type argument, and this defect is what that cost. With no default, a caller that has not decided which set it means fails to compile.try/catch/branchesdirection; and a@ts-expect-errorpin that the omitted call no longer compiles.Red leg — both rule ids, silent before
Every new case, run against the parent commit before the source edit (
pnpm --filter @objectstack/lint exec vitest run src/lint-flow-patterns.test.ts):After:
Test Files 2 passed (2) · Tests 168 passed (168); whole package102 passed / 3576 passed | 5 skipped.The false-positive guard (
{record.id},{$User.Id},Total $5) was green in both states — it was never red, deliberately, and is marked as such.Ablation — the required-parameter pin can fail
regionKeys' default restored on disk, anchor counts inverted (required 1 → 0,default 0 → 1), blob170f7cdd→a77412b3:Blast radius — measured, both states, with a positive control
Same harness run against the fixed source and against the flat-union source (call site reverted on disk, blob
f73cc7ef→e2d2b67b, restored byte-identical):examples/app-showcase— 29 flowsexamples/app-crm— 1 flowexamples/app-todo— 4 flows{{ }}injected into a real showcasehttppayloadThe control lands on
flow 'showcase_project_escalation' · try_catch 'push_incident' try · node 'push' (http)— a realhttpnode nested in atry_catchtry, reported once and attributed to the node that carries it. Zero real findings is a measurement here, not an absence of one: those three payloads use correct single-brace tokens.Both rules keep their existing severity.
验收备注
try/catch/branches— the census the card asked for, both directionsIs any node type using these names as ordinary config today? No. Grepping every
*.zod.tsunderpackages/spec/src/automationfor a declared key by each name:bodyloop.config.body(control-flow.zod.ts)HttpConfigSchema.body— "Request body (JSON-serialised)". Alsonotification.config.body, the pre-17 spelling ofmessage, rewritten at load by the ADR-0087 conversionflow-node-notify-config-aliasesand then interpolated (notify-node.ts) — a second real casualty of the same union, fixed by the same change.trytry_catch.config.trycatchtry_catch.config.catchbranchesparallel.config.branchesSo
bodywas the only key colliding with a declared config key, and it collides on two node types, not one.try/catch/branchesare still covered by the fix and by a pin, for the reason the strip has to answer anyway:FlowNodeSchema.configis an openz.record(ADR-0018 removed the node-type enum gate), so any of those names authored on a node owning no region is metadata a rule must read rather than silently delete. The pin asserts three findings from three such keys, at top level and nested.The double-count direction, per the triage comment
Pinned at
reports a payload token inside a 'loop' body ONCE, on the node carrying it: anhttpnode whose payload carries{{lead.name}}inside the sharedloopBodyFlowcontainer, asserted at exactly one finding,whereequal toflow 'campaign_enrollment' · loop 'loop_leads' body · node 'push' (http)and explicitly not namingloop_leadsas the node. This case is red on the parent commit for the visibility reason and would be red on an over-corrected fix for the count reason, so it pins both directions with one assertion pair.Scope
packages/spec/src/automation/io-node-config.zod.tsis untouched — cited as evidence, not changed. #16751 is not folded in.Gates
Derived from the real changed set with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackand reconciled with--ran: 55 derived, 55 run, 0 UNRUN.53 exit 0. Two exit 3 —
PREREQUISITE NOT MET, which each script prints as "⛔ This is NOT a pass: nothing was measured":check:dual-build-cjs-loadsandcheck:type-check-debtboth read built output across the whole workspace and wantpnpm build/ the full turbo closure first. CI builds before those steps; they are declared to CI, not skipped silently.Repo-wide
pnpm lint(eslint . --no-inline-config) ran in full and is green — no narrowing claimed and none owed. Ratchet families re-run at the final commit837710e25bwith a clean tree:check:type-check-coverage,check:cross-package-test-inputs,check:nul-bytes,check:test-source-aliasall exit 0.pnpm --filter @objectstack/lint typecheckgreen, withcheck:test-typecheckholding the ledger unchanged at 2 files / 6 errors / 2 pinned signatures.🤖 Generated with Claude Code
https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU
Generated by Claude Code