Skip to content

Commit 23619f5

Browse files
os-trumpclaude
andauthored
fix(cli): make os explain flow teach a flow that actually parses (#14809)
* fix(cli): make `os explain flow` teach a flow that actually parses The catalog entry is hand-maintained and does not derive from FlowSchema, so its sample drifted into teaching a shape the spec rejects outright: - `steps` and `trigger` are strictObject ALIASES on FlowSchema (for `nodes` and `type`). Authoring either is a loud parse error, and a record-change flow binds its object on the START node's `config`, not at the flow top level. - A node's per-type data lives under `config`, so the sample's top-level `field`/`value` pair were undeclared keys on a `.strict()` node schema, and the required `id`/`label` were missing. - `edges` is required; the sample had no graph at all. - The assignment value `'$currentUser'` is a `$`-prefixed sentinel no resolver in this repo recognises. The flow value dialect is brace-based and the acting user is `{$User.Id}`. Also: an `assignment` node sets a flow VARIABLE, not a record field, so "assign on create" is an `update_record` node. The sample now shows the real shape end to end and is pinned by a test that parses it against FlowSchema, which is the only guard that cannot itself drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * chore(changeset): patch @objectstack/cli for the os explain flow sample fix Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * test(cli): keep the new explain pins free of implicit any `packages/cli/test/commands.test.ts` sits outside every tsc program in the repo (the TEST_DEBT ledger records the package), so nothing would have reported an implicit `any` in the pins added for #14782 — and an implicit `any` there silently stops the assertion from checking anything. Measured with an ad-hoc strict pass over the file: origin/main carries 15 errors (13 TS2835 from its extensionless relative imports, 2 TS7006 in the pre-existing ownership test). The first draft of the pins took that to 18. With a local `CatalogField` shape and a typed `Object.entries` cast it is back to exactly the baseline 15 — no new error, and no widening of what the pins actually assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b91c351 commit 23619f5

3 files changed

Lines changed: 107 additions & 9 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
Fix `os explain flow`, whose example taught a flow shape the spec rejects and an assignment value nothing resolves.
6+
7+
`os explain` is an authoring aid whose whole audience is authors — increasingly AI authors — writing their first flow, and its catalog is hand-maintained rather than derived from `FlowSchema`. The flow entry had drifted until the sample it printed could not be pasted into a working app:
8+
9+
- `steps` and `trigger` are strict-object **aliases** on `FlowSchema` (for `nodes` and `type`), so authoring either is a loud parse error rather than a working flow. A record-change flow binds its object on the START node's `config` (`{ objectName, triggerType }`), not at the flow top level.
10+
- A node's per-type data lives under `config`, so the sample's top-level `field`/`value` pair were undeclared keys on a `.strict()` node schema, and the required `id` / `label` were absent. `edges` is required, and the sample declared no graph at all.
11+
- The value `'$currentUser'` was a `$`-prefixed sentinel no resolver in the platform recognises. Flow values interpolate with **single braces**, and the acting user is `{$User.Id}` — the filter surface's `{current_user_id}` is a different dialect that does not carry over, because assignment and `fields` values go through `interpolate`, not `interpolateFilter`.
12+
- An `assignment` node sets a flow **variable**, not a record field, so "auto-assign on create" is an `update_record` node. The old sample would not have written `assigned_to` even with a resolving token.
13+
14+
The entry's field list now matches `FlowSchema` (`nodes` / `edges` / the full five-value `type` enum / `status` / `runAs`), and the example is pinned by a test that parses it against `FlowSchema` — the one guard that cannot drift alongside the catalog it checks.

packages/cli/src/commands/explain.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,25 +107,43 @@ export const SCHEMAS: Record<string, SchemaInfo> = {
107107

108108
flow: {
109109
name: 'Flow',
110-
description: 'Visual logic orchestration for business processes. Flows can be auto-launched, screen-based, or scheduled.',
110+
description: 'Visual logic orchestration for business processes. A flow is a GRAPH — `nodes` plus the `edges` that connect them — auto-launched, record-change, screen-based, scheduled, or API-invoked.',
111111
required: [
112112
{ name: 'name', type: 'string (snake_case)', description: 'Machine name identifier' },
113-
{ name: 'type', type: '"autolaunched" | "screen" | "schedule"', description: 'Trigger type' },
113+
{ name: 'label', type: 'string', description: 'Display name' },
114+
{ name: 'type', type: '"autolaunched" | "record_change" | "schedule" | "screen" | "api"', description: 'Flow type' },
115+
{ name: 'nodes', type: 'FlowNode[]', description: 'Graph nodes, each { id, type, label, config? }. Per-node data lives under `config` — there are no top-level `field`/`value` keys.' },
116+
{ name: 'edges', type: 'FlowEdge[]', description: 'Graph connections, each { id, source, target, condition?, label? }. Bare CEL in `condition` — never {…} braces.' },
114117
],
115118
optional: [
116-
{ name: 'label', type: 'string', description: 'Display name' },
117119
{ name: 'description', type: 'string', description: 'Documentation for the flow' },
118-
{ name: 'trigger', type: 'TriggerConfig', description: 'Event that starts the flow' },
119-
{ name: 'steps', type: 'FlowStep[]', description: 'Sequence of actions' },
120+
{ name: 'status', type: '"draft" | "active" | "obsolete" | "invalid"', description: 'Deployment status (default "draft") — the engine arms flows from this' },
120121
{ name: 'variables', type: 'Variable[]', description: 'Flow-scoped variables' },
122+
{ name: 'runAs', type: '"system" | "user"', description: 'Execution identity (default "user" — runs as the triggering user, respecting RLS)' },
121123
],
122124
example: `{
123125
name: 'assign_on_create',
124-
type: 'autolaunched',
126+
type: 'record_change',
125127
label: 'Auto-Assign on Create',
126-
trigger: { object: 'project_task', event: 'afterInsert' },
127-
steps: [
128-
{ type: 'assignment', field: 'assigned_to', value: '$currentUser' },
128+
status: 'active',
129+
nodes: [
130+
// A record-change flow binds its object on the START node's config,
131+
// not at the flow top level.
132+
{ id: 'start', type: 'start', label: 'On Task Create',
133+
config: { objectName: 'project_task', triggerType: 'record-after-create' } },
134+
// Values interpolate with SINGLE braces. {$User.Id} is the acting user;
135+
// {record.<field>} reads the triggering record.
136+
{ id: 'assign', type: 'update_record', label: 'Assign to Actor',
137+
config: {
138+
objectName: 'project_task',
139+
filter: { id: '{record.id}' },
140+
fields: { assigned_to: '{$User.Id}' },
141+
} },
142+
{ id: 'done', type: 'end', label: 'Done' },
143+
],
144+
edges: [
145+
{ id: 'e1', source: 'start', target: 'assign' },
146+
{ id: 'e2', source: 'assign', target: 'done' },
129147
],
130148
}`,
131149
related: ['object', 'trigger', 'agent'],

packages/cli/test/commands.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import Generate from '../src/commands/generate';
1212
import Lint from '../src/commands/lint';
1313
import Diff from '../src/commands/diff';
1414
import Explain, { SCHEMAS } from '../src/commands/explain';
15+
import { FlowSchema } from '@objectstack/spec/automation';
1516

1617
describe('CLI Commands (oclif)', () => {
1718
it('should have compile command', () => {
@@ -94,4 +95,69 @@ describe('os explain — schema catalog accuracy', () => {
9495
// …and must never regress back to the contribution-kind values.
9596
expect(ownership!.type).not.toBe('"own" | "extend"');
9697
});
98+
99+
// ── `os explain flow` ───────────────────────────────────────────────────
100+
//
101+
// The flow entry shipped a sample that could not parse, and the catalog is
102+
// hand-maintained (it does NOT derive from FlowSchema), so nothing said so:
103+
// • `steps` and `trigger` are strictObject ALIASES on FlowSchema (for
104+
// `nodes` and `type`) — authoring either is a loud parse error;
105+
// • a node's per-type data lives under `config`, so the sample's top-level
106+
// `field`/`value` pair are undeclared keys on a `.strict()` node, and its
107+
// required `id`/`label` were absent;
108+
// • `edges` is required — a graph with no edges was not expressible;
109+
// • the value `'$currentUser'` was a `$`-prefixed sentinel NO resolver in
110+
// the repo recognises. The flow value dialect is brace-based, and the
111+
// acting user is `{$User.Id}` (template.ts `resolveToken`, whose
112+
// `$User.Id` branch returns `context.userId`). The neighbouring FILTER
113+
// dialect's `{current_user_id}` is a different door and does NOT carry
114+
// over: assignment/`fields` values go through plain `interpolate`, not
115+
// `interpolateFilter`.
116+
//
117+
// Parsing the sample against the real schema is the guard that cannot itself
118+
// drift — it re-derives the truth from the spec on every run, which is what
119+
// the hand-maintained catalog otherwise has no way to do.
120+
// The catalog's element shape, stated locally: `SchemaInfo` is not exported,
121+
// and these tests must stay honest even where `SCHEMAS` widens to `any`
122+
// (this file sits outside every tsc program — see the TEST_DEBT ledger — so
123+
// an implicit `any` here would silently stop checking anything).
124+
type CatalogField = { name: string; type: string };
125+
const flowFields = (kind: 'required' | 'optional'): CatalogField[] => SCHEMAS.flow[kind];
126+
127+
it('ships a flow example that actually parses as a Flow (#14782)', () => {
128+
// The catalog stores examples as authored source, so evaluate the literal.
129+
const literal = new Function(`return (${SCHEMAS.flow.example});`)() as unknown;
130+
const result = FlowSchema.safeParse(literal);
131+
expect(
132+
result.success,
133+
`os explain flow's example must parse as a Flow. Issues: ${
134+
result.success ? '' : JSON.stringify(result.error.issues, null, 2)
135+
}`,
136+
).toBe(true);
137+
});
138+
139+
it('documents flow.type as the full FlowSchema type enum (#14782)', () => {
140+
const type = flowFields('required').find((f) => f.name === 'type');
141+
expect(type, 'flow schema should document a `type` field').toBeDefined();
142+
const tokens = (type!.type.match(/'[^']+'|"[^"]+"/g) ?? []).map((t) => t.slice(1, -1));
143+
expect(new Set(tokens)).toEqual(
144+
new Set(['autolaunched', 'record_change', 'schedule', 'screen', 'api']),
145+
);
146+
});
147+
148+
it('teaches the acting user as {$User.Id}, and no catalog example revives $currentUser (#14782)', () => {
149+
expect(SCHEMAS.flow.example).toContain('{$User.Id}');
150+
const entries = Object.entries(SCHEMAS) as Array<[string, { example: string }]>;
151+
for (const [key, info] of entries) {
152+
expect(info.example, `os explain ${key} example`).not.toContain('$currentUser');
153+
}
154+
});
155+
156+
it('never re-teaches `steps` / `trigger` as flow keys — both are aliases, not fields (#14782)', () => {
157+
const declared = [...flowFields('required'), ...flowFields('optional')].map((f) => f.name);
158+
expect(declared).not.toContain('steps');
159+
expect(declared).not.toContain('trigger');
160+
expect(declared).toContain('nodes');
161+
expect(declared).toContain('edges');
162+
});
97163
});

0 commit comments

Comments
 (0)