Skip to content

Commit fb33767

Browse files
huangyiireneclaude
huangyiirene
and
claude
authored
fix(example-todo): move both loop bodies into config.body so currentTask binds and the escalation notice is sent (#19262)
Fixes #19206 `Clause-②: no` ## What was wrong Both `loop` nodes in `examples/app-todo/src/flows/task.flow.ts` declared `collection` + `iteratorVariable` and **no `config.body`**. That is the **legacy flat-graph** form: `packages/services/service-automation/src/builtin/loop-node.ts` opens with `if (raw.body == null)`, sets `$loopItems` / `$loopIndex` and returns **without binding `iteratorVariable`**. The per-item steps were wired as ordinary nodes *after* the container, so every `{currentTask.…}` token in them resolved to nothing. Measured on a real engine in this PR's control test (`CONTROL: the pre-fix body-less shape still dies at update_priority`): ``` update_record: refusing to run — 1 filter condition(s) resolved to nothing and were dropped from the query: `{currentTask.id}` (at id). ``` `notify_owner` sits one node **behind** that refusal, so **no escalation notice was sent at all** — and the control asserts that too (`emits` has length 0). `task_reminder` had the identical shape. ## The repair — established from the schema and the engine The authority is `LoopConfigSchema` / `FlowRegionSchema` in `packages/spec/src/automation/control-flow.zod.ts` (ADR-0031 representation **(B)**, nested sub-structure) plus the executor's structured branch: - `config.body` is a `FlowRegionSchema`: `{ nodes: min 1, edges: default [] }`, **single-entry / single-exit / acyclic** (`analyzeRegion`), rejected at `registerFlow()` via `validateControlFlow` when it is not. - The executor binds `iteratorVariable` in the **enclosing** scope per item and runs the region through `engine.runRegion`; the loop node's **ordinary out-edge is the after-loop continuation**, which is why `e3` now goes `loop → end`. Each body carries one `try_catch` (`flow-loop-body-uncontained`, the shape the lint rule's own hint prescribes, and the form `examples/app-showcase` already demonstrates), with the minimal handler — one bare `assignment` node. On `overdue_escalation` **one** `try_catch` wraps **both** per-item steps rather than one each: the notice announces the escalation the update performs, so a row whose update was refused must not be told its priority was raised. `maxIterations: 200` states, on the iterating side, the same bound the `get_record` step's `limit: 200` already imposes. ## What the notice actually renders — the reading this card unblocks With the loop body executing, the escalation notice is sent, and (composed with the `days_overdue` formula that landed in #19205 / #18584) it interpolates. Read off the captured `messaging.emit()` payload in a real run: ``` title: URGENT: task overdue — Alpha body: Due 2026-09-16, 4 day(s) overdue. ``` Three seeded rows render three distinct notices with their own owner, subject, due date and day count — no leaked binding, no blank hole. ## Tests `examples/app-todo/test/loop-body-iteration.test.ts` — drives the **real** flows on a real `AutomationEngine` over a real sqlite-wasm database holding the app's real `todo_task`. The only double is the `messaging` service, and it is a **capture**, not a simulation: capturing `emit()` is how the rendered title/body becomes readable at all. 1. ESCALATION — run succeeds, the seeded row's `priority` is `urgent` (so the body's first node matched the right row), exactly one notice, and its rendered body carries the real day count and no unresolved brace. 2. EVERY item is processed, each with its own values (a leaked binding would show as three identical notices). 3. CONTAINMENT — one row with an unresolvable recipient fails inside the body's `try_catch` and the sweep still reaches the rows after it. 4. REMINDER — the second loop in the file binds its iterator too. 5. CONTROL — the pre-fix body-less shape, rebuilt from the real flow, still dies at `update_priority` and notifies nobody. `test/overdue-escalation-days-overdue.test.ts` (the #18584 pin) keeps pinning: its `node()` helper now walks the ADR-0031 region slots, because the nodes it inspects moved into the loop body. The question it asks is about those nodes' **config**, which nesting does not change. ## Evidence - `pnpm --filter @objectstack/example-todo exec vitest run` — **exit 0**, 7 files / 238 tests passed. - `pnpm --filter @objectstack/example-todo typecheck` — the package's own script is `tsc --noEmit` (read from its `package.json`, one leg) — **exit 0**. - `pnpm --filter '@objectstack/example-todo^...' build` — **exit 0**. - Gate families derived from the real change set with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands`: **37 derived, 37 run, 0 NOT-MEASURED, 0 UNRUN** (`--ran` reconciliation with exit codes recorded before any pipe). `check:dual-build-cjs-loads` first answered **exit 3 = PREREQUISITE NOT MET = not measured**; after the missing `dist/` was present it was re-run and answered **exit 0**. - `pnpm lint` (repo-wide `eslint . --no-inline-config`, the whole population, not a narrowed run) — **exit 0** at `c6ca194c2`. ## Changeset `skip-changeset`: `examples/app-todo/package.json` declares `"private": true` and the diff touches nothing else, so this publishes nothing from any released package. ## Scope `examples/app-todo/` only. No engine change was needed — and none was made: the container the schema already declares is what binds the iterator. `packages/spec/**` and `content/docs/releases/**` are untouched. ## Acceptance notes - `noted, not filed:` the two scheduled sweeps read date macros (`{tomorrow}`, `{3_days_ago}`) that `interpolateFilter` deliberately passes through verbatim for the data layer to resolve. Working as designed here; recorded only because the same brace syntax means something different one line away in the same config. Not a defect, not filed. --- _Generated by [Claude Code](https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1739f71 commit fb33767

3 files changed

Lines changed: 419 additions & 38 deletions

File tree

‎examples/app-todo/src/flows/task.flow.ts‎

Lines changed: 136 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,69 @@ export const TaskReminderFlow: Flow = {
5858
config: { objectName: 'todo_task', filter: { due_date: '{tomorrow}', status: { $ne: 'completed' } }, outputVariable: 'tasksToRemind', limit: 200 },
5959
},
6060
{
61+
// #19206 — the per-item steps live in `config.body`, and that is what
62+
// binds `currentTask` at all. A `loop` node carrying only `collection` +
63+
// `iteratorVariable` takes the LEGACY flat-graph path
64+
// (`loop-node.ts`: `if (raw.body == null)`), which sets `$loopItems` /
65+
// `$loopIndex` and returns WITHOUT ever binding `iteratorVariable` — so
66+
// every `{currentTask.X}` token in the nodes wired after it resolved to
67+
// nothing. The structured container (ADR-0031) is the form that
68+
// iterates: it binds `iteratorVariable` in the enclosing scope and runs
69+
// the body region once per item, and the node's ordinary out-edge
70+
// (`→ end`) is the after-loop continuation.
6171
id: 'loop_tasks', type: 'loop', label: 'Loop Through Tasks',
62-
config: { collection: '{tasksToRemind}', iteratorVariable: 'currentTask' },
63-
},
64-
{
65-
// `notify` is what actually delivers (#4343): it hands the messaging
66-
// service the notification — the in-app inbox by default, and email once
67-
// `@objectstack/plugin-email` is installed. The `script` node this
68-
// replaced only ever logged a line and reported success.
69-
id: 'send_reminder', type: 'notify', label: 'Send Reminder',
7072
config: {
71-
recipients: '{currentTask.owner}',
72-
title: 'Task due tomorrow: {currentTask.subject}',
73-
message: 'Due {currentTask.due_date} · priority {currentTask.priority}.',
74-
sourceObject: 'todo_task',
75-
sourceId: '{currentTask.id}',
73+
collection: '{tasksToRemind}',
74+
iteratorVariable: 'currentTask',
75+
// The read above is capped at `limit: 200`, so this cap is the same
76+
// bound stated on the side that iterates it — the engine FAILS the
77+
// node on a longer collection rather than truncating in silence.
78+
maxIterations: 200,
79+
body: {
80+
nodes: [
81+
{
82+
// Per-iteration containment (`flow-loop-body-uncontained`). A
83+
// `loop` body has no error handling of its own: the container
84+
// iterates with a bare `await`, so a body node answering
85+
// `success: false` propagates straight out and ends the WHOLE
86+
// sweep. `notify` fails on an empty resolved recipient set, so
87+
// one task with a blank `owner` would leave every later task
88+
// unreminded. The guard is a `try_catch` INSIDE the body.
89+
id: 'guard_reminder', type: 'try_catch', label: 'Guarded Reminder',
90+
config: {
91+
try: {
92+
nodes: [
93+
{
94+
// `notify` is what actually delivers (#4343): it hands the
95+
// messaging service the notification — the in-app inbox by
96+
// default, and email once `@objectstack/plugin-email` is
97+
// installed. The `script` node this replaced only ever
98+
// logged a line and reported success.
99+
id: 'send_reminder', type: 'notify', label: 'Send Reminder',
100+
config: {
101+
recipients: '{currentTask.owner}',
102+
title: 'Task due tomorrow: {currentTask.subject}',
103+
message: 'Due {currentTask.due_date} · priority {currentTask.priority}.',
104+
sourceObject: 'todo_task',
105+
sourceId: '{currentTask.id}',
106+
},
107+
},
108+
],
109+
},
110+
// The shortest handler that works: ONE bare `assignment` node
111+
// with no `config`. A `catch` region cannot be empty —
112+
// `FlowRegionSchema.nodes` is `.min(1)`, so `catch: {}` and
113+
// `catch: { nodes: [] }` are both refused by the parse, and
114+
// omitting `catch` entirely parses while containing NOTHING.
115+
catch: {
116+
nodes: [
117+
{ id: 'reminder_failed', type: 'assignment', label: 'Reminder Failed (contained)' },
118+
],
119+
},
120+
},
121+
},
122+
],
123+
},
76124
},
77125
},
78126
{ id: 'end', type: 'end', label: 'End' },
@@ -81,8 +129,9 @@ export const TaskReminderFlow: Flow = {
81129
edges: [
82130
{ id: 'e1', source: 'start', target: 'get_upcoming_tasks', type: 'default' },
83131
{ id: 'e2', source: 'get_upcoming_tasks', target: 'loop_tasks', type: 'default' },
84-
{ id: 'e3', source: 'loop_tasks', target: 'send_reminder', type: 'default' },
85-
{ id: 'e4', source: 'send_reminder', target: 'end', type: 'default' },
132+
// The loop's ordinary out-edge is the AFTER-loop continuation; the
133+
// per-item step is `config.body`, not a node wired after the container.
134+
{ id: 'e3', source: 'loop_tasks', target: 'end', type: 'default' },
86135
],
87136
};
88137

@@ -117,26 +166,78 @@ export const OverdueEscalationFlow: Flow = {
117166
},
118167
},
119168
{
169+
// #19206 — same repair as `task_reminder` above, and this is the flow
170+
// where the omission was MEASURED: with the per-item steps wired as
171+
// ordinary nodes AFTER a body-less `loop`, `currentTask` was never
172+
// bound, and the run died at `update_priority` with "refusing to run -
173+
// 1 filter condition(s) resolved to nothing: {currentTask.id} (at id)".
174+
// `notify_owner` sits one node BEHIND that refusal, so no escalation
175+
// notice was ever sent — the loud refusal is the only reason a filter
176+
// that resolved to nothing did not match every task in the table.
120177
id: 'loop_overdue', type: 'loop', label: 'Loop Through Overdue Tasks',
121-
config: { collection: '{overdueTasks}', iteratorVariable: 'currentTask' },
122-
},
123-
{
124-
id: 'update_priority', type: 'update_record', label: 'Escalate Priority',
125178
config: {
126-
objectName: 'todo_task',
127-
filter: { id: '{currentTask.id}' },
128-
fields: { priority: 'urgent', tags: ['important', 'follow_up'] },
129-
},
130-
},
131-
{
132-
id: 'notify_owner', type: 'notify', label: 'Notify Task Owner',
133-
config: {
134-
recipients: '{currentTask.owner}',
135-
title: 'URGENT: task overdue — {currentTask.subject}',
136-
message: 'Due {currentTask.due_date}, {currentTask.days_overdue} day(s) overdue.',
137-
severity: 'critical',
138-
sourceObject: 'todo_task',
139-
sourceId: '{currentTask.id}',
179+
collection: '{overdueTasks}',
180+
iteratorVariable: 'currentTask',
181+
// Same bound as the read above (`limit: 200`), stated on the side
182+
// that iterates: a longer collection fails the node rather than
183+
// truncating in silence.
184+
maxIterations: 200,
185+
body: {
186+
nodes: [
187+
{
188+
// Per-iteration containment (`flow-loop-body-uncontained`): both
189+
// per-item steps are fallible — `update_record` answers
190+
// `success: false` on a refused write, `notify` on an empty
191+
// resolved recipient set — and an uncontained failure propagates
192+
// straight out of the container, ending the sweep at the first
193+
// bad row with every later task left unescalated.
194+
//
195+
// ONE `try_catch` over BOTH steps, not one per step: the notice
196+
// announces the escalation the update performs, so a row whose
197+
// update was refused must not be told its priority was raised.
198+
id: 'guard_escalation', type: 'try_catch', label: 'Guarded Escalation',
199+
config: {
200+
try: {
201+
nodes: [
202+
{
203+
id: 'update_priority', type: 'update_record', label: 'Escalate Priority',
204+
config: {
205+
objectName: 'todo_task',
206+
filter: { id: '{currentTask.id}' },
207+
fields: { priority: 'urgent', tags: ['important', 'follow_up'] },
208+
},
209+
},
210+
{
211+
id: 'notify_owner', type: 'notify', label: 'Notify Task Owner',
212+
config: {
213+
recipients: '{currentTask.owner}',
214+
title: 'URGENT: task overdue — {currentTask.subject}',
215+
// `days_overdue` is the formula field the record
216+
// projection carries (#18584); the loop binding this
217+
// template reads it from is what #19206 restores.
218+
message: 'Due {currentTask.due_date}, {currentTask.days_overdue} day(s) overdue.',
219+
severity: 'critical',
220+
sourceObject: 'todo_task',
221+
sourceId: '{currentTask.id}',
222+
},
223+
},
224+
],
225+
edges: [
226+
{ id: 'be1', source: 'update_priority', target: 'notify_owner', type: 'default' },
227+
],
228+
},
229+
// One bare `assignment` — the shortest handler that works; a
230+
// `catch` region's `nodes` is `.min(1)`, so an empty one is
231+
// refused at parse and an omitted one contains nothing.
232+
catch: {
233+
nodes: [
234+
{ id: 'escalation_failed', type: 'assignment', label: 'Escalation Failed (contained)' },
235+
],
236+
},
237+
},
238+
},
239+
],
240+
},
140241
},
141242
},
142243
{ id: 'end', type: 'end', label: 'End' },
@@ -145,9 +246,8 @@ export const OverdueEscalationFlow: Flow = {
145246
edges: [
146247
{ id: 'e1', source: 'start', target: 'get_overdue_tasks', type: 'default' },
147248
{ id: 'e2', source: 'get_overdue_tasks', target: 'loop_overdue', type: 'default' },
148-
{ id: 'e3', source: 'loop_overdue', target: 'update_priority', type: 'default' },
149-
{ id: 'e4', source: 'update_priority', target: 'notify_owner', type: 'default' },
150-
{ id: 'e5', source: 'notify_owner', target: 'end', type: 'default' },
249+
// The container's ordinary out-edge is the after-loop continuation.
250+
{ id: 'e3', source: 'loop_overdue', target: 'end', type: 'default' },
151251
],
152252
};
153253

0 commit comments

Comments
 (0)