fix(example-todo): arm task_completion — it bound to nothing and gated on a key nothing reads (#6882)

.changeset/todo-task-completion-trigger-armed.md

@@ -0,0 +1,51 @@
+---
+"@objectstack/example-todo": patch
+---
+
+fix(example-todo): `task_completion` is a real record-change flow again — it bound to nothing and gated on a key nothing reads (#6882)
+
+`examples/app-todo`'s `TaskCompletionFlow` declared `type: 'record_change'` and then
+declared neither key that arms one. It was 1 of the 34 authored flows across the three
+bundled apps, and the only dead one.
+
+**Two faults on one start node, both silent.**
+
+1. **No `triggerType` at all.** `AutomationEngine.resolveTriggerBinding` claims a
+   record-change flow only when the authored token starts with `record-`. With the key
+   absent every later branch missed too (`timeRelative`, `config.schedule`,
+   `flow.type === 'schedule'`, `flow.type === 'api'`), the method returned `undefined`,
+   and `activateFlowTrigger` returned without binding. The flow declared itself
+   record-triggered and was, at runtime, a manual flow that never fired.
+2. **The predicate was written to `triggerCondition`.** The trigger gate is
+   `config.condition` — the key the binding copies and `execute()` evaluates. A node
+   `config` is an open slot by design (ADR-0018), so the misspelling parsed silently.
+   Fixing (1) alone would have been *worse* than dead: the flow would have fired on every
+   update of every task.
+
+**Why no channel reported it.** `getTriggerBindingAudit` — the platform's own silent-miss
+surface, and the source for both the automation plugin's `kernel:bootstrapped` warn loop
+and the CLI startup summary's `unbound` list — opens with `if (!resolved) continue`,
+reading "no binding" as "manual/screen flow, nothing to bind". So the missing key did not
+*add* a diagnostic; it removed the flow from every diagnostic channel there is. The only
+trace anywhere was the startup banner counting one more flow registered than bound, with
+no name and no reason.
+
+**The repair.** `triggerType: 'record-after-update'` plus the predicate moved to
+`config.condition` as `status == "completed" && previous.status != "completed"` — the
+shape `showcase_task_completed` already uses for this exact semantic. `-after-update`
+rather than `-after-write` on purpose: "marked as complete" is a transition, and the
+insert leg has no `previous` to transition from — `previous` binds to `null` there, and
+`previous.status` against `null` aborts the whole CEL predicate with `No such key:
+status` rather than answering false.
+
+A third fault surfaced the moment the flow could run: `get_task` filtered on `{taskId}`,
+an `isInput` variable nothing ever bound (a record-change run seeds `params` from the
+triggering record, which carries `id`, not `taskId`), so the first armed run failed with
+"1 filter condition(s) resolved to nothing and were dropped from the query". It now reads
+`{record.id}`, the handle every other record-change flow in the corpus uses, and the dead
+declaration is gone.
+
+`@objectstack/example-todo` also runs its own vitest suite now (`vitest run`, as
+`app-crm` and `app-showcase` already do) instead of `objectstack test`, which is a
+Quality-Protocol runner that needs a live server and matched no `qa/*.test.json` here —
+so the package's test files had never executed in CI.

examples/app-todo/package.json

@@ -16,7 +16,7 @@
     "build": "objectstack build",
     "validate": "objectstack validate",
     "typecheck": "tsc --noEmit",
-    "test": "objectstack test",
+    "test": "vitest run",
     "test:mcp": "tsx test/mcp-actions.e2e.ts"
   },
   "dependencies": {
@@ -32,6 +32,9 @@
   },
   "devDependencies": {
     "@objectstack/cli": "workspace:*",
+    "@objectstack/core": "workspace:*",
+    "@objectstack/service-automation": "workspace:*",
+    "@objectstack/trigger-record-change": "workspace:*",
     "tsx": "^4.23.1",
     "typescript": "^6.0.3",
     "vitest": "^4.1.10"

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

@@ -120,17 +120,67 @@ export const TaskCompletionFlow: Flow = {
   label: 'Task Completion Process',
   description: 'Flow triggered when a task is marked as complete',
   type: 'record_change',
+  // The other half of "arm it deliberately" (#6882). `draft` is the status the
+  // schema applies when none is authored, and draft flows DO fire — so on a
+  // flow that now genuinely routes, the omission is the ambiguity
+  // `flow-draft-status-ambiguous` exists to report, and it started reporting
+  // here the moment the trigger resolved (while the flow was dead it routed
+  // nowhere, so even that rule skipped it). Declared, not silenced: `active`
+  // says the firing is intended, which is exactly this card's decision. The two
+  // schedule flows above keep their pre-existing `draft` — a separate call.
+  status: 'active',
 
   variables: [
-    { name: 'taskId', type: 'text', isInput: true, isOutput: false },
+    // #6882 — `taskId` used to be declared here as an `isInput` variable and
+    // read by `get_task` as `{taskId}`. Nothing ever bound it: a record-change
+    // run seeds `params` from the triggering RECORD, which carries `id`, not
+    // `taskId`, and `seedDeclaredVariables` binds an input only from
+    // `context.params[name]` (or a `defaultValue`, #4697 — this had neither).
+    // Invisible while the flow was dead; the first armed run failed at
+    // `get_task` with "1 filter condition(s) resolved to nothing and were
+    // dropped from the query: `{taskId}` (at id)". The triggering record is
+    // already in scope as `record`, which is how every other record-change flow
+    // in the corpus addresses it, so the declaration is gone rather than
+    // re-plumbed — declared means bound.
     { name: 'completedTask', type: 'record', isInput: false, isOutput: false },
   ],
 
   nodes: [
-    { id: 'start', type: 'start', label: 'Start', config: { objectName: 'todo_task', triggerCondition: 'record.status != previous.status && record.status == "completed"' } },
+    // #6882 — this start node declared NEITHER of the two keys that arm a
+    // record-change flow, so the flow was registered and never bound:
+    //
+    //   • no `triggerType` at all. `AutomationEngine.resolveTriggerBinding`
+    //     claims a record-change flow only for a token starting with `record-`;
+    //     with the key absent every later branch missed too (`timeRelative`,
+    //     `config.schedule`, `flow.type === 'schedule'|'api'`) and the method
+    //     returned `undefined`, so `activateFlowTrigger` returned without
+    //     binding. `getTriggerBindingAudit` then SKIPS it (`if (!resolved)
+    //     continue` — it reads as a manual/screen flow), which is why nothing
+    //     anywhere reported the dead flow.
+    //   • the predicate was written to `triggerCondition`, which no code reads.
+    //     The trigger gate is `config.condition` — the key the binding copies
+    //     and `execute()` evaluates. A node `config` is an open slot by design
+    //     (ADR-0018), so the misspelling parsed silently; arming the trigger
+    //     without moving it would have fired this flow on EVERY update.
+    //
+    // `record-after-update` (not `-write`): "marked as complete" is a
+    // transition, so the insert leg has no `previous` to transition FROM — the
+    // same shape the showcase's `showcase_task_completed` uses for this exact
+    // semantic. Keeping insert out of the binding is also what makes the
+    // predicate total: `previous` is bound to `null` on the insert leg, and
+    // `previous.status` against `null` aborts the whole CEL predicate with
+    // `No such key: status` (measured) rather than answering false.
+    {
+      id: 'start', type: 'start', label: 'Start',
+      config: {
+        objectName: 'todo_task',
+        triggerType: 'record-after-update',
+        condition: 'status == "completed" && previous.status != "completed"',
+      },
+    },
     {
       id: 'get_task', type: 'get_record', label: 'Get Completed Task',
-      config: { objectName: 'todo_task', filter: { id: '{taskId}' }, outputVariable: 'completedTask' },
+      config: { objectName: 'todo_task', filter: { id: '{record.id}' }, outputVariable: 'completedTask' },
     },
     // A plain exclusive gateway — the branching is on the OUT-EDGES (e3/e4
     // carry the predicate and its negation). It used to also set

examples/app-todo/test/task-completion-trigger.test.ts

@@ -0,0 +1,207 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#6882] `task_completion` is a LIVE record-change flow — binding, gate, and
+ * the silence that hid it.
+ *
+ * The defect this pins was not a wrong answer; it was **no answer anywhere**.
+ * `TaskCompletionFlow` declared `type: 'record_change'` and then declared
+ * neither key that arms one:
+ *
+ *   1. no `config.triggerType`. `AutomationEngine.resolveTriggerBinding` claims
+ *      a record-change flow only for a token starting with `record-`; absent,
+ *      every later branch missed too and it returned `undefined`, so
+ *      `activateFlowTrigger` returned without binding.
+ *   2. the predicate was written to `config.triggerCondition`, a key nothing
+ *      reads. The gate is `config.condition`. A node `config` is an open slot
+ *      by design (ADR-0018), so the misspelling parsed silently — and arming
+ *      (1) without fixing (2) would have been WORSE than dead: the flow would
+ *      have fired on every single update.
+ *
+ * Why nothing caught it, and why the reverse check below asserts an ABSENCE:
+ * `getTriggerBindingAudit` — the platform's own silent-miss channel, and the
+ * source for both the automation plugin's `kernel:bootstrapped` warn loop and
+ * the CLI startup summary's `unbound` list — opens with
+ * `if (!resolved) continue`, reading "no binding" as "manual/screen flow,
+ * nothing to bind". So removing `triggerType` does not ADD a diagnostic; it
+ * REMOVES the flow from every diagnostic channel there is. That inverted
+ * direction is the whole reason this needs a test rather than a lint rule
+ * reading a warning nobody emits.
+ *
+ * The suite drives the app's REAL metadata (`allFlows`, the real `Task`
+ * object) through a real kernel and real writes, so it fails if any link is
+ * re-broken: the trigger token, the condition key, the predicate's dialect, or
+ * the engine's resolver.
+ */
+
+import { describe, it, expect, afterEach } from 'vitest';
+import { ObjectKernel } from '@objectstack/core';
+import { ObjectQLPlugin } from '@objectstack/objectql';
+import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
+import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation';
+import { RecordChangeTriggerPlugin } from '@objectstack/trigger-record-change';
+
+import { allFlows, TaskCompletionFlow } from '../src/flows/index.js';
+import { Task } from '../src/objects/task.object.js';
+
+const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
+
+/** Every driver a test opened, closed when that test ends. */
+const openDrivers: Array<{ disconnect?: () => Promise<void> }> = [];
+afterEach(async () => {
+  while (openDrivers.length) {
+    try { await openDrivers.pop()?.disconnect?.(); } catch { /* noop */ }
+  }
+});
+
+/**
+ * A real kernel with the app's real `todo_task` object: ObjectQL over an
+ * in-process sqlite-wasm database (the same driver `test/mcp-actions.e2e.ts`
+ * boots), the automation engine, and the record-change trigger. No test double
+ * anywhere in the chain — #6882 was precisely a chain that looked correct in
+ * the metadata and bound to nothing at runtime.
+ */
+async function bootTodoKernel(): Promise<{
+  automation: AutomationEngine & Record<string, any>;
+  data: any;
+}> {
+  const kernel = new ObjectKernel({ logger: { level: 'silent' } } as any);
+  await kernel.use(new ObjectQLPlugin());
+  await kernel.use(new AutomationServicePlugin());
+  await kernel.use(new RecordChangeTriggerPlugin());
+  await kernel.bootstrap();
+
+  const objectql: any = kernel.getService('objectql');
+  const data: any = kernel.getService('data');
+  const automation = kernel.getService<AutomationEngine>('automation') as AutomationEngine & Record<string, any>;
+
+  const driver: any = new SqliteWasmDriver({ filename: ':memory:' });
+  await driver.connect();
+  objectql.registerDriver(driver, true);
+  openDrivers.push(driver);
+  objectql.registry.registerObject(Task, 'todo', 'todo');
+  await objectql.syncSchemas();
+
+  for (const flow of allFlows) automation.registerFlow(flow.name, flow);
+  return { automation, data };
+}
+
+/** The `start` node config of a flow, as authored. */
+function startConfig(flow: unknown): Record<string, unknown> {
+  const nodes = (flow as { nodes?: Array<{ type?: string; config?: Record<string, unknown> }> }).nodes ?? [];
+  return nodes.find((n) => n?.type === 'start')?.config ?? {};
+}
+
+describe('#6882 — app-todo `task_completion` is armed, not dead', () => {
+  it('declares the two keys the engine actually reads', () => {
+    const config = startConfig(TaskCompletionFlow);
+
+    // The token has to be one the record-change trigger maps to a hook, not
+    // merely `record-`-prefixed: the engine routes ANY `record-` string to the
+    // trigger, which then maps an unknown token to no hook at all.
+    expect(config.triggerType).toBe('record-after-update');
+    expect(config.objectName).toBe('todo_task');
+
+    // `condition` is the gate; `triggerCondition` is the misspelling that made
+    // the predicate inert. Assert BOTH, so re-introducing the typo alongside a
+    // correct `condition` is still caught.
+    expect(typeof config.condition).toBe('string');
+    expect(config).not.toHaveProperty('triggerCondition');
+  });
+
+  it('no start node in the app writes its predicate to an unread key', () => {
+    // The class, not just the instance: a node `config` is an open slot
+    // (ADR-0018), so any misspelling here parses silently and gates nothing.
+    for (const flow of allFlows) {
+      const config = startConfig(flow);
+      for (const key of ['triggerCondition', 'trigger_condition', 'startCondition']) {
+        expect(config, `flow '${flow.name}' start node`).not.toHaveProperty(key);
+      }
+    }
+  });
+
+  it('BINDS: the flow is wired to afterUpdate on todo_task at runtime', async () => {
+    const { automation } = await bootTodoKernel();
+
+    const state = automation.getFlowRuntimeStates().find((s: any) => s.name === 'task_completion');
+    expect(state).toBeDefined();
+    expect(state!.bound, 'the flow resolved a binding and the trigger accepted it').toBe(true);
+    expect(state!.triggerType).toBe('record_change');
+    expect(state!.object).toBe('todo_task');
+
+    // And the silent-miss audit has nothing to say about it, because there is
+    // nothing left unbound.
+    const audit = automation.getTriggerBindingAudit();
+    expect(audit.find((a: any) => a.flowName === 'task_completion')).toBeUndefined();
+  });
+
+  it('REVERSE: drop `triggerType` and the flow vanishes from every audit channel', async () => {
+    const { automation } = await bootTodoKernel();
+
+    // The pre-#6882 shape, rebuilt from the real flow so it cannot drift: the
+    // authored trigger token removed, everything else identical.
+    const dead = JSON.parse(JSON.stringify(TaskCompletionFlow)) as any;
+    dead.name = 'task_completion_unarmed_fixture';
+    delete dead.nodes.find((n: any) => n.type === 'start').config.triggerType;
+    automation.registerFlow(dead.name, dead);
+
+    const state = automation.getFlowRuntimeStates().find((s: any) => s.name === dead.name);
+    expect(state!.bound).toBe(false);
+    // `triggerType` undefined is the resolver having given up entirely — the
+    // flow reads as manual/screen, which is why nothing downstream reports it.
+    expect(state!.triggerType).toBeUndefined();
+
+    // The direction that makes this a test and not a lint rule: the audit gains
+    // NOTHING. An unbound-but-resolvable flow appears here; this one does not,
+    // so no warn loop and no CLI `unbound` line ever names it.
+    const audit = automation.getTriggerBindingAudit();
+    expect(audit.find((a: any) => a.flowName === dead.name)).toBeUndefined();
+
+    // Non-vacuous: the same fixture WITH the key is bound and audited-clean —
+    // so the assertions above are about the missing key, not about the fixture
+    // failing to register.
+    expect(
+      automation.getFlowRuntimeStates().find((s: any) => s.name === 'task_completion')!.bound,
+    ).toBe(true);
+  });
+
+  it('GATES: only a transition INTO completed fires the flow', async () => {
+    const { automation, data } = await bootTodoKernel();
+    const ctx = { context: { userId: 'u_todo' } };
+    const runCount = async () => (await automation.listRuns('task_completion')).length;
+
+    // `completed_date` is seeded on CREATE (the engine allows a read-only field
+    // to be seeded by an insert, and the object's `completed_date_required`
+    // rule refuses the completion write otherwise — see the note on that rule).
+    const created = await data.insert('todo_task', {
+      subject: 'Water the plants',
+      status: 'not_started',
+      priority: 'normal',
+      is_recurring: false,
+      completed_date: '2026-08-09T10:00:00.000Z',
+    }, ctx);
+    const id = Array.isArray(created) ? created[0].id : created.id;
+    await sleep(150);
+    expect(await runCount(), 'an INSERT must not reach an after-update flow').toBe(0);
+
+    // A write that should NOT fire it: the record changes, the status does not.
+    await data.update('todo_task', { priority: 'high' }, { where: { id }, ...ctx });
+    await sleep(150);
+    expect(await runCount(), 'the start condition must gate on the status transition').toBe(0);
+
+    // A write that SHOULD fire it.
+    await data.update('todo_task', { status: 'completed' }, { where: { id }, ...ctx });
+    await sleep(400);
+    const afterTransition = await automation.listRuns('task_completion');
+    expect(afterTransition.length, 'the completion transition must launch the flow').toBe(1);
+    // It RAN, rather than merely being dispatched: the whole flow reached `end`.
+    expect(afterTransition[0].status).toBe('completed');
+    expect((afterTransition[0].steps ?? []).map((s: any) => s.nodeId)).toContain('get_task');
+
+    // And it does not re-fire on every later save of an already-completed task —
+    // the half of the predicate that `previous` exists for.
+    await data.update('todo_task', { progress_percent: 100 }, { where: { id }, ...ctx });
+    await sleep(300);
+    expect(await runCount(), 're-saving a completed task must not re-fire it').toBe(1);
+  }, 30000);
+});