Uh oh!
There was an error while loading. Please reload this page.
lint(flows): warn on an uncontained loop body and on a try_catch with no catch, and document loop { try_catch } - #14617
Conversation
…no catch
Two authoring-time rules in the flow anti-pattern family, both warnings:
`flow-loop-body-uncontained` (a `loop` body running a fallible node with no
`try_catch` between the loop and it) and `flow-try-catch-without-catch` (the
near-miss shape that gives zero containment while looking like containment).
Documents `loop { try_catch { … } }` in the flow docs as the per-iteration
containment spelling, with the measured minimal `catch` — one bare `assignment`
node, `edges` and `errorVariable` omitted — and the two empty-`catch` spellings
the region schema refuses.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV`check:doc-authoring` refuses a NEW internal issue id in string prose a runtime surface shows an author: the ids move to the adjacent comments, where the reader who can resolve them already looks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV
…op-containment-lint
📓 Docs Drift CheckThis PR changes 1 package(s): 22 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. 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 436833382a9e7c7309fb841bc87488817c2722a6 && git checkout 436833382a9e7c7309fb841bc87488817c2722a6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2514d49f388e898e666ae04f19ba376d04db5422 126f9d5e0576d3bb0ee286267158eb3440c387b7 && git checkout -B drift-repro 2514d49f388e898e666ae04f19ba376d04db5422 && git merge --no-ff 126f9d5e0576d3bb0ee286267158eb3440c387b7
node scripts/docs-audit/affected-docs.mjs --json 2514d49f388e898e666ae04f19ba376d04db5422
|
Uh oh!
There was an error while loading. Please reload this page.
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 33653485310 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 33665522337 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes#14394
B-branch items 1-2 of #13681: two authoring-time lint rules for per-iteration containment, plus the docs orthography. No
packages/specchange, no engine change, noloop.config.onIterationError— the ruling of record selected branch B by measurement (the combination already works), so this card is lint + docs only.The two rules (both
warning)flow-loop-body-uncontained— aloopwhosebodyregion runs a fallible node with notry_catchbetween the loop and that node.loop-node.tsiterates with a bareawaitand has notry/catchat all, so the first failing item ends the whole run. The finding names the loop and the node inwhereand the loop again in the message; the hint carries the prescribed spelling and the measured minimalcatch.flow-try-catch-without-catch— atry_catchwith nocatchregion, anywhere in a flow (not only inside a loop). Measured on the real engine: the no-catchrun and the unwrapped control produce identical output. This is the family's first target, not an extra — an author who wrapped the node and stopped there previously got zero containment and zero diagnostics.Both stay warnings under the file's own severity bar: a loop deliberately allowed to stop at the first bad row, and a retry-then-fail
try_catch, are legitimate readings the rules cannot disprove.Decisions this PR makes, stated
The fallible set is a closed literal list with a per-type justification read off each executor (
FALLIBLE_NODE_TYPESdocblock cites file and line for every member):get_record,create_record,update_record,delete_record,http,notify,connector_action,script,subflow,map,approval. Deliberately absent, also by reading the executor:assignmentanddecision(every path returns success),waitandscreen(they suspend and have no failure return),start/end(sentinels a region may not carry), and the three containersloop/parallel/try_catch(they fail only by propagation, so the walk descends to the leaf that carries the real failure instead of double-reporting the wrapper).Unknown (plugin-registered) node types are NOT counted fallible. An unread executor can certainly return
success: false, so counting them would be defensible for a warning — but it would flag nodes nobody here has read, and this family's precedent (INERT_CONDITION_NODE_TYPES) is that membership means "we have read this executor". The cost is a false negative on a third-party node; the alternative is a false positive on every one of them. Stated in the rule's docblock, pinned by a test.One node, one rule. Rule A treats ANY enclosing
try_catchas containment — including a catch-less one, which contains nothing — precisely so the near-miss is reported once, by rule B, which is the rule that can name the missing key. Telling an author who wrapped their node that they must wrap it is the one thing worth avoiding. Both docblocks say so, and a test pins the single finding.Ancestry, not depth. A
try_catchinside aparallelbranch inside the loop still contains; a fallible node three levels down with notry_catchon the path still kills the sweep. The descent walks through any container using the shared region-slot table, and stops at a nestedloop— that loop is judged in its own graph, so a nested body is never reported once per enclosing loop.Acceptance (from the issue), each line against a test name
rule A ... > flags the fallible node, naming the loop AND the node(assertswhere=flow 'case_sla_monitor' · loop 'each' body · node 'notify_owner' (notify), message containsinside loop 'each',severityundefined).try_catch-without-catchfixture gives a warning —rule B ... > flags the near-miss inside a loop body, and says it dies like an unwrapped node, plus... > flags a TOP-LEVEL catch-less try_catch tooand... > says what retry does and does not buy, when one is declared.loop { try_catch { try, catch: [assignment] } }fixture gives no finding —the documented spelling ... > raises NO finding from either containment ruleand... > raises no finding from ANY flow rule.FlowSchema—the documented spelling ... > parses under FlowSchema — the docs example is authorable(same ids and labels as the page).Additional controls: every fallible builtin flagged once each; the non-fallible builtins left alone; an unknown node type left alone; a legacy flat-graph loop (no
body) left alone; aparallelbranch reported with the branch inwhere; a nested loop reported once against the inner loop; a present-but-malformedcatchleft to the schema (withTryCatchConfigSchema.safeParsepinning thatcatch: {}andcatch: { nodes: [] }are refused and the minimal spelling is accepted).Eight pre-existing cases in this file asserted a bare total finding count on fixtures whose loop bodies carry a write or a
notify; they are now scoped to the rule under test, with a comment saying why. One of them instead pins the co-occurrence explicitly ([FLOW_RUNAS_UNSCOPED, FLOW_LOOP_BODY_UNCONTAINED]) rather than filtering it away.Docs
content/docs/automation/flows.mdx, new subsection#### Per-iteration containment: loop { try_catch { … } }under### Loop container: why an uncontained body dies, the measured numbers on both sides, the full example, the refusal table (catchomitted contains nothing;catch: {}andcatch: { nodes: [] }refused, the region'snodesbeing.min(1)), the minimal handler (one bareassignment,edgesanderrorVariableomitted) and the two rule ids. The existing### Loop containerexample no longer teaches the warned shape — its body node is wrapped.### Try / catch / retrynow states that omittingcatchgives zero containment and points at the subsection.Verification
Command:
pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/lint-flow-patterns.test.ts→Test Files 1 passed (1) · Tests 132 passed (132)(117 before this PR). Run through the shared verify lock, on this head.pnpm --filter @objectstack/lint typecheck→ exit 0, and itscheck:test-typecheckleg compiles the test layer undertsconfig.test.json, so the new test file really is type-checked.pnpm lint(the full-repoeslint . --no-inline-config) → exit 0 on this head. Not narrowed.Gate union derived in this worktree with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(57 families: 53 by path, 6 by change kind, 2 shared; the union was re-derived after the merge and is unchanged). All 57 run on the final commit126f9d5e0: 54 RAN-PASS, 3 NOT MEASURED, each refusing on a prerequisite in its own words rather than reporting a finding:node scripts/check-test-completeness.mjs— exit 3, "this gate grades a savedturbo run testlog, and no log was named ... the local reading for this gate is NOT MEASURED".pnpm check:dual-build-cjs-loads— exit 3, "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist" (83 packages listed). Self-test green (93 cases).pnpm check:type-check-debt— exit 3, "PREREQUISITE NOT MET"; it needs the whole built workspace closure. Self-test green.pnpm --filter @objectstack/spec run check:skill-examplesfirst refused the same way ("packages/client-react/dist holds no .d.ts declarations ... a verdict now would be computed against a build that no longer matches src"); after building that closure it RAN and passed: "261 prose examples type-check across 3 surface(s) — every marked block parsed, so tsc ran the SEMANTIC pass on all of them". It is counted in the 54.check:doc-authoringfailed once, genuinely: the two new hints carried(#13681, #14394)in runtime string prose, which the gate refuses for a NEW (file, id) pair — "a runtime string reaches authors, operators and generated surfaces — none of whom can resolve#NNNN". The ids moved to the adjacent comments and it is green.git diff --stat origin/main...HEAD— the five declared paths and nothing else:Changeset:
.changeset/flow-loop-per-iteration-containment-lint.md,@objectstack/lintpatch.Out of scope and untouched, per the card: the run-summary visibility half (
FlowRunSummarySchema, the iteration index on nested try/catch steps,$errorrow identity) stays on #13681, and #13803 is not addressed here.Draft on purpose: not ready, no auto-merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV
Generated by Claude Code
Generated by Claude Code