From cc539783e38c0e1ac36ea51bc43fd371aae40cb8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 12:51:08 +0000 Subject: [PATCH] =?UTF-8?q?feat(lint):=20never-fire=20=E6=97=8F=E4=B8=89?= =?UTF-8?q?=E6=9D=A1=E8=A7=84=E5=88=99=20warning=20=E2=86=92=20error,?= =?UTF-8?q?=E6=B3=A8=E5=86=8C=E8=A1=A8=20tier=20=E5=90=8C=E6=AD=A5=20gatin?= =?UTF-8?q?g=20(#5762)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `validateFlowTriggerReadiness` 的规则此前一律 warning。族内复审后按**一个**标准 重新定级 —— 仅凭这份 stack 是否已足够断定 flow 不会运行 —— 三条答「是」升 error: - `flow-time-relative-descriptor-invalid`(#5496):判它的正是 trigger 在 bind 时 `safeParse` 的同一个 `TimeRelativeTriggerSchema`,schema 拒的描述符运行时同样被 拒,sweep 永不安装。 - `flow-time-relative-descriptor-unroutable`(#5647):判据就是引擎自己的路由谓词 `typeof … === 'object'`,任何部署都不会把它路由到 time-relative trigger —— 全族最硬的判定,也是唯一连 bind 时那行 warn 都没有的一条。 - `flow-trigger-unknown-event`(#3427/#3457/#3481):引擎用**硬编码前缀**把任何 `record-` token 路由给 record-change trigger(不查注册表,故装任何包都无法声明 新 `record-*` token),该 trigger 再用 `triggerTypeToHookEvents` 的**封闭文法** 映射 —— 文法外即零 hook event。与 unknown-object 不同,没有跨包免责的读法。 两条对照维持 warning,这正是本次不是「整个文件都升 error」的原因: `flow-trigger-unknown-object` 的跨包免责是真的(对象可能由另一个已安装包提供, 规则看不见);`flow-draft-status-ambiguous` 的 draft flow **确实会**触发,是意图 歧义而非死 flow。两者作为反例钉在新增的 severity map 测试里。 注册表 tier `advisory` → `gating`。tier 声明并非自述:`authoring-rule-wiring.test.ts` 会读规则源码核对,故留 advisory 必红 —— 已反向验证。 P1 运行时发布门边界经实测,与直觉相反且对租户有利:门交给规则的快照里 `flows` 只装 正在写入的那一个 item,且减掉 baseline 的 finding,`RuntimeStackContext` 也只有 `objects` 一个通道 —— 已存储的 flow 进不了快照。实测:发布死 flow 自身 errors=1 (拒收),已存死 flow 时发布好 flow errors=0(不受连带)。被拒的是死 flow 自己的 发布,以及 CLI 侧含死 flow 的包。 仓内 showcase / CRM / todo 三个示例 `os validate` 升级前后输出逐字一致、均通过, 无需修改任何示例。 Claude-Session: https://claude.ai/code/session_01GX3sL71LFq8m2usg6VqTSE Co-authored-by: Claude Fable 5 --- .changeset/lint-never-fire-family-gates.md | 56 ++++++ packages/lint/src/authoring-rules.ts | 28 ++- .../validate-flow-trigger-readiness.test.ts | 168 +++++++++++++++++- .../src/validate-flow-trigger-readiness.ts | 73 +++++++- 4 files changed, 310 insertions(+), 15 deletions(-) create mode 100644 .changeset/lint-never-fire-family-gates.md diff --git a/.changeset/lint-never-fire-family-gates.md b/.changeset/lint-never-fire-family-gates.md new file mode 100644 index 0000000000..b10aac58a5 --- /dev/null +++ b/.changeset/lint-never-fire-family-gates.md @@ -0,0 +1,56 @@ +--- +"@objectstack/lint": minor +--- + +never-fire 族三条 lint 规则 warning → error:声明了触发器却确定不会运行的 flow 现在被拒收 + +`validateFlowTriggerReadiness` 里的规则此前一律是 warning。族内复审(#5762)后按**一个**标准重新定级 —— +「仅凭这份 stack 是否已经足够断定这个 flow 不会运行」—— 三条规则答「是」,升为 `error`: + +- **`flow-time-relative-descriptor-invalid`**(#5496)—— `config.timeRelative` 描述符 + `TimeRelativeTriggerSchema` 判不过。判它的正是 trigger 在 bind 时 `safeParse` 的同一个 schema, + 所以 schema 拒的描述符在运行时同样被拒:sweep 永不安装,flow 声明了 time-relative 触发器却永不运行。 +- **`flow-time-relative-descriptor-unroutable`**(#5647)—— `config.timeRelative` 不是对象 + (`timeRelative: 'daily'` 是典型)。判据就是引擎自己的路由谓词 `typeof … === 'object'`, + 所以这个值不会被任何部署路由到 time-relative trigger —— 全族里最硬的判定,也是唯一连 + bind 时那一行 warn 都没有的一条。 +- **`flow-trigger-unknown-event`**(#3427/#3457/#3481)—— `record-*` token 落在文法之外 + (`record-after-updated` 拼错、`record-change` 缺相位、数组形式)。引擎用硬编码前缀把任何 + `record-` 开头的 token 路由给 record-change trigger(不查注册表,所以装任何包都无法声明新 + `record-*` token),该 trigger 再用 `triggerTypeToHookEvents` 的封闭文法映射 —— 文法外就是零 + hook event,即绑不上任何东西。 + +**两条对照规则维持 warning,这正是本次定级不是「整个文件都升 error」的原因:** + +- `flow-trigger-unknown-object` —— 本 stack 没定义的对象名**可能由另一个已安装包提供**, + 规则看不见那个包的对象。免责是真的,所以继续 advisory(它自己的 hint 就这么写)。 +- `flow-draft-status-ambiguous` —— draft flow **确实会**触发,这是意图歧义而非死 flow。 + +## 影响面:P1 运行时发布门(`surfaces: CLI_AND_RUNTIME`) + +注册表 tier 同步 `advisory` → `gating`。除 `os validate` / `os build` / `os lint` 之外,本规则也跑在 +元数据写入路径的 publish 门上,`error` 会让 `state: 'active'` 的写入被拒(422)。**边界值得写清,因为 +直觉猜错的方向恰好对租户有利**:该门交给规则的快照里 `flows` 只装**正在写入的那一个** item +(`runtime-gate.ts`:`candidate = { objects, [stackKey]: [item] }`),并且会减掉 baseline 已产出的 +finding。所以: + +- 发布 flow A **不会**因为已存储的 flow B 是死 flow 而被拒;租户既有的死 flow 继续被读取和服务 + (ADR-0087 读路径不对称); +- 被拒的是**死 flow 自己的发布**(含再次发布),以及 CLI 侧 —— stack 里含死 flow 的包 + `os validate` / `os build` / `os lint` 转红; +- draft 保存从不过门(#4463 D1),只有发布才过。 + +## 迁移 + +修死 flow,不要降级规则: + +- 描述符判不过 → 按 finding 里 `TimeRelativeTriggerSchema` 的原文改(它会点名该写的键); + 描述符要 `{ object, dateField, 且 withinDays | offsetDays 恰好其一 }`。 +- `timeRelative` 写成了 `'daily'` 这类节奏值 → 节奏是**同级兄弟键** `config.schedule` + (默认每日 08:00 UTC,通常可省),`timeRelative` 只描述扫哪些记录。 +- `record-*` token 落在文法外 → 用 `record-{before,after}-{create,insert,update,delete,write}`; + 「创建或更新」用 `record-after-write` 一条 flow 覆盖(#3427);多事件数组仍未支持(#3457), + 按事件各写一条 flow。 + +仓内三个示例 app(showcase / CRM / todo)`os validate` 升级前后输出逐字一致、均通过, +本次升级不需要修改任何示例。 diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 54ecc98a53..24c89d4926 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -558,18 +558,32 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ + 'severity change on a published rule id and belongs in its own PR, not riding a wiring change.', run: (stack) => validateCapabilityReferences(stack), }, - // A record-change flow whose start-node objectName matches nothing never - // fires — silently. Reads the pre-parse tier so an author sees what they - // wrote. Advisory: the object may come from another installed package. + // A flow that LOOKS armed and never launches — silently. Reads the pre-parse + // tier so an author sees what they wrote. + // + // `gating` since #5762, which reviewed the file's rules as one family and + // split them on a single question: is THIS STACK enough to know the flow is + // dead? Three rules answer yes and now emit `error` — a `config.timeRelative` + // the spec's own `TimeRelativeTriggerSchema` refuses, one the engine's routing + // predicate cannot route at all, and a `record-*` triggerType outside the + // closed token grammar `triggerTypeToHookEvents` maps. None of those verdicts + // can be changed by installing a package, so there is no reading under which + // the flow fires. `flow-trigger-unknown-object` deliberately stayed `warning` + // (the object may come from another installed package — a hedge this rule + // cannot decide), as did `flow-draft-status-ambiguous` (draft flows DO fire; + // that one is ambiguity of intent, not a dead flow). { name: 'validateFlowTriggerReadiness', - tier: 'advisory', + tier: 'gating', input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-flow-trigger-readiness.ts', - // Runtime publish gate (#4463): the FLOW family. Advisory at this surface - // too — its findings are logged, not thrown (P1 gates on `error` only; P2 - // puts advisories on the response for Studio to render). + // Runtime publish gate (#4463): the FLOW family. Its `error` findings now + // REFUSE a `state: 'active'` write (P1 gates on `error` only); the rules that + // stayed `warning` keep being logged as advisories. The gate judges a + // snapshot whose `flows` holds only the written item and subtracts the + // baseline's findings, so this refuses the dead flow's own publish — never + // another flow's save on account of a stored one. surfaces: CLI_AND_RUNTIME, runtimeTypes: ['flow'], run: (stack) => validateFlowTriggerReadiness(stack), diff --git a/packages/lint/src/validate-flow-trigger-readiness.test.ts b/packages/lint/src/validate-flow-trigger-readiness.test.ts index 2732bcf071..236ba9bafd 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.test.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.test.ts @@ -223,7 +223,7 @@ describe('validateFlowTriggerReadiness', () => { expect(findings).toHaveLength(1); const [f] = findings; expect(f.rule).toBe(FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID); - expect(f.severity).toBe('warning'); + expect(f.severity).toBe('error'); // Criterion 1: the finding NAMES config.timeRelative, in both channels the // CLI prints (`• where: message` then `at path`). expect(f.path).toBe('flows[0].nodes[0].config.timeRelative'); @@ -410,7 +410,7 @@ describe('validateFlowTriggerReadiness', () => { expect(findings).toHaveLength(1); const [f] = findings; expect(f.rule).toBe(FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE); - expect(f.severity).toBe('warning'); + expect(f.severity).toBe('error'); expect(f.path).toBe('flows[0].nodes[0].config.timeRelative'); expect(f.where).toBe('flow "task_due_reminder" › start node'); // The value the author wrote AND why it is not a descriptor. @@ -622,7 +622,7 @@ describe('validateFlowTriggerReadiness', () => { const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); expect(findings).toHaveLength(1); expect(findings[0].rule).toBe(FLOW_TRIGGER_UNKNOWN_EVENT); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].message).toContain("'record-after-updated'"); expect(findings[0].message).toMatch(/never fires/i); expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType'); @@ -691,7 +691,7 @@ describe('validateFlowTriggerReadiness', () => { ]; const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); expect(findings.map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].message).toMatch(/array/i); expect(findings[0].message).toMatch(/never fires/i); expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType'); @@ -720,6 +720,166 @@ describe('validateFlowTriggerReadiness', () => { expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false); }); + // ── #5762 — the family's severity map ──────────────────────────────────── + // + // The rules in this file were reviewed as ONE family and split on a single + // question: is this stack enough to know the flow is dead? Three rules answer + // yes and gate; two hedge and advise. The split is the contract, so it is + // pinned as a map rather than as five scattered `severity` lines — a later + // rule added to this file has to decide which side it is on, and a later edit + // that quietly demotes one of the three has to come past this test. + // + // Every entry is provoked through a real stack, so an id whose criterion stops + // firing fails here instead of passing vacuously (the empty-verdict trap: an + // assertion about findings that are never produced is green for the wrong + // reason). + describe('severity (#5762)', () => { + /** Minimal stacks, one per rule id, each provoking exactly that finding. */ + const provoke: Array<[string, 'error' | 'warning', Record]> = [ + [ + FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID, + 'error', + { + objects: [{ name: 'task', label: 'Task', fields: {} }], + flows: [ + { + name: 'bad_shape', + type: 'schedule', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + config: { timeRelative: { object: 'task', field: 'due_at', offsetDays: -1 } }, + }, + ], + }, + ], + }, + ], + [ + FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE, + 'error', + { + objects: [{ name: 'task', label: 'Task', fields: {} }], + flows: [ + { + name: 'scalar_descriptor', + type: 'autolaunched', + status: 'active', + nodes: [{ id: 'start', type: 'start', config: { timeRelative: 'daily' } }], + }, + ], + }, + ], + [ + FLOW_TRIGGER_UNKNOWN_EVENT, + 'error', + { + objects: [candidateObject], + flows: [ + { + name: 'typo_token', + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + config: { objectName: 'app_candidate', triggerType: 'record-after-updated' }, + }, + ], + }, + ], + }, + ], + // ── The controls. Both are hedged, and the hedge is the whole reason the + // promotion above is not "everything in this file is an error now". + [ + FLOW_TRIGGER_UNKNOWN_OBJECT, + 'warning', + { + objects: [candidateObject], + flows: [ + { + name: 'other_package_object', + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + config: { objectName: 'billing_invoice', triggerType: 'record-after-update' }, + }, + ], + }, + ], + }, + ], + [ + FLOW_DRAFT_STATUS_AMBIGUOUS, + 'warning', + { + objects: [candidateObject], + flows: [recordFlow()], + }, + ], + ]; + + for (const [rule, severity, stack] of provoke) { + it(`${rule} is ${severity}`, () => { + const matching = validateFlowTriggerReadiness(stack).filter((f) => f.rule === rule); + // Non-vacuous first: the fixture really does provoke this id. + expect(matching.length, `${rule} was not provoked by its own fixture`).toBeGreaterThan(0); + for (const f of matching) expect(f.severity).toBe(severity); + }); + } + + it('the hedged rules say so in their own hint, and the gating ones do not', () => { + // The severity claim and the prose have to agree. A rule that gates while + // telling the author the finding "can be ignored" is the contradiction that + // makes a gate read as a bug — and the cross-package sentence is exactly + // what earns `flow-trigger-unknown-object` its warning. + for (const [rule, severity, stack] of provoke) { + for (const f of validateFlowTriggerReadiness(stack).filter((x) => x.rule === rule)) { + if (severity === 'warning' && rule === FLOW_TRIGGER_UNKNOWN_OBJECT) { + expect(f.hint, rule).toMatch(/another installed package/); + } + if (severity === 'error') { + expect(f.hint, rule).not.toMatch(/can be ignored/i); + expect(f.hint, rule).not.toMatch(/another installed package/); + } + } + } + }); + + it('a clean stack produces no finding of any severity', () => { + // The floor the promotion must not move: promoting a rule must not make a + // correct flow fail. Without this, "everything is an error" would pass + // every assertion above. + expect( + validateFlowTriggerReadiness({ + objects: [candidateObject, { name: 'task', label: 'Task', fields: {} }], + flows: [ + recordFlow({ status: 'active' }), + { + name: 'renewal', + type: 'schedule', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + config: { timeRelative: { object: 'task', dateField: 'due_at', withinDays: 30 } }, + }, + ], + }, + ], + }), + ).toEqual([]); + }); + }); + it('handles map-keyed flows/objects and stacks with no flows', () => { expect(validateFlowTriggerReadiness({})).toEqual([]); const findings = validateFlowTriggerReadiness({ diff --git a/packages/lint/src/validate-flow-trigger-readiness.ts b/packages/lint/src/validate-flow-trigger-readiness.ts index 16dc8bfeda..4334ba3a76 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.ts @@ -46,6 +46,44 @@ // second copy of the descriptor's shape living in this file. It stays inside the // package's stated dependency direction — lint → `@objectstack/spec`, never onto // a runtime. +// +// ## Severity: the never-fire family gates, the hedged rules advise (#5762) +// +// The file's rules do not share a severity, and the line between them is not +// how bad the outcome is — every rule here describes a flow that does not run. +// It is whether THIS STACK is enough to know that: +// +// - `error` — the never-fire family. `flow-time-relative-descriptor-invalid`, +// `flow-time-relative-descriptor-unroutable` and +// `flow-trigger-unknown-event` each read a value whose verdict is settled by +// a contract that ships in this repo: `TimeRelativeTriggerSchema` for the +// first, the engine's own `typeof … === 'object'` routing predicate for the +// second, `triggerTypeToHookEvents`' closed token grammar for the third. +// Nothing an author or a tenant can INSTALL changes any of those verdicts, +// so there is no reading of the stack under which the flow fires. A rule +// that can prove a declared trigger is dead should not be asking the author +// to notice a warning about it. +// - `warning` — `flow-trigger-unknown-object`, both halves. An object name +// this stack does not define may be defined by another installed package, +// and this rule cannot see that package's objects. The hedge is real, so the +// rule advises and says so in its own hint. It is the deliberate control for +// the paragraph above: the family was reviewed together (#5762) and this is +// the one that stayed advisory. +// - `warning` — `flow-draft-status-ambiguous`. Draft flows DO fire, so this is +// an ambiguity of intent, not a dead flow. Gating it would refuse a stack +// whose flows all work. +// +// The consequence of `error` is not confined to `os validate`: this rule runs at +// the runtime publish gate too (`surfaces: CLI_AND_RUNTIME` in +// `authoring-rules.ts`), which refuses a `state: 'active'` metadata write whose +// findings include one. What that does and does NOT reach is worth stating, +// because the obvious guess is wrong in the tenant's favour: the gate hands this +// rule a snapshot whose `flows` array holds ONLY the item being written +// (`runtime-gate.ts` — `candidate = { objects, [stackKey]: [item] }`), and +// subtracts every finding the baseline already produced. So publishing flow A is +// never refused because stored flow B is dead, and a tenant's existing dead +// flows keep being served. What IS refused is the dead flow's own publish — and, +// on the CLI surface, a package build whose stack contains one. import { TimeRelativeTriggerSchema } from '@objectstack/spec/automation'; @@ -273,7 +311,12 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines .map((i) => `${i.path.join('.') || '(root)'}: ${i.message.replace(/\s+/g, ' ').trim()}`) .join('; '); findings.push({ - severity: 'warning', + // `error` (#5762): the verdict is `TimeRelativeTriggerSchema`'s, and it + // is the same schema the trigger safeParses at bind time. A descriptor + // it refuses is refused at bind too — the sweep is never installed, on + // every deployment, with no installed package able to change the + // answer. Nothing is left for the author to weigh. + severity: 'error', rule: FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID, where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.timeRelative`, @@ -297,7 +340,15 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines // (only a runtime warn). Surface the never-fire defect at authoring time. if (start && isRecordTriggered && !VALID_RECORD_TRIGGER.test((triggerType ?? '').trim())) { findings.push({ - severity: 'warning', + // `error` (#5762). The token grammar is CLOSED and local: the engine + // routes any `record-`-prefixed string to the record-change trigger by a + // hardcoded prefix test (no registry lookup, so installing a package + // cannot claim a new `record-*` token), and that trigger maps the token + // with `triggerTypeToHookEvents` — the same regex this file's + // `VALID_RECORD_TRIGGER` mirrors. Off-grammar means zero hook events, + // which means bound-to-nothing on every deployment. Unlike an object + // name, there is no other-package reading that rescues it. + severity: 'error', rule: FLOW_TRIGGER_UNKNOWN_EVENT, where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, @@ -317,7 +368,14 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines // single tokens above (same rule id — both are "this token never fires"). if (start && isArrayRecordTriggered) { findings.push({ - severity: 'warning', + // `error` (#5762), same id and same reason as 1c: an array maps to no + // hook event either. The engine routes it to the record-change trigger + // for the express purpose of making it loud, and its own comment names + // THIS rule as the primary catch — a primary catch that only warns is + // the "declared ≠ enforced" shape the registry's tier exists to close. + // Multi-event arrays are deferred, not unsupported-by-accident (#3457), + // so if they land the grammar widens here in the same commit. + severity: 'error', rule: FLOW_TRIGGER_UNKNOWN_EVENT, where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, @@ -384,7 +442,14 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines `never fires — with zero diagnostics at any layer, not even the one bind-time warn a ` + `descriptor that IS an object gets when the trigger refuses it.`; findings.push({ - severity: 'warning', + // `error` (#5762). The criterion IS the engine's routing predicate, so a + // value that fails it is not routed to the time-relative trigger by any + // deployment — the strongest verdict in this file, and the one case with + // no runtime channel to fall back on (not even the bind-time warn 1b-ii + // moves earlier). Note the two consequences below are both defects: one + // never fires, the other silently drops the descriptor. Neither is a + // shape the author can have meant, so both gate. + severity: 'error', rule: FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE, where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.timeRelative`,