Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 56 additions & 0 deletions .changeset/lint-never-fire-family-gates.md
Original file line number Diff line number Diff line change
@@ -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` 升级前后输出逐字一致、均通过,
本次升级不需要修改任何示例。
28 changes: 21 additions & 7 deletions packages/lint/src/authoring-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
168 changes: 164 additions & 4 deletions packages/lint/src/validate-flow-trigger-readiness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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<string, unknown>]> = [
[
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({
Expand Down
Loading
Loading