Skip to content

fix(lint): the flow-template rules read an http node's request payload - #16880

Merged
baozhoutao merged 1 commit into
mainfrom
claude/issue-16405-http-body-template-reach
Sep 8, 2026
Merged

fix(lint): the flow-template rules read an http node's request payload#16880
baozhoutao merged 1 commit into
mainfrom
claude/issue-16405-http-body-template-reach

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #16405

flow-double-brace-interpolation and flow-bare-dollar-reference scan 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 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 {{ }} or a bare $ref.field 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.

What changed

  • lint-flow-patterns.tsstripRegions(node.config, ownRegionKeys(node.type)). 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 did not.
  • flow-walk.tsregionKeys is 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.
  • Tests — the two payload pins in both token shapes, at top level and nested in a region, for both rule ids; the double-count negative control; the try/catch/branches direction; and a @ts-expect-error pin 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):

× flags a `{{ }}` token in a top-level `http` node payload        AssertionError: expected [] to have a length of 1 but got +0
× flags the same token when the `http` node sits inside a region  AssertionError: expected [] to have a length of 1 but got +0
× reaches a token nested deep inside the payload                  AssertionError: expected [] to have a length of 1 but got +0
× flags a bare `$ref.field` in a top-level `http` node payload    AssertionError: expected [] to have a length of 1 but got +0
× flags the same reference when the `http` node sits inside a region  AssertionError: expected [] to have a length of 1 but got +0
× reports a payload token inside a `loop` body ONCE               AssertionError: expected [] to have a length of 1 but got +0
× reads a `try` / `catch` / `branches` key on a node owning no region  AssertionError: expected [] to have a length of 3 but got +0
  Tests  7 failed | 142 passed (149)

After: Test Files 2 passed (2) · Tests 168 passed (168); whole package 102 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), blob 170f7cdda77412b3:

MUT_EXIT=1
  • src/flow-walk.test.ts: 1 type error(s) in a file the ledger does not cover.
    (A deleted `@ts-expect-error` shows up exactly here, as TS2578/TS2694.)
RESTORE_EXIT=0   restored blob 170f7cdd   git diff HEAD: []

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 f73cc7efe2d2b67b, restored byte-identical):

scanned before after
examples/app-showcase — 29 flows 0 0
examples/app-crm — 1 flow 0 0
examples/app-todo — 4 flows 0 0
CONTROL — one {{ }} injected into a real showcase http payload 0 1

The control lands on flow 'showcase_project_escalation' · try_catch 'push_incident' try · node 'push' (http) — a real http node nested in a try_catch try, 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 directions

Is any node type using these names as ordinary config today? No. Grepping every *.zod.ts under packages/spec/src/automation for a declared key by each name:

key declared as a region declared as ordinary config
body loop.config.body (control-flow.zod.ts) HttpConfigSchema.body — "Request body (JSON-serialised)". Also notification.config.body, the pre-17 spelling of message, rewritten at load by the ADR-0087 conversion flow-node-notify-config-aliases and then interpolated (notify-node.ts) — a second real casualty of the same union, fixed by the same change.
try try_catch.config.try none
catch try_catch.config.catch none
branches parallel.config.branches none

So body was the only key colliding with a declared config key, and it collides on two node types, not one. try / catch / branches are still covered by the fix and by a pin, for the reason the strip has to answer anyway: FlowNodeSchema.config is an open z.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

a token in a loop body must still be reported ONCE, on the node that carries it, not also against the container. That is what the strip exists for, and it is the direction that breaks if the fix over-corrects to stripping nothing.

Pinned at reports a payload token inside a 'loop' body ONCE, on the node carrying it: an http node whose payload carries {{lead.name}} inside the shared loopBodyFlow container, asserted at exactly one finding, where equal to flow 'campaign_enrollment' · loop 'loop_leads' body · node 'push' (http) and explicitly not naming loop_leads as 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.ts is 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/objectstack and 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-loads and check:type-check-debt both read built output across the whole workspace and want pnpm 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 commit 837710e25b with a clean tree: check:type-check-coverage, check:cross-package-test-inputs, check:nul-bytes, check:test-source-alias all exit 0. pnpm --filter @objectstack/lint typecheck green, with check:test-typecheck holding 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

`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
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot 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
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 4cfc93b80270b12854fc31f7a4837f1f8bcf5513packageMentionDocs.

Which tree this was computed on

This run read content/docs from 89ed5e18842bd1ca305a6a7aa87a353160ab3fa0 — the merge of head 837710e25b665fee1fb1f32809db52b210d5b59b into base 4cfc93b80270b12854fc31f7a4837f1f8bcf5513, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants