Skip to content

feat(lint): widen flow-trigger-unroutable to the omission shape (#7215) #7273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-project-manager merged 1 commit into from
Aug 10, 2026
Merged

feat(lint): widen flow-trigger-unroutable to the omission shape (#7215) #7273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .changeset/lint-flow-trigger-unroutable-omission.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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` 也不
触发"的新用例。
111 changes: 90 additions & 21 deletions packages/lint/src/validate-flow-trigger-readiness.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
Expand Down Expand Up @@ -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({
Expand Down
72 changes: 45 additions & 27 deletions packages/lint/src/validate-flow-trigger-readiness.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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';

Expand Down Expand Up @@ -560,25 +566,33 @@ 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
// different one ("mis-bound", not "never bound"), with its own
// 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 ||
Expand All @@ -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
Expand All @@ -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, ` +
Expand Down
Loading