From 1bb3cd8fab9472ac057bbcc4cafdd0dbd327724a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 04:21:20 +0000 Subject: [PATCH] feat(lint): widen flow-trigger-unroutable to the omission shape (#7215) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `flow-trigger-unroutable` (#6637) fired only on the contradiction shape: `config.triggerType` present but the engine routes it nowhere. Per #7215 (maintainer-ruled, disposition 1), it now also fires on the omission shape: `type: 'record_change'` with no `triggerType` key at all. Both shapes fall through `resolveTriggerBinding` the same way and are equally dead at runtime, so they share the id and severity rather than getting a new one. Corpus-verified before landing: the only two real `type: 'record_change'` flows in the tree (examples/app-todo's TaskCompletionFlow, repaired by #7039, and examples/app-showcase's UrgentTaskAlertFlow) both already declare an explicit triggerType — zero omission instances, so the widening ships green with no baseline churn. Deletes the scope-boundary pin test in validate-flow-trigger-readiness.test.ts per its own instruction that widening "is a deliberate edit that has to delete this test, not a side effect" — replaced with tests for the omission shape firing, staying silent on non-record_change flow types, and staying silent when a routed sibling key is present. Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn Co-authored-by: Claude --- .../lint-flow-trigger-unroutable-omission.md | 44 +++++++ .../validate-flow-trigger-readiness.test.ts | 111 ++++++++++++++---- .../src/validate-flow-trigger-readiness.ts | 72 +++++++----- 3 files changed, 179 insertions(+), 48 deletions(-) create mode 100644 .changeset/lint-flow-trigger-unroutable-omission.md diff --git a/.changeset/lint-flow-trigger-unroutable-omission.md b/.changeset/lint-flow-trigger-unroutable-omission.md new file mode 100644 index 0000000000..8050bdb261 --- /dev/null +++ b/.changeset/lint-flow-trigger-unroutable-omission.md @@ -0,0 +1,44 @@ +--- +"@objectstack/lint": minor +--- + +feat(lint): `flow-trigger-unroutable` 收紧到"省略"形态 —— 无 `triggerType` 的 `record_change` flow 同样报错 (#7215) + +`flow-trigger-unroutable`(#6637)此前只判"矛盾"形态:`config.triggerType` **存在**但引擎路由不到 +任何 trigger(如 `triggerType: 'onCreate'`)。本次按 #7215(维护者裁定,方案一:现在就收紧)扩展到 +"省略"形态:`type: 'record_change'` 且**完全没有** `triggerType` 这个键。两种写法在运行期是同一个 +缺陷 —— `AutomationEngine.resolveTriggerBinding` 对两者走的是同一条回退链,最终都返回 +`undefined`,flow 被静默降级为手动 flow,连 `getTriggerBindingAudit` 都因为"看起来像手动/screen +flow"而跳过它,不会在任何地方点名。因此复用同一个 rule id 与同一档 severity(`error`),而不是新开 +一条 —— 这是同一个缺陷的两种写法,不是两个缺陷。 + +## 为什么现在收紧,而不是 #6637 立规则时就收紧 + +#6637 立规则时,语料测出一个真实的省略实例:`examples/app-todo` 的 `TaskCompletionFlow`。当时把它 +判死,等于在一个已发布的示例 app 上,对该 app 的语义意图下一个未经确认的猜测,所以判据当时要求 +`triggerType` 这个 key 必须**存在**,省略形态被单独立卡搁置(#7041 item 2)。#7039 已经把那个实例 +修好 —— `TaskCompletionFlow` 现在显式声明 `triggerType: 'record-after-update'` 并正确路由 —— +语料窗口转绿,#7215 因此裁定:一个零命中规则,只要缺陷类别有过真实实例(学费已经交过)、oracle 是 +封闭的(能不能路由是引擎自己的硬编码链,不是猜测)、当下语料对它是绿的(收紧不产生 churn)、 +severity 与危害匹配,就应当趁窗口开着落地,而不是等下一个省略实例出现、把落地成本重新推高。 + +## 语料计数(先测,后收紧) + +用生产入口(`validateFlowTriggerReadiness`)跑过本仓 `examples/`、`apps/`、`packages/` 下按内容 +搜索到的**每一个** `type: 'record_change'` 真实 flow 定义 —— 全库只有两个: +`examples/app-todo/src/flows/task.flow.ts`(`TaskCompletionFlow`)与 +`examples/app-showcase/src/automation/flows/index.ts`(`UrgentTaskAlertFlow`),两者都已显式声明 +`triggerType`。**省略实例命中数为 0**。收紧后的判据在整棵树上是绿的:不产生任何新的 baseline 条目, +不需要修任何示例 app。 + +## 判据里没有变的部分 + +"这条规则不判的第二种形态"维持原样:一个 `record_change` flow 若同时声明了引擎**确实**会路由的东西 +(`config.schedule`、`triggerType: 'api'`、`config.timeRelative` 对象),它会按错误的 trigger 绑定 +并触发 —— 这是一个不同的缺陷("绑错"而不是"没绑上"),仍然不是这条规则要判的,`routesToSomeTrigger` +分支字符对字符保持不变。 + +`validate-flow-trigger-readiness.test.ts` 里原先钉住"省略形态故意不判"的边界测试(其自身注释写明 +"收紧是必须删掉这个测试的有意行为,不是顺手带过的副作用")已按 #7215 删除,替换为覆盖"省略形态触发"、 +"省略但有其它路由 sibling 时不触发"、"非 `record_change` 类型的 flow 即使省略 `triggerType` 也不 +触发"的新用例。 diff --git a/packages/lint/src/validate-flow-trigger-readiness.test.ts b/packages/lint/src/validate-flow-trigger-readiness.test.ts index a5300b44fc..2e21b8f1e2 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.test.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.test.ts @@ -798,6 +798,51 @@ describe('validateFlowTriggerReadiness', () => { expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false); }); + // ── #7215 — the omission shape ──────────────────────────────────────── + // + // Widened from the original #6637 contradiction-only criterion (`triggerType` + // present but off-grammar) to also cover `triggerType` ABSENT entirely. Both + // fall through `AutomationEngine.resolveTriggerBinding` to `undefined` by the + // exact same chain, so both are equally dead at runtime — two spellings of + // one defect, not a new one. + // + // At #6637 time the omission shape had a live instance in `examples/app-todo` + // (`TaskCompletionFlow`, #6882) whose repair was a judgement about that app's + // semantics rather than a lint decision, so covering it then would have gated + // a shipped example app on a guess — the criterion required the key to be + // PRESENT and the omission was tracked separately (#7041 item 2, carried by + // the now-deleted `an ABSENT triggerType` pin this block replaces). #7039 + // repaired that instance and #7215 (maintainer-ruled, disposition 1) widened + // the criterion now that the corpus is green for it (verified at branch time: + // zero omission instances in-tree). + it('flags a record_change flow with triggerType entirely absent (the omission shape)', () => { + const findings = validateFlowTriggerReadiness(unroutable({})); + expect(findings.map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNROUTABLE]); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType'); + expect(findings[0].message).toMatch(/no triggerType at all/i); + // The message must not misreport the absent value as the literal word + // "undefined" (the `renderNonObject`/`renderTriggerToken` lesson applied + // to this new branch too). + expect(findings[0].message).not.toContain('undefined'); + }); + + it('reports the same rule, severity and path whether the token is wrong or missing', () => { + // "Same rule, same severity: two spellings of one defect" is the issue's + // own framing — assert it directly rather than trusting two separate + // tests to agree by construction. + const present = validateFlowTriggerReadiness(unroutable({ triggerType: 'onCreate' }))[0]; + const absent = validateFlowTriggerReadiness(unroutable({}))[0]; + expect(absent.rule).toBe(present.rule); + expect(absent.severity).toBe(present.severity); + expect(absent.path).toBe(present.path); + // Both still carry the measured audit claim from 1f's comment. + expect(absent.message).toMatch(/record_change/); + expect(absent.message).toMatch(/never fires/i); + expect(absent.message).toMatch(/audit/i); + expect(absent.message).toMatch(/count/i); + }); + describe('does NOT flag (each paired with the mutation that makes it fire)', () => { it('a canonical record-* token — the whole point of declaring record_change', () => { expect(validateFlowTriggerReadiness(unroutable({ triggerType: 'record-after-update' }))).toEqual([]); @@ -859,32 +904,56 @@ describe('validateFlowTriggerReadiness', () => { ).toEqual([FLOW_TRIGGER_UNROUTABLE]); }); - it('an ABSENT triggerType — dead too, deliberately deferred (#6637 corpus)', () => { - // Scope boundary, pinned rather than left to memory. A `record_change` - // flow with no triggerType at all resolves to no binding by the same - // fall-through and is just as dead — but it is an omission rather than a - // contradiction, and at the time this criterion was cut (#6637) the - // corpus measurement found a LIVE instance of it in `examples/app-todo` - // (`TaskCompletionFlow`, tracked as #6882). Covering it then would have - // gated a shipped example app on a guess about that app's semantics, so - // the criterion requires the key to be PRESENT and the omission case was - // filed separately. That instance has since been repaired by #7039 — - // `TaskCompletionFlow` now declares `triggerType: 'record-after-update'` - // and routes correctly, so there is no live instance in the tree as of - // this writing. Whether the omission shape should now be covered too is - // a separate, undecided question (#7041 item 2) — this test still pins - // the deliberate non-coverage of it. Widening this is then a deliberate - // edit that has to delete this test, not a side effect of touching the - // predicate. + it('an omission with a sibling key the engine DOES route — same exclusion, absent token', () => { + // The omission counterpart of the test above (#7215). `triggerType` is + // missing outright, but something else on the start node routes — the + // same `routesToSomeTrigger` chain that excludes the present-but-routed + // case excludes this one too, character for character. + for (const extra of [ + { schedule: { type: 'interval', intervalMs: 60000 } }, + { timeRelative: { object: 'app_candidate', dateField: 'due_at', withinDays: 7 } }, + ]) { + expect( + validateFlowTriggerReadiness(unroutable({ ...extra })).map((f) => f.rule), + JSON.stringify(extra), + ).not.toContain(FLOW_TRIGGER_UNROUTABLE); + } + // Drop the routed sibling and the same (still keyless) flow is dead. expect( validateFlowTriggerReadiness(unroutable({})).map((f) => f.rule), - ).not.toContain(FLOW_TRIGGER_UNROUTABLE); - // Non-vacuous: add the key back, and the same fixture fires. - expect( - validateFlowTriggerReadiness(unroutable({ triggerType: 'onCreate' })).map((f) => f.rule), ).toEqual([FLOW_TRIGGER_UNROUTABLE]); }); + it('a genuinely manual flow with no triggerType at all — autolaunched/screen stay silent (#7215)', () => { + // The omission widening still only speaks for `record_change`: 1f's + // guard is `flow.type === 'record_change'`, so a flow that is + // legitimately manual (`autolaunched`/`screen`, which have no + // triggerType to be missing) must not start getting flagged just + // because the key happens to be absent. Mirrors the present-token + // scope-boundary test above, one key over. + for (const type of ['autolaunched', 'screen']) { + expect( + validateFlowTriggerReadiness(unroutable({}, { type })).map((f) => f.rule), + type, + ).not.toContain(FLOW_TRIGGER_UNROUTABLE); + } + // Same fixture, type flipped back to record_change: now it fires. + expect( + validateFlowTriggerReadiness(unroutable({}, { type: 'record_change' })).map((f) => f.rule), + ).toContain(FLOW_TRIGGER_UNROUTABLE); + }); + + it('every non-record_change flow type stays silent with triggerType absent — flow.type is read literally', () => { + // Enumerated from the code rather than guessed: 1f's guard is + // `flow.type === 'record_change'`, an exact string match against + // `Flow.type`'s enum. Every other declared type — plus a flow with no + // `type` at all — never reaches this criterion, omission or not. + for (const type of ['autolaunched', 'screen', 'schedule', 'api', undefined]) { + const findings = validateFlowTriggerReadiness(unroutable({}, { type })); + expect(findings.map((f) => f.rule), String(type)).not.toContain(FLOW_TRIGGER_UNROUTABLE); + } + }); + it('a flow with no start node at all', () => { expect( validateFlowTriggerReadiness({ diff --git a/packages/lint/src/validate-flow-trigger-readiness.ts b/packages/lint/src/validate-flow-trigger-readiness.ts index 905252bef6..c7b4a49874 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.ts @@ -43,11 +43,13 @@ // the one bind-time warn rule 3's case gets. // // 5. A `type: 'record_change'` flow whose start-node `triggerType` the engine -// routes NOWHERE — `triggerType: 'onCreate'` (#6637). The quietest member -// of the family: rules 3 and 4 are about one key's shape, this one is -// about a flow that declares WHAT it is and then contradicts it. See 1f -// for the measured silence — every named runtime channel skips it because -// they all key off the same resolution that already gave up. +// routes NOWHERE — present but off-grammar (`triggerType: 'onCreate'`, +// #6637's original specimen) or absent entirely (the omission shape, +// widened into this same id by #7215 once #7039 had repaired the corpus's +// one live instance). Both are a flow that declares WHAT it is and then +// never arms it — two spellings of one defect. See 1f for the measured +// silence — every named runtime channel skips it because they all key off +// the same resolution that already gave up. // // The spec import is deliberate and is what makes rule 3 possible without a // second copy of the descriptor's shape living in this file. It stays inside the @@ -150,7 +152,10 @@ export const FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE = 'flow-time-relative-desc /** * #6637 — a `type: 'record_change'` flow whose start-node `triggerType` the * engine routes to NO trigger at all, so the flow is silently demoted to a - * manual one. + * manual one. Widened by #7215 to also cover the token being ABSENT + * entirely — the omission shape is exactly as dead at runtime as the + * contradiction shape this id originally caught, so one id and one severity + * cover both (see 1f for the history of why the widening waited). * * A separate id from `flow-trigger-unknown-event`, on the same distinction that * separates the two `timeRelative` ids: whether the engine ROUTES the value. @@ -161,9 +166,10 @@ export const FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE = 'flow-time-relative-desc * ROUTES it to the record-change trigger, which maps it to zero hook events * and says so in a bind-time warn. That is `…-UNKNOWN-EVENT`, and this rule * file moves that warn earlier. - * - anything else (`onCreate`, `on_update`, `''`, `['onCreate']`) — the engine - * routes it NOWHERE. That is this id, and there is no runtime channel to - * move earlier from: see 1f for the three call sites that each skip it. + * - anything else (`onCreate`, `on_update`, `''`, `['onCreate']`, or the key + * absent entirely) — the engine routes it NOWHERE. That is this id, and + * there is no runtime channel to move earlier from: see 1f for the three + * call sites that each skip it. */ export const FLOW_TRIGGER_UNROUTABLE = 'flow-trigger-unroutable'; @@ -560,18 +566,8 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines // are the types a genuinely manual flow declares, and neither reaches // here. That is what makes this decidable at authoring time. // - // Two shapes are deliberately NOT this rule's, each pinned by a test: + // One shape is deliberately NOT this rule's, pinned by a test: // - // - `triggerType` ABSENT on a `record_change` flow. Dead the same way - // and arguably worse, but it is an omission rather than a - // contradiction, and the corpus measurement (#6637) found a live - // instance of it in `examples/app-todo` whose repair is a judgement - // about that app's semantics, not a lint decision (#6882 — the flow - // also writes its predicate to a `triggerCondition` key nothing - // reads, so arming it is not a one-token edit). Widening this - // criterion to cover it would gate a shipped example app on a guess. - // The criterion here requires the key to be PRESENT so that widening - // is a deliberate act, not a side effect. // - a `record_change` flow that ALSO declares something the engine // does route (`config.schedule`, `triggerType: 'api'`). That flow // binds and fires — on the wrong trigger's terms. A real defect, a @@ -579,6 +575,24 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines // severity argument to make. `routesToSomeTrigger` below is the // engine's chain character for character precisely so this rule // stays silent there instead of guessing at a second verdict. + // + // The criterion covers BOTH ways a `record_change` flow ends up + // unrouted: `triggerType` PRESENT but off-grammar (the contradiction — + // `triggerType: 'onCreate'`, #6637's original specimen) and + // `triggerType` ABSENT entirely (the omission — dead the same way, + // arguably worse, and previously excluded on purpose). They were not + // always one rule's concern: at #6637 time the omission shape had a + // live instance in `examples/app-todo` (`TaskCompletionFlow`, #6882) + // whose repair was a judgement about that app's semantics rather than a + // lint decision, so covering the omission then would have gated a + // shipped example app on a guess — the criterion required the key to be + // PRESENT and the omission was tracked separately (#7041 item 2). + // #7039 repaired that instance (`TaskCompletionFlow` now declares + // `triggerType: 'record-after-update'`), which put a green corpus + // under the open question; #7215 (maintainer-ruled) decided to widen + // now rather than wait for the next omission instance to make landing + // costly again. Both shapes are equally dead at runtime — two + // spellings of one defect — so they share this id and severity. const routesToSomeTrigger = isRecordTriggered || isArrayRecordTriggered || @@ -587,7 +601,8 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines flow.type === 'schedule' || flow.type === 'api' || triggerType === 'api'; - if (start && flow.type === 'record_change' && config.triggerType != null && !routesToSomeTrigger) { + if (start && flow.type === 'record_change' && !routesToSomeTrigger) { + const hasTriggerType = config.triggerType != null; findings.push({ // `error` (#5762's criterion, applied to a fourth id). The verdict is // the engine's own routing chain — literal `startsWith`/`typeof` tests @@ -601,12 +616,15 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, message: - `declares type: 'record_change' but its start node's triggerType is ` + - `${renderTriggerToken(config.triggerType)}, which the engine routes to NO trigger — it binds a ` + - `record-change flow only for a token starting with 'record-', so this flow is demoted to a manual ` + - `one and never fires. Nothing NAMES it: the unbound-flow audit resolves the same binding and skips ` + - `the flow as "manual — nothing to bind", so neither the boot warning nor the startup summary lists ` + - `it; the only trace is the banner's flow count being one higher than its bound count.`, + `declares type: 'record_change' but ` + + (hasTriggerType + ? `its start node's triggerType is ${renderTriggerToken(config.triggerType)}, which the engine ` + + `routes to NO trigger` + : `its start node has no triggerType at all, so there is nothing for the engine to route`) + + ` — it binds a record-change flow only for a token starting with 'record-', so this flow is demoted ` + + `to a manual one and never fires. Nothing NAMES it: the unbound-flow audit resolves the same binding ` + + `and skips the flow as "manual — nothing to bind", so neither the boot warning nor the startup ` + + `summary lists it; the only trace is the banner's flow count being one higher than its bound count.`, hint: `Use record-{before,after}-{create,update,delete,write} ('write' is create OR update in one flow, ` + `#3427; create/insert are synonyms). If the flow really is launched by hand or from a screen, ` +