Skip to content

Commit 1bb3cd8

Browse files
committed
feat(lint): widen flow-trigger-unroutable to the omission shape (#7215)
`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 <noreply@anthropic.com>
1 parent 0caf122 commit 1bb3cd8

3 files changed

Lines changed: 179 additions & 48 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
feat(lint): `flow-trigger-unroutable` 收紧到"省略"形态 —— 无 `triggerType``record_change` flow 同样报错 (#7215)
6+
7+
`flow-trigger-unroutable`(#6637)此前只判"矛盾"形态:`config.triggerType` **存在**但引擎路由不到
8+
任何 trigger(如 `triggerType: 'onCreate'`)。本次按 #7215(维护者裁定,方案一:现在就收紧)扩展到
9+
"省略"形态:`type: 'record_change'`**完全没有** `triggerType` 这个键。两种写法在运行期是同一个
10+
缺陷 —— `AutomationEngine.resolveTriggerBinding` 对两者走的是同一条回退链,最终都返回
11+
`undefined`,flow 被静默降级为手动 flow,连 `getTriggerBindingAudit` 都因为"看起来像手动/screen
12+
flow"而跳过它,不会在任何地方点名。因此复用同一个 rule id 与同一档 severity(`error`),而不是新开
13+
一条 —— 这是同一个缺陷的两种写法,不是两个缺陷。
14+
15+
## 为什么现在收紧,而不是 #6637 立规则时就收紧
16+
17+
#6637 立规则时,语料测出一个真实的省略实例:`examples/app-todo``TaskCompletionFlow`。当时把它
18+
判死,等于在一个已发布的示例 app 上,对该 app 的语义意图下一个未经确认的猜测,所以判据当时要求
19+
`triggerType` 这个 key 必须**存在**,省略形态被单独立卡搁置(#7041 item 2)。#7039 已经把那个实例
20+
修好 —— `TaskCompletionFlow` 现在显式声明 `triggerType: 'record-after-update'` 并正确路由 ——
21+
语料窗口转绿,#7215 因此裁定:一个零命中规则,只要缺陷类别有过真实实例(学费已经交过)、oracle 是
22+
封闭的(能不能路由是引擎自己的硬编码链,不是猜测)、当下语料对它是绿的(收紧不产生 churn)、
23+
severity 与危害匹配,就应当趁窗口开着落地,而不是等下一个省略实例出现、把落地成本重新推高。
24+
25+
## 语料计数(先测,后收紧)
26+
27+
用生产入口(`validateFlowTriggerReadiness`)跑过本仓 `examples/``apps/``packages/` 下按内容
28+
搜索到的**每一个** `type: 'record_change'` 真实 flow 定义 —— 全库只有两个:
29+
`examples/app-todo/src/flows/task.flow.ts`(`TaskCompletionFlow`)与
30+
`examples/app-showcase/src/automation/flows/index.ts`(`UrgentTaskAlertFlow`),两者都已显式声明
31+
`triggerType`**省略实例命中数为 0**。收紧后的判据在整棵树上是绿的:不产生任何新的 baseline 条目,
32+
不需要修任何示例 app。
33+
34+
## 判据里没有变的部分
35+
36+
"这条规则不判的第二种形态"维持原样:一个 `record_change` flow 若同时声明了引擎**确实**会路由的东西
37+
(`config.schedule``triggerType: 'api'``config.timeRelative` 对象),它会按错误的 trigger 绑定
38+
并触发 —— 这是一个不同的缺陷("绑错"而不是"没绑上"),仍然不是这条规则要判的,`routesToSomeTrigger`
39+
分支字符对字符保持不变。
40+
41+
`validate-flow-trigger-readiness.test.ts` 里原先钉住"省略形态故意不判"的边界测试(其自身注释写明
42+
"收紧是必须删掉这个测试的有意行为,不是顺手带过的副作用")已按 #7215 删除,替换为覆盖"省略形态触发"、
43+
"省略但有其它路由 sibling 时不触发"、"非 `record_change` 类型的 flow 即使省略 `triggerType` 也不
44+
触发"的新用例。

packages/lint/src/validate-flow-trigger-readiness.test.ts

Lines changed: 90 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,51 @@ describe('validateFlowTriggerReadiness', () => {
798798
expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false);
799799
});
800800

801+
// ── #7215 — the omission shape ────────────────────────────────────────
802+
//
803+
// Widened from the original #6637 contradiction-only criterion (`triggerType`
804+
// present but off-grammar) to also cover `triggerType` ABSENT entirely. Both
805+
// fall through `AutomationEngine.resolveTriggerBinding` to `undefined` by the
806+
// exact same chain, so both are equally dead at runtime — two spellings of
807+
// one defect, not a new one.
808+
//
809+
// At #6637 time the omission shape had a live instance in `examples/app-todo`
810+
// (`TaskCompletionFlow`, #6882) whose repair was a judgement about that app's
811+
// semantics rather than a lint decision, so covering it then would have gated
812+
// a shipped example app on a guess — the criterion required the key to be
813+
// PRESENT and the omission was tracked separately (#7041 item 2, carried by
814+
// the now-deleted `an ABSENT triggerType` pin this block replaces). #7039
815+
// repaired that instance and #7215 (maintainer-ruled, disposition 1) widened
816+
// the criterion now that the corpus is green for it (verified at branch time:
817+
// zero omission instances in-tree).
818+
it('flags a record_change flow with triggerType entirely absent (the omission shape)', () => {
819+
const findings = validateFlowTriggerReadiness(unroutable({}));
820+
expect(findings.map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNROUTABLE]);
821+
expect(findings[0].severity).toBe('error');
822+
expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType');
823+
expect(findings[0].message).toMatch(/no triggerType at all/i);
824+
// The message must not misreport the absent value as the literal word
825+
// "undefined" (the `renderNonObject`/`renderTriggerToken` lesson applied
826+
// to this new branch too).
827+
expect(findings[0].message).not.toContain('undefined');
828+
});
829+
830+
it('reports the same rule, severity and path whether the token is wrong or missing', () => {
831+
// "Same rule, same severity: two spellings of one defect" is the issue's
832+
// own framing — assert it directly rather than trusting two separate
833+
// tests to agree by construction.
834+
const present = validateFlowTriggerReadiness(unroutable({ triggerType: 'onCreate' }))[0];
835+
const absent = validateFlowTriggerReadiness(unroutable({}))[0];
836+
expect(absent.rule).toBe(present.rule);
837+
expect(absent.severity).toBe(present.severity);
838+
expect(absent.path).toBe(present.path);
839+
// Both still carry the measured audit claim from 1f's comment.
840+
expect(absent.message).toMatch(/record_change/);
841+
expect(absent.message).toMatch(/never fires/i);
842+
expect(absent.message).toMatch(/audit/i);
843+
expect(absent.message).toMatch(/count/i);
844+
});
845+
801846
describe('does NOT flag (each paired with the mutation that makes it fire)', () => {
802847
it('a canonical record-* token — the whole point of declaring record_change', () => {
803848
expect(validateFlowTriggerReadiness(unroutable({ triggerType: 'record-after-update' }))).toEqual([]);
@@ -859,32 +904,56 @@ describe('validateFlowTriggerReadiness', () => {
859904
).toEqual([FLOW_TRIGGER_UNROUTABLE]);
860905
});
861906

862-
it('an ABSENT triggerType — dead too, deliberately deferred (#6637 corpus)', () => {
863-
// Scope boundary, pinned rather than left to memory. A `record_change`
864-
// flow with no triggerType at all resolves to no binding by the same
865-
// fall-through and is just as dead — but it is an omission rather than a
866-
// contradiction, and at the time this criterion was cut (#6637) the
867-
// corpus measurement found a LIVE instance of it in `examples/app-todo`
868-
// (`TaskCompletionFlow`, tracked as #6882). Covering it then would have
869-
// gated a shipped example app on a guess about that app's semantics, so
870-
// the criterion requires the key to be PRESENT and the omission case was
871-
// filed separately. That instance has since been repaired by #7039 —
872-
// `TaskCompletionFlow` now declares `triggerType: 'record-after-update'`
873-
// and routes correctly, so there is no live instance in the tree as of
874-
// this writing. Whether the omission shape should now be covered too is
875-
// a separate, undecided question (#7041 item 2) — this test still pins
876-
// the deliberate non-coverage of it. Widening this is then a deliberate
877-
// edit that has to delete this test, not a side effect of touching the
878-
// predicate.
907+
it('an omission with a sibling key the engine DOES route — same exclusion, absent token', () => {
908+
// The omission counterpart of the test above (#7215). `triggerType` is
909+
// missing outright, but something else on the start node routes — the
910+
// same `routesToSomeTrigger` chain that excludes the present-but-routed
911+
// case excludes this one too, character for character.
912+
for (const extra of [
913+
{ schedule: { type: 'interval', intervalMs: 60000 } },
914+
{ timeRelative: { object: 'app_candidate', dateField: 'due_at', withinDays: 7 } },
915+
]) {
916+
expect(
917+
validateFlowTriggerReadiness(unroutable({ ...extra })).map((f) => f.rule),
918+
JSON.stringify(extra),
919+
).not.toContain(FLOW_TRIGGER_UNROUTABLE);
920+
}
921+
// Drop the routed sibling and the same (still keyless) flow is dead.
879922
expect(
880923
validateFlowTriggerReadiness(unroutable({})).map((f) => f.rule),
881-
).not.toContain(FLOW_TRIGGER_UNROUTABLE);
882-
// Non-vacuous: add the key back, and the same fixture fires.
883-
expect(
884-
validateFlowTriggerReadiness(unroutable({ triggerType: 'onCreate' })).map((f) => f.rule),
885924
).toEqual([FLOW_TRIGGER_UNROUTABLE]);
886925
});
887926

927+
it('a genuinely manual flow with no triggerType at all — autolaunched/screen stay silent (#7215)', () => {
928+
// The omission widening still only speaks for `record_change`: 1f's
929+
// guard is `flow.type === 'record_change'`, so a flow that is
930+
// legitimately manual (`autolaunched`/`screen`, which have no
931+
// triggerType to be missing) must not start getting flagged just
932+
// because the key happens to be absent. Mirrors the present-token
933+
// scope-boundary test above, one key over.
934+
for (const type of ['autolaunched', 'screen']) {
935+
expect(
936+
validateFlowTriggerReadiness(unroutable({}, { type })).map((f) => f.rule),
937+
type,
938+
).not.toContain(FLOW_TRIGGER_UNROUTABLE);
939+
}
940+
// Same fixture, type flipped back to record_change: now it fires.
941+
expect(
942+
validateFlowTriggerReadiness(unroutable({}, { type: 'record_change' })).map((f) => f.rule),
943+
).toContain(FLOW_TRIGGER_UNROUTABLE);
944+
});
945+
946+
it('every non-record_change flow type stays silent with triggerType absent — flow.type is read literally', () => {
947+
// Enumerated from the code rather than guessed: 1f's guard is
948+
// `flow.type === 'record_change'`, an exact string match against
949+
// `Flow.type`'s enum. Every other declared type — plus a flow with no
950+
// `type` at all — never reaches this criterion, omission or not.
951+
for (const type of ['autolaunched', 'screen', 'schedule', 'api', undefined]) {
952+
const findings = validateFlowTriggerReadiness(unroutable({}, { type }));
953+
expect(findings.map((f) => f.rule), String(type)).not.toContain(FLOW_TRIGGER_UNROUTABLE);
954+
}
955+
});
956+
888957
it('a flow with no start node at all', () => {
889958
expect(
890959
validateFlowTriggerReadiness({

packages/lint/src/validate-flow-trigger-readiness.ts

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,13 @@
4343
// the one bind-time warn rule 3's case gets.
4444
//
4545
// 5. A `type: 'record_change'` flow whose start-node `triggerType` the engine
46-
// routes NOWHERE — `triggerType: 'onCreate'` (#6637). The quietest member
47-
// of the family: rules 3 and 4 are about one key's shape, this one is
48-
// about a flow that declares WHAT it is and then contradicts it. See 1f
49-
// for the measured silence — every named runtime channel skips it because
50-
// they all key off the same resolution that already gave up.
46+
// routes NOWHERE — present but off-grammar (`triggerType: 'onCreate'`,
47+
// #6637's original specimen) or absent entirely (the omission shape,
48+
// widened into this same id by #7215 once #7039 had repaired the corpus's
49+
// one live instance). Both are a flow that declares WHAT it is and then
50+
// never arms it — two spellings of one defect. See 1f for the measured
51+
// silence — every named runtime channel skips it because they all key off
52+
// the same resolution that already gave up.
5153
//
5254
// The spec import is deliberate and is what makes rule 3 possible without a
5355
// 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
150152
/**
151153
* #6637 — a `type: 'record_change'` flow whose start-node `triggerType` the
152154
* engine routes to NO trigger at all, so the flow is silently demoted to a
153-
* manual one.
155+
* manual one. Widened by #7215 to also cover the token being ABSENT
156+
* entirely — the omission shape is exactly as dead at runtime as the
157+
* contradiction shape this id originally caught, so one id and one severity
158+
* cover both (see 1f for the history of why the widening waited).
154159
*
155160
* A separate id from `flow-trigger-unknown-event`, on the same distinction that
156161
* 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
161166
* ROUTES it to the record-change trigger, which maps it to zero hook events
162167
* and says so in a bind-time warn. That is `…-UNKNOWN-EVENT`, and this rule
163168
* file moves that warn earlier.
164-
* - anything else (`onCreate`, `on_update`, `''`, `['onCreate']`) — the engine
165-
* routes it NOWHERE. That is this id, and there is no runtime channel to
166-
* move earlier from: see 1f for the three call sites that each skip it.
169+
* - anything else (`onCreate`, `on_update`, `''`, `['onCreate']`, or the key
170+
* absent entirely) — the engine routes it NOWHERE. That is this id, and
171+
* there is no runtime channel to move earlier from: see 1f for the three
172+
* call sites that each skip it.
167173
*/
168174
export const FLOW_TRIGGER_UNROUTABLE = 'flow-trigger-unroutable';
169175

@@ -560,25 +566,33 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines
560566
// are the types a genuinely manual flow declares, and neither reaches
561567
// here. That is what makes this decidable at authoring time.
562568
//
563-
// Two shapes are deliberately NOT this rule's, each pinned by a test:
569+
// One shape is deliberately NOT this rule's, pinned by a test:
564570
//
565-
// - `triggerType` ABSENT on a `record_change` flow. Dead the same way
566-
// and arguably worse, but it is an omission rather than a
567-
// contradiction, and the corpus measurement (#6637) found a live
568-
// instance of it in `examples/app-todo` whose repair is a judgement
569-
// about that app's semantics, not a lint decision (#6882 — the flow
570-
// also writes its predicate to a `triggerCondition` key nothing
571-
// reads, so arming it is not a one-token edit). Widening this
572-
// criterion to cover it would gate a shipped example app on a guess.
573-
// The criterion here requires the key to be PRESENT so that widening
574-
// is a deliberate act, not a side effect.
575571
// - a `record_change` flow that ALSO declares something the engine
576572
// does route (`config.schedule`, `triggerType: 'api'`). That flow
577573
// binds and fires — on the wrong trigger's terms. A real defect, a
578574
// different one ("mis-bound", not "never bound"), with its own
579575
// severity argument to make. `routesToSomeTrigger` below is the
580576
// engine's chain character for character precisely so this rule
581577
// stays silent there instead of guessing at a second verdict.
578+
//
579+
// The criterion covers BOTH ways a `record_change` flow ends up
580+
// unrouted: `triggerType` PRESENT but off-grammar (the contradiction —
581+
// `triggerType: 'onCreate'`, #6637's original specimen) and
582+
// `triggerType` ABSENT entirely (the omission — dead the same way,
583+
// arguably worse, and previously excluded on purpose). They were not
584+
// always one rule's concern: at #6637 time the omission shape had a
585+
// live instance in `examples/app-todo` (`TaskCompletionFlow`, #6882)
586+
// whose repair was a judgement about that app's semantics rather than a
587+
// lint decision, so covering the omission then would have gated a
588+
// shipped example app on a guess — the criterion required the key to be
589+
// PRESENT and the omission was tracked separately (#7041 item 2).
590+
// #7039 repaired that instance (`TaskCompletionFlow` now declares
591+
// `triggerType: 'record-after-update'`), which put a green corpus
592+
// under the open question; #7215 (maintainer-ruled) decided to widen
593+
// now rather than wait for the next omission instance to make landing
594+
// costly again. Both shapes are equally dead at runtime — two
595+
// spellings of one defect — so they share this id and severity.
582596
const routesToSomeTrigger =
583597
isRecordTriggered ||
584598
isArrayRecordTriggered ||
@@ -587,7 +601,8 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines
587601
flow.type === 'schedule' ||
588602
flow.type === 'api' ||
589603
triggerType === 'api';
590-
if (start && flow.type === 'record_change' && config.triggerType != null && !routesToSomeTrigger) {
604+
if (start && flow.type === 'record_change' && !routesToSomeTrigger) {
605+
const hasTriggerType = config.triggerType != null;
591606
findings.push({
592607
// `error` (#5762's criterion, applied to a fourth id). The verdict is
593608
// the engine's own routing chain — literal `startsWith`/`typeof` tests
@@ -601,12 +616,15 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines
601616
where: `flow "${flowName}" › start node`,
602617
path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`,
603618
message:
604-
`declares type: 'record_change' but its start node's triggerType is ` +
605-
`${renderTriggerToken(config.triggerType)}, which the engine routes to NO trigger — it binds a ` +
606-
`record-change flow only for a token starting with 'record-', so this flow is demoted to a manual ` +
607-
`one and never fires. Nothing NAMES it: the unbound-flow audit resolves the same binding and skips ` +
608-
`the flow as "manual — nothing to bind", so neither the boot warning nor the startup summary lists ` +
609-
`it; the only trace is the banner's flow count being one higher than its bound count.`,
619+
`declares type: 'record_change' but ` +
620+
(hasTriggerType
621+
? `its start node's triggerType is ${renderTriggerToken(config.triggerType)}, which the engine ` +
622+
`routes to NO trigger`
623+
: `its start node has no triggerType at all, so there is nothing for the engine to route`) +
624+
` — it binds a record-change flow only for a token starting with 'record-', so this flow is demoted ` +
625+
`to a manual one and never fires. Nothing NAMES it: the unbound-flow audit resolves the same binding ` +
626+
`and skips the flow as "manual — nothing to bind", so neither the boot warning nor the startup ` +
627+
`summary lists it; the only trace is the banner's flow count being one higher than its bound count.`,
610628
hint:
611629
`Use record-{before,after}-{create,update,delete,write} ('write' is create OR update in one flow, ` +
612630
`#3427; create/insert are synonyms). If the flow really is launched by hand or from a screen, ` +

0 commit comments

Comments
 (0)