Skip to content

fix(lint): flow template rules reach a {record.FIELD} token outside a node filter - #16407

Merged
baozhoutao merged 4 commits into
mainfrom
claude/issue-16111-flow-template-leaves-outside-filters
Sep 6, 2026
Merged

fix(lint): flow template rules reach a {record.FIELD} token outside a node filter#16407
baozhoutao merged 4 commits into
mainfrom
claude/issue-16111-flow-template-leaves-outside-filters

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #16111

flow-template-unknown-field and flow-template-lookup-traversal each declare a per-position severity — error inside a filter-guarded CRUD node's filter, warning everywhere else — and the warning half never fired on the shape a real hand-off flow has. This makes it reach. Existing rule ids, existing severities: severity: inFilter ? 'error' : 'warning' is untouched.

The cause is one key, and it is in the shared walk, not in either rule

WalkedFlowNode.localConfig is the region-stripped view a recursive config scan must read, or it reports every nested finding a second time against the container. It was built by removing every key that holds a region on any node type.

body is loop's region slot and the canonical request-payload key on an http node — packages/spec/src/automation/io-node-config.zod.ts says so in as many words ("body IS canonical on an http node"). So config.body was deleted from every node's view before any rule saw it, and the whole of an http payload was invisible to the scan.

That position is exactly where it matters at run time: packages/services/service-automation/src/builtin/http-nodes.ts interpolates the raw config wholesale (interpolate(raw, variables, context)), so an unresolved token there renders an empty value into a real outbound request, on every run, with nothing anywhere reporting it.

stripRegions now takes the keys to remove; walkFlowNodes passes the slots the node's own type declares — the same lookup that decides where the walk descends. Remove fewer and a nested finding is reported twice; remove more and a key that was never a region is deleted unread. The middle is the only correct answer, and both edges are pinned below.

Red-first, on this tree

packages/lint/src/flow-walk.ts swapped for its origin/main copy (proven on disk: blob b8b8be6ae in place of 8992cf6e5), then the two suites run:

 Test Files  2 failed (2)
      Tests  8 failed | 51 passed (59)

The split is the point:

pin on the unmodified walker
unknown field in a filter (negative control) green — the gating half already worked
lookup traversal in a filter (negative control) green — same
unknown field in a payload redexpected [] to have a length of 1
lookup traversal in a payload redexpected [] to have a length of 1
payload nested in a loop body redexpected [] to have a length of 1

Restored with git checkout HEAD -- packages/lint/src/flow-walk.ts, proven by blob hash back to 8992cf6e5 and an empty git diff HEAD. Same two suites on the fix: Tests 59 passed (59).

Ablation — three legs, both directions

The red-first swap above conflates two things for the walker pins (behaviour, and the fact that ownRegionKeys does not exist on main at all), so it is not the whole ablation. Two targeted single-argument mutations separate them. Every leg proves the mutation reached disk before its colour is read — anchor counted with grep -cF before and after, plus a blob-hash comparison against HEAD — and every leg restores under a trap ... EXIT INT TERM with an absolute path, verified by blob hash.

  • Leg B — stripRegions(raw.config) (the flat union restored, new exports still present): 5 failed | 54 passed. Exactly the 3 rule pins and the 2 behavioural walker pins; the 3 API-existence pins stay green. This isolates the single argument as the cause, not the surrounding refactor.
  • Leg C — stripRegions(raw.config, []) (strip nothing): 5 failed | 54 passed, and the failures are the opposite direction — expected [ ... ] to have a length of 1 but got 2, on the loop-body pin and on two pre-existing pins from card 4380 (reports a nested token once, not also against the container, keeps the gating filter-position severity inside a region). So the fix is pinned from both sides: over-strip and the warning half goes silent, under-strip and everything nested double-counts.

Leg C's first attempt was declared VOID by its own guard and is reported rather than quietly retried: the observation predicate used grep -c with a bracket-bearing pattern, which is a regex error, so the injected-text count came back empty. The mutation had landed; the observation was broken. It was re-run with grep -cF. Nothing was read from the void run.

Blast radius of the shared walker

flow-walk.ts is shared, so the change was scoped by reading every consumer rather than by assertion.

  • localConfig has exactly one reader: validate-flow-template-paths.ts. Every other walkFlowNodes caller (validate-approval-approvers, validate-flow-node-writes, validate-readonly-flow-writes, validate-translation-references) reads node / path / regionTrail only, so none of them can move.
  • stripRegions has one other caller, lint-flow-patterns.ts, calling it with one argument. The flat union stays the DEFAULT so that caller keeps its current behaviour under this PR.
  • The walk's descent is unchanged: ownRegionKeys resolves through the same REGION_SLOTS map, and an empty result takes the same early return the old undefined did.

Floor: the whole @objectstack/lint suite, 100 passed (100) / Tests 3424 passed, plus typecheck (the test-typecheck debt ledger held at 2 files / 6 errors — nothing added). Direct dependents: @objectstack/metadata-protocol 2414 passed, @objectstack/example-showcase 379 passed, @objectstack/cli unit tier 2453 passed | 6 expected fail. Repo-wide pnpm lint (eslint . --no-inline-config, the whole population, not a narrowed subset) exits 0.

Does an example gain a finding?

Measured, not reasoned. objectstack validate on examples/app-showcase: passes, exit 0, zero flow-template-* findings. The showcase is the only authored corpus in the repo that carries a {record.FIELD} token under one of the formerly-stripped keys — two http payloads, one inside a parallel branch, one inside a try_catch try — and both name real fields (title, id).

A clean result is also what a rule that never ran would produce, so it was made a measurement with an anti-vacuity control: injecting one typo into the showcase's real nested payload produces

⚠ flow "showcase_fan_out_notify" parallel "Notify in parallel" › branch Post to Slack node "http":
  template references '{record.titel_nope}', but 'titel_nope' is not a field on object
  'showcase_task' — it resolves to an empty string at runtime (silently).

with validate still exit 0 — which is also the direct confirmation that a warning moves no exit code. Control reverted, blob hash verified.

Tests

Four pins on one hand-off fixture, matching the four measured injections: the two filter positions as negative controls that must keep reporting error, the two payload positions as the warning half. Plus the both-positions dedupe (one leaf used in a filter and in fields resolves once, at the gating severity), a clean flow reporting nothing, and a payload nested in a loop body reported once, on the node that carries it, with the full region trail. Every assertion reads the findings array — never a pass/fail verdict — because a warning does not move the exit code.

Gates

All 54 families dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derives for this diff, run at head d5a30458f, reconciled with --ran against the tool's own output:

✓ dispatch-gates --ran: 54 derived famil(ies) accounted for — 54 run, 0 NOT-MEASURED.

All 54 exit 0. Two needed a second run and neither was a finding: check:dual-build-cjs-loads first printed PREREQUISITE NOT MET (it reads built output and 7 packages had no dist/), and check:type-check-debt first OOMed at 4096 MB — re-run at the 6144 ceiling it documents for itself, 55 raw tsc error(s) total, none above its recorded number. Both are NOT MEASURED, not red, and both are green once run properly. Also run, because the derivation flags their roster silence as non-evidence for paths like mine: check-changeset-fixed, check:authz-resolver, check:error-code-casing, check:filter-alias-parity — all exit 0.

Takeover

Picked up from a dev killed by a container restart mid-task. Its one commit 50f343ad3 was pushed; no PR and no report existed.

Inherited and kept — the diagnosis and the fix shape (stripRegions taking the owning type's slots), the four pins plus the dedupe / clean-flow / loop-body pins, and the changeset.

Re-verified from scratch, nothing taken on trust — the previous dev's logs were read as context and none of its numbers were reused. Every figure in this description is from a run in this session: the red-first swap, all three ablation legs, the full lint suite, typecheck, the dependent suites, all 54 gates, repo-wide eslint, and the showcase validate measurement with its control. Both factual claims the inherited commit message rests on were checked against source rather than believed: that body is canonical on an http node (the spec's own message), and that the http executor interpolates its raw config wholesale (http-nodes.ts).

Corrected

  1. Merged origin/main. It had moved onto packages/lint/src/lint-flow-patterns.ts — the other stripRegions caller — while the branch sat idle. The inherited measurements predate that. Merged, confirmed the incoming version still calls stripRegions with one argument (so the preserved default keeps it source-compatible), and re-ran everything on the joint tree, per AGENTS.md section 10.
  2. A comment that overstated the fix. It read as though the flat-union default were correct for the caller still taking it. It is not — lint-flow-patterns.ts runs its own recursive template scan over the union view and is blind to an http node's body for exactly the reason this PR documents. Same defect, one call site over. The note now says the default is scope control, names the sibling defect, and states the endgame: once that caller passes its own slots, regionKeys has no default-takers left and must become required.
  3. A changeset clause that put the empty-string rendering "at authoring time and at run time alike". The rendering is a run-time event; what happens at both ends is that nothing reports it.

Out of scope

  • Filed as card 16405: the sibling defect in lint-flow-patterns.ts described above. Not fixed here because it changes the reach of two different rule ids (flow-double-brace-interp, flow-bare-dollar-ref), which earns its own changeset line and its own review rather than riding along on this one. Searched first; card 5383 and card 4380 are the closed ancestors of this walk, and nothing open covers it.
  • Card 16096 (filter-token-unknown never walks flows) is not addressed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8


Generated by Claude Code

… node filter

`flow-template-unknown-field` and `flow-template-lookup-traversal` each declare
a per-position severity — `error` inside a filter-guarded CRUD node's `filter`,
`warning` everywhere else — and the `warning` half never fired on the shape a
real hand-off flow has.

The cause is one key, and it is in the shared flow walk rather than in either
rule. `WalkedFlowNode.localConfig` is the region-stripped view a recursive
config scan must read or it reports every nested finding a second time against
the container; it was built by removing every key that holds a region on ANY
node type. `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
any rule saw it — and the http executor interpolates its raw config wholesale,
so a `{record.<typo>}` there renders an empty value into an outbound request on
every run.

`stripRegions` now takes the keys to remove; `walkFlowNodes` passes the slots
the node's own type declares, the same lookup that decides where the walk
descends. Nothing is double-reported and nothing that was never a region is
dropped. The flat-union view stays the default argument so the helper's other
caller keeps the behaviour it was written against.

Tests: the four measured injections as four pins on one hand-off fixture — the
two filter positions as negative controls that must stay `error`, the two
payload positions as the `warning` half — plus the both-positions dedupe, a
clean flow, and a payload nested in a loop body reported once on the node that
carries it. Every assertion reads the findings array; `warning` does not move
the exit code.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
…e-filters

origin/main landed a change to `packages/lint/src/lint-flow-patterns.ts` — the
other caller of `stripRegions`, whose behaviour this branch's signature change
has to preserve. Merged before opening the PR so the suite is measured on the
joint tree (AGENTS.md §10).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
The clause read "at authoring time and at run time alike" against "renders as
an empty string", which puts the rendering at authoring time. The rendering is
a run-time event; what happens at BOTH ends is that nothing reports it — no
build-time finding, no run-time error — which is the property that makes the
failure survive to production.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
The note read as if the flat-union default were correct for the caller that
still takes it. It is not: `lint-flow-patterns.ts` runs its own recursive
template scan over the union view, so it is blind to an `http` node's `body`
for exactly the reason this function's own doc gives one paragraph earlier —
the same defect, one call site over. The default is scope control for this
change, not a verdict, and the note now says which.

Also states the endgame, so the next reader does not have to rediscover it:
once that caller passes its own slots, `regionKeys` has no default-takers left
and must become required, or the shorter call keeps handing out the trap.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
@github-actions github-actions Bot added the size/m label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

5 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 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 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; 102 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 de75e407e53af5a54b265261bc886e167626ce98packageMentionDocs.

Which tree this was computed on

This run read content/docs from 076a4c88b018fba5a20a3bd5d8276474fcb2ac46 — the merge of head d5a30458f812ae2b851a367efda1d428d90ddaa6 into base de75e407e53af5a54b265261bc886e167626ce98, 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 076a4c88b018fba5a20a3bd5d8276474fcb2ac46 && git checkout 076a4c88b018fba5a20a3bd5d8276474fcb2ac46
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin de75e407e53af5a54b265261bc886e167626ce98 d5a30458f812ae2b851a367efda1d428d90ddaa6 && git checkout -B drift-repro de75e407e53af5a54b265261bc886e167626ce98 && git merge --no-ff d5a30458f812ae2b851a367efda1d428d90ddaa6

node scripts/docs-audit/affected-docs.mjs --json de75e407e53af5a54b265261bc886e167626ce98

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

@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Queue note — third group run; one honest shard-5 red so far, no removal — PM seat domain:devx @ objectstack (#6023), session session_01Vbw3RPgdtqesx4azk9SbW8, 2026-09-06T20:45Z.

group run created Test Core (5/6) aggregate outcome
34057177361 20:10:49Z success 23m19s success 20:34Z base moved — regrouped, not merged
34057283911 20:12:51Z cancelled at 30m19s (20:43:30Z) failure 20:43Z (the #16316 attestation gate) regrouped again
34058893500 20:43:58Z running

Not this PR's failure: the diff is the lint flow walker + tests + a changeset; every PR-side row at d5a30458f was green. Shard 5 is the CLI slice whose predicted time is stale (#16173, pm:awaiting-maintainer for the dataset refresh — the only lever). The queue has regrouped rather than removed, so the seat's one re-queue stays unspent; the policy from the standing rule holds (a removal ⇒ one re-queue; a second removal ⇒ park citing #16173). ⛔ No test skipped, no shard retried by hand, no empty commit. Next read 21:18Z.


Generated by Claude Code

Merged via the queue into main with commit dff0bdd Sep 6, 2026
35 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-16111-flow-template-leaves-outside-filters branch September 6, 2026 21:10
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