Skip to content

Commit be59695

Browse files
hotlongclaude
andauthored
fix(lint): flow-runas-unscoped 用一句两读皆真的措辞命名有效身份 (#5693) (#6060)
规则原先按 `typeof flow.runAs === 'string'` 分支消息:作者写了就打 `runAs:'user'`,没写就打 `the default runAs:'user'`。这个区分是真的、也有 用,但规则观察不到它 —— 作者拿到哪一支取决于**面**,而不是取决于他们的文件: - CLI 恒为 explicit 支。`FlowSchema.runAs` 带 `.default('user')`,registry 把 该规则接为 `input: 'parsed'`;`os lint` 虽不 Zod-parse,但 `defineStack` / `defineFlow` 在**定义时**就 parse,所以连它拿到的也是默认值已物化的 stack。 - runtime publish gate(#4463)判的是逐字原文 body,两支都可达。 于是同一个 flow 被两个 shipped 面告知两件不同的事,而在作者最先遇到的那个面上, 拿到的恰是读起来像指控的那支:对一个没写过 runAs 的人说「runs as `runAs:'user'`」。 改法:删掉分支,换成一句对两种作者输入都为真的措辞 —— `runs under \`runAs:'user'\` (the default when none is declared)`。括号是对**取值**的陈述, 不是对作者的指控,所以对确实写了 `runAs:'user'` 的人同样为真。这是本仓已有的 写法而非新发明:`flow-draft-status-ambiguous` 在同一机制下说 `has status 'draft' (the default when none is authored)`。 只有措辞变了:命中的 flow、`severity: 'error'`、`where`、`hint`、嵌套证据节点的 region 从句都不变。 测试同批把可达性钉住:parsed 形状(`FlowSchema.parse`)证明两种作者输入到规则 手上已是同一个对象;unparsed 形状(runtime gate 的输入)是唯一还能区分二者的 输入,因而是防止分支复活的那道闸。 Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent a34185b commit be59695

4 files changed

Lines changed: 239 additions & 19 deletions

File tree

.changeset/flow-runas-unscoped-region-descent.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,20 @@ is a flow property and the region only supplies the evidence. The region is name
3636
in the **message** so you can find the node:
3737

3838
```
39-
flow 'nightly_sweep' · runAs: schedule-triggered flow runs as `runAs:'user'`, but a
40-
schedule run has no trigger user — so its data node 'touch' (update_record), in loop
41-
'loop_rows' body, has no identity to scope to and will be REFUSED at run time.
39+
flow 'nightly_sweep' · runAs: schedule-triggered flow runs under `runAs:'user'`
40+
(the default when none is declared), but a schedule run has no trigger user — so its
41+
data node 'touch' (update_record), in loop 'loop_rows' body, has no identity to scope
42+
to and will be REFUSED at run time.
4243
```
4344

45+
(The sentence's opening was re-worded by #5693 in this same release window; the
46+
sample above is the wording that actually ships.)
47+
4448
**Nothing about the top-level case moved.** A flow whose evidence is a top-level
45-
data node produces the byte-identical message it always did (no region clause),
46-
and when a flow has data nodes at both altitudes the top-level one is still the
47-
node cited — `collectFlowGraphs` yields the flow's own graph before it descends.
48-
Both are pinned by tests.
49+
data node produces the same message with no region clause, and when a flow has
50+
data nodes at both altitudes the top-level one is still the node cited —
51+
`collectFlowGraphs` yields the flow's own graph before it descends. Both are
52+
pinned by tests.
4953

5054
**If this newly fails your build:** the flow was already broken at run time. Add
5155
`runAs: 'system'` to declare the elevation the sweep needs (a schedule /
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): `flow-runas-unscoped` stops telling an author they declared a `runAs` they never wrote (#5693)
6+
7+
The rule's message branched on whether `runAs` was **authored** or **defaulted**:
8+
9+
```ts
10+
typeof flow.runAs === 'string' ? `runAs:'user'` : `the default runAs:'user'`
11+
```
12+
13+
That distinction is real and useful — "you wrote something incoherent" is not
14+
"you inherited a default that does not fit a user-less trigger" — but the rule
15+
cannot observe it, and which arm an author got depended on the **surface** rather
16+
than on their file.
17+
18+
**On the CLI, only the explicit arm was reachable.** `FlowSchema.runAs` carries
19+
`.default('user')` and the registry wires this rule `input: 'parsed'`, so
20+
`flow.runAs` is the string `'user'` whether the author wrote it or not. `os lint`
21+
does not Zod-parse, and would have escaped that — except `defineStack` /
22+
`defineFlow` parse at *definition* time, so the config module hands even the
23+
non-parsing command a stack with the default already filled in.
24+
25+
Measured on `examples/app-todo`, `overdue_escalation` with its `runAs` line
26+
deleted — the author declared nothing:
27+
28+
```
29+
BEFORE — os validate
30+
flow 'overdue_escalation' · runAs: schedule-triggered flow runs as `runAs:'user'`, but a
31+
schedule run has no trigger user — so its data node 'get_overdue_tasks' (get_record) …
32+
33+
BEFORE — os lint
34+
✗ flow 'overdue_escalation' · runAs: schedule-triggered flow runs as `runAs:'user'`, but a
35+
schedule run has no trigger user — so its data node 'get_overdue_tasks' (get_record) …
36+
```
37+
38+
Both commands told someone who had written no `runAs` that their flow "runs as
39+
`runAs:'user'`" — which invites *"I never wrote that, the tool is confused"* at
40+
exactly the moment the tool is right and the fix is one line away.
41+
42+
Meanwhile the **runtime publish gate** (#4463) judges the verbatim authored body,
43+
so it really did reach the other arm — the same flow was told two different
44+
things by two shipped surfaces.
45+
46+
**What changed.** One sentence, true of both authoring inputs, on every surface:
47+
48+
```
49+
AFTER — os validate and os lint, identical
50+
flow 'overdue_escalation' · runAs: schedule-triggered flow runs under `runAs:'user'`
51+
(the default when none is declared), but a schedule run has no trigger user — so its
52+
data node 'get_overdue_tasks' (get_record) has no identity to scope to and will be
53+
REFUSED at run time.
54+
```
55+
56+
The parenthetical is a statement about the **value**, not an accusation about the
57+
author, so it stays true for someone who did write `runAs:'user'`. This is the
58+
house pattern rather than a new one: `flow-draft-status-ambiguous` says `has
59+
status 'draft' (the default when none is authored)` for the same reason, on the
60+
same mechanism.
61+
62+
Only the wording moved: the same flows are flagged, with the same
63+
`severity: 'error'`, the same `where`, the same `hint`, and the same region
64+
clause when the evidence node is nested.

packages/lint/src/lint-flow-patterns.test.ts

Lines changed: 109 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4-
import { TimeRelativeTriggerSchema, LoopConfigSchema } from '@objectstack/spec/automation';
4+
import { TimeRelativeTriggerSchema, LoopConfigSchema, FlowSchema } from '@objectstack/spec/automation';
5+
import { AUTHORING_RULES } from './authoring-rules.js';
56
import {
67
lintFlowPatterns,
78
FLOW_TIME_RELATIVE_ANTIPATTERN,
@@ -339,7 +340,8 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR-
339340
expect(fnds).toHaveLength(1);
340341
expect(fnds[0].rule).toBe(FLOW_RUNAS_UNSCOPED);
341342
expect(fnds[0].where).toContain('nightly_sweep');
342-
expect(fnds[0].message).toMatch(/default .*runAs:'user'/);
343+
// #5693 — one wording for both authoring inputs; see the dedicated block below.
344+
expect(fnds[0].message).toMatch(/runAs:'user'` \(the default when none is declared\)/);
343345
expect(fnds[0].message).toMatch(/REFUSED/);
344346
expect(fnds[0].hint).toMatch(/runAs:'system'/);
345347
});
@@ -552,8 +554,11 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR-
552554
});
553555

554556
// The A/B twin. Same flow, same node, moved out of the body — this was
555-
// already flagged before #5633, and its message must not have moved a byte.
556-
it('leaves the TOP-LEVEL twin byte-identical (no region clause, no regression)', () => {
557+
// already flagged before #5633, and its message must carry no region clause.
558+
// (The sentence itself was re-worded once since, by #5693 — see the block at
559+
// the end of this file for why the authored-vs-defaulted branch it used to
560+
// carry could not survive.)
561+
it('leaves the TOP-LEVEL twin without a region clause (no regression)', () => {
557562
const fnds = lintFlowPatterns({
558563
flows: [{
559564
name: 'nightly_sweep',
@@ -568,8 +573,8 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR-
568573
expect(fnds).toHaveLength(1);
569574
expect(fnds[0].where).toBe("flow 'nightly_sweep' · runAs");
570575
expect(fnds[0].message).toBe(
571-
"schedule-triggered flow runs as the default `runAs:'user'`, but a schedule run has no trigger " +
572-
"user — so its data node 'touch' (update_record) has no identity to scope to and " +
576+
"schedule-triggered flow runs under `runAs:'user'` (the default when none is declared), but a " +
577+
"schedule run has no trigger user — so its data node 'touch' (update_record) has no identity to scope to and " +
573578
"will be REFUSED at run time.",
574579
);
575580
expect(fnds[0].message).not.toContain('in loop');
@@ -673,6 +678,104 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR-
673678
});
674679
});
675680
});
681+
682+
/**
683+
* #5693 — ONE wording, true whether the author declared `runAs:'user'` or
684+
* declared nothing.
685+
*
686+
* The message used to branch: `` `runAs:'user'` `` when `flow.runAs` was a
687+
* string, `the default …` when it was absent. The distinction is real and
688+
* useful — "you wrote something incoherent" is not "you inherited a default
689+
* that does not fit a user-less trigger" — but *this rule cannot observe it*,
690+
* and which arm an author got depended on the SURFACE rather than on their
691+
* file:
692+
*
693+
* - **CLI** — always the explicit arm. `FlowSchema.runAs` carries
694+
* `.default('user')` and the registry wires this rule `input: 'parsed'`, so
695+
* `flow.runAs` is the string `'user'` either way. `os lint` does not parse,
696+
* but `defineStack`/`defineFlow` parse at *definition* time, so even it
697+
* receives the default already materialized. Measured on `app-todo` with the
698+
* `runAs` line deleted: `os validate` AND `os lint` both told an author who
699+
* had declared nothing that their flow "runs as `runAs:'user'`".
700+
* - **Runtime publish gate (#4463)** — both arms, because it judges the
701+
* verbatim authored body (`saveMetaItem` keeps `request.item` past the
702+
* schema check).
703+
*
704+
* So the branch was not merely dead: it made one flow get two different
705+
* sentences from two shipped surfaces, and on the surface authors meet first it
706+
* produced the one that reads as an accusation. #5693 removed it in favour of a
707+
* sentence that is true of both inputs on every surface.
708+
*
709+
* These two cases split the work deliberately, and only the second has teeth
710+
* against a re-introduction — say so rather than let the pair read as one
711+
* assertion made twice:
712+
*
713+
* - the PARSED case pins *why* the branch was pointless (both inputs arrive as
714+
* the same object). A re-introduced branch would still pass it — that is the
715+
* point: the CLI cannot tell these apart, which is the whole defect.
716+
* - the UNPARSED case is the regression guard. It is the one input shape where
717+
* the two authoring choices are still distinguishable (and the shape the
718+
* runtime gate really passes), so any future `typeof flow.runAs === 'string'`
719+
* branch in the message fails it immediately.
720+
*/
721+
describe("#5693 — one wording for authored `runAs:'user'` and for none at all", () => {
722+
/**
723+
* Authorable on purpose: `FlowSchema` requires `label` on the flow and on
724+
* every node, so the raw literals the rest of this file feeds would fail the
725+
* parse — and a fixture that cannot be parsed cannot demonstrate anything
726+
* about the parsed tier. This one is the same sweep, declared in full.
727+
*/
728+
const sweep = (runAs?: 'user') => ({
729+
name: 'nightly_sweep',
730+
label: 'Nightly Sweep',
731+
type: 'schedule',
732+
...(runAs ? { runAs } : {}),
733+
nodes: [
734+
{ id: 'start', type: 'start', label: 'Start', config: { triggerType: 'schedule', cron: '0 8 * * *' } },
735+
{ id: 'op', type: 'update_record', label: 'Touch', config: { objectName: 'thing', fields: { a: 1 } } },
736+
],
737+
edges: [{ id: 'e1', source: 'start', target: 'op' }],
738+
});
739+
740+
const EXPECTED =
741+
"schedule-triggered flow runs under `runAs:'user'` (the default when none is declared), but a " +
742+
"schedule run has no trigger user — so its data node 'op' (update_record) has no identity to scope to and " +
743+
"will be REFUSED at run time.";
744+
745+
const messagesFor = (flow: unknown) =>
746+
lintFlowPatterns({ flows: [flow] })
747+
.filter((f) => f.rule === FLOW_RUNAS_UNSCOPED)
748+
.map((f) => f.message);
749+
750+
it('is wired to the tier where the default has already been filled in', () => {
751+
const wiring = AUTHORING_RULES.find((r) => r.name === 'lintFlowPatterns');
752+
expect(wiring?.input).toBe('parsed');
753+
});
754+
755+
it('PARSED input: the two authoring choices are literally the same object here', () => {
756+
const authored = FlowSchema.parse(sweep('user'));
757+
const defaulted = FlowSchema.parse(sweep());
758+
// The premise, asserted rather than assumed: the parse materializes the
759+
// default, so `flow.runAs` carries no trace of what was authored.
760+
expect(defaulted.runAs).toBe('user');
761+
expect(authored.runAs).toBe('user');
762+
763+
expect(messagesFor(defaulted)).toEqual([EXPECTED]);
764+
expect(messagesFor(authored)).toEqual([EXPECTED]);
765+
});
766+
767+
it('UNPARSED input: an absent key and an explicit one still get the same sentence', () => {
768+
// The runtime publish gate's shape — the only surface where the omission
769+
// survives to the rule. Both must read the same, or the surfaces disagree
770+
// about one flow again.
771+
expect(messagesFor(sweep())).toEqual([EXPECTED]);
772+
expect(messagesFor(sweep('user'))).toEqual([EXPECTED]);
773+
});
774+
775+
it('says the same thing to both surfaces about the same flow', () => {
776+
expect(messagesFor(FlowSchema.parse(sweep()))).toEqual(messagesFor(sweep()));
777+
});
778+
});
676779
});
677780

678781
/**

packages/lint/src/lint-flow-patterns.ts

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,18 @@
122122
* flow-level, and the region is named in the **message** rather than in `where`
123123
* (`its data node 'touch' (update_record), in loop 'loop_rows' body,`): `where`
124124
* says which declaration is wrong, the message says where to find the proof. A
125-
* top-level hit yields the byte-identical message it always did — pinned in the
126-
* tests, since the wording is what every existing author already sees.
125+
* top-level hit adds no region clause at all — pinned in the tests, since a
126+
* top-level finding is what most authors see.
127127
*
128128
* What this fixes is not a corner. Query a set, loop it, write per item is *the*
129129
* shape of a scheduled data flow, so the write is almost always the nested node —
130130
* and because this rule gates the build, the shape it was missing built clean and
131131
* then could not run at all. That is precisely what promoting it to `error`
132132
* (#3760) was for.
133+
*
134+
* #5693 rewrote how that same message names the identity — one wording true of
135+
* both authoring inputs instead of a branch on `flow.runAs` that no CLI command
136+
* could take. See {@link RUNAS_EFFECTIVE_IDENTITY}.
133137
*/
134138

135139
import {
@@ -249,6 +253,49 @@ const INERT_CONDITION_NODE_TYPES = new Set([
249253
/** Node types that perform a data operation — the ones `flow.runAs` governs (#1888). */
250254
const DATA_NODE_TYPES = new Set(['get_record', 'create_record', 'update_record', 'delete_record']);
251255

256+
/**
257+
* How {@link FLOW_RUNAS_UNSCOPED} names the identity the run would use — ONE
258+
* wording, true whether the author wrote `runAs:'user'` or wrote nothing (#5693).
259+
*
260+
* It replaces a ternary that told the author which of the two they had done:
261+
* `` `runAs:'user'` `` when `flow.runAs` was a string, `the default …` when it
262+
* was absent. That distinction is real, but the rule cannot observe it, and the
263+
* arm it picked depended on the SURFACE rather than on the metadata:
264+
*
265+
* - **CLI (`os validate` / `os build` / `os lint`) — always the explicit arm.**
266+
* `FlowSchema.runAs` carries `.default('user')`, and the registry wires this
267+
* rule `input: 'parsed'`, so `flow.runAs` is the string `'user'` either way.
268+
* `os lint` does not parse and would have escaped that, except that
269+
* `defineStack` (and `defineFlow`) parse at *definition* time, so the config
270+
* module hands even the non-parsing command a stack with the default already
271+
* materialized. Measured on `examples/app-todo` with the `runAs` line deleted:
272+
* both commands printed the EXPLICIT arm at an author who had declared nothing.
273+
* - **Runtime publish gate (#4463) — both arms.** It judges `request.item`, the
274+
* verbatim authored body (`saveMetaItem` keeps it verbatim past the schema
275+
* check), so an omitted key really is absent there.
276+
*
277+
* So the same flow was told two different things by two shipped surfaces, and on
278+
* the surface an author uses first it was told the one that reads as an
279+
* accusation: *"you declared `runAs:'user'`"* to someone who declared nothing —
280+
* inviting "the tool is confused" at the exact moment the tool is right and the
281+
* fix is one line. Restoring the distinction would mean giving a `parsed`-tier
282+
* rule a second, pre-parse input; #5693 chose the wording instead.
283+
*
284+
* The parenthetical is a statement about the VALUE, not an accusation about the
285+
* author, so it stays true for someone who did write `runAs:'user'`. That is the
286+
* house pattern, not a new one: `flow-draft-status-ambiguous` says `has status
287+
* 'draft' (the default when none is authored)` for exactly this reason, on
288+
* exactly this mechanism (`validate-flow-trigger-readiness.ts`).
289+
*
290+
* `'user'` is spelled out rather than interpolated from `flow.runAs` because the
291+
* branch that uses this has already excluded `'system'` and the enum holds only
292+
* those two — so on every surface that reaches the message (CLI: parsed; runtime
293+
* gate: `safeParse`d against the overlay schema before the gate runs) the
294+
* effective identity IS `'user'`. Interpolating would re-introduce a limb only
295+
* an off-spec literal could reach, which is the defect this replaced.
296+
*/
297+
const RUNAS_EFFECTIVE_IDENTITY = "`runAs:'user'` (the default when none is declared)";
298+
252299
/**
253300
* The first data node ANYWHERE in a flow, with the region it was found in —
254301
* {@link FLOW_RUNAS_UNSCOPED}'s evidence that the flow performs a data operation
@@ -1034,22 +1081,24 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] {
10341081
// build-GATING rule is its own change with its own blast radius — this
10351082
// is that change. The shape it was missing is the DEFAULT one for a
10361083
// scheduled data flow: query a set, loop it, write per item.
1084+
// #5693 — the message states the EFFECTIVE identity in one wording that
1085+
// is true of both authoring inputs, and does not branch on whether the
1086+
// author wrote `runAs` (see {@link RUNAS_EFFECTIVE_IDENTITY}).
10371087
const runAs = typeof flow.runAs === 'string' ? flow.runAs : 'user';
10381088
const userLessKind = userLessTriggerKind(flow, startCfg);
10391089
if (userLessKind && runAs !== 'system') {
10401090
const dataNode = findDataNodeAnywhere(nodes, edges);
10411091
if (dataNode) {
1042-
const declared = typeof flow.runAs === 'string' ? `\`runAs:'${runAs}'\`` : `the default \`runAs:'user'\``;
10431092
// The region is named in the MESSAGE, not in `where`: `where` says which
10441093
// declaration is wrong (`flow 'x' · runAs`, unchanged), the message says
10451094
// where to look for the node that proves it. A top-level hit adds nothing
1046-
// here, so its wording is byte-identical to before (pinned in the tests).
1095+
// here, so its wording carries no region clause (pinned in the tests).
10471096
const at = dataNode.scope ? `, in ${dataNode.scope},` : '';
10481097
findings.push({
10491098
where: `flow '${flowName}' · runAs`,
10501099
message:
1051-
`${userLessKind}-triggered flow runs as ${declared}, but a ${userLessKind} run has no trigger ` +
1052-
`user — so its data node '${dataNode.node.id}' (${dataNode.node.type})${at} has no identity to scope to and ` +
1100+
`${userLessKind}-triggered flow runs under ${RUNAS_EFFECTIVE_IDENTITY}, but a ${userLessKind} run ` +
1101+
`has no trigger user — so its data node '${dataNode.node.id}' (${dataNode.node.type})${at} has no identity to scope to and ` +
10531102
`will be REFUSED at run time.`,
10541103
hint:
10551104
`Declare \`runAs:'system'\` to make the elevation explicit and intended (the run reads/writes ` +

0 commit comments

Comments
 (0)