fix(service-automation): get_record names id in the projection it hands the engine, so output.id is the row id whatever fields names

.changeset/22344-get-record-id-projection.md

@@ -0,0 +1,11 @@
+---
+'@objectstack/service-automation': patch
+---
+
+fix(service-automation): a `get_record` whose `fields` leave out `id` answers the row's id as its `id` output on the SQL drivers
+
+Clause-②: no
+
+- **What was wrong.** On its single-row branch (no `limit` above 1) a `get_record` node answers `output: { record, id, object }`, and a later node reads that `id` as `{read.id}` (for a node whose id is `read`). The node handed the author's `fields` to the engine unchanged, and the SQL drivers return exactly the columns a projection names. So a `get_record` with `fields: ['title']` read a row with no `id` column, and `{read.id}` was empty with no error. A later `update_record` filtering on it was refused for a filter condition that resolved to nothing, and any other reference read nothing.
+- **What it does now.** That branch names `id` in the projection it hands the engine, so `{read.id}` is the row's id whatever `fields` names, on every driver. The record the node serves, and binds to its `outputVariable`, carries that `id` beside the columns the author named, so the variable's `id` reads the same value. The stored-metadata tables (`sys_metadata`, `sys_metadata_history`) are read the same way: the served row is the named columns plus `id`.
+- **Unchanged.** A `get_record` with no `fields`, or an empty list, still reads the whole row. A projection that already names `id` is handed over as written. The multi-row branch (`limit` above 1) answers `records` only and is untouched.

packages/services/service-automation/src/builtin/crud-nodes.ts

@@ -278,6 +278,32 @@ async function serveFamilyRead<A>(
     return answer;
 }
 
+/**
+ * [#22344] The projection `get_record`'s single-row (`findOne`) branch hands
+ * the engine: the author's `fields`, with `id` named.
+ *
+ * That branch answers `output.id` from the row it read, and a later node reads
+ * it as `{<node>.id}`. The SQL drivers return exactly the columns a projection
+ * names, so the author's `fields` handed over unchanged read a row with no
+ * `id` whenever they left it out: `output.id` was `undefined`, with no error.
+ * Naming `id` here makes the declared output independent of the author's
+ * projection, on every driver and on the stored-metadata family path alike
+ * ({@link serveFamilyRead} takes this projection as the node's own).
+ *
+ * `record` then carries the `id` the engine returned, so `{var.id}` through the
+ * node's `outputVariable` agrees with `{<node>.id}`; it is not taken back off,
+ * which would be a second projection layer over the engine's answer.
+ *
+ * Only a non-empty projection is touched: no `fields`, or an empty list, is a
+ * whole-row read already, and a projection that names `id` is handed over as
+ * written. The `find` branch (`limit > 1`) answers rows only, with no
+ * top-level `id`, and does not come through here.
+ */
+function projectionNamingId(fields: string[] | undefined): string[] | undefined {
+    if (!fields || fields.length === 0 || fields.includes('id')) return fields;
+    return [...fields, 'id'];
+}
+
 /**
  * [#21623] Refuse a node whose filter EVALUATES the stored-metadata family's
  * body or content hash, the way the generic data door refuses the same filter.
@@ -498,7 +524,9 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
                             metrics: { selected: Array.isArray(records) ? records.length : 0 },
                         };
                     }
-                    const record = await serveFamilyRead(data, objectName, fields, (projection) =>
+                    // [#22344] `output.id` is read off the row, so the
+                    // projection the engine is handed names `id`.
+                    const record = await serveFamilyRead(data, objectName, projectionNamingId(fields), (projection) =>
                         data.findOne(objectName, { where: filter, fields: projection, context: dataCtx }));
                     if (outputVariable) variables.set(outputVariable, record);
                     return {

packages/services/service-automation/src/builtin/get-record-formula-projection.integration.test.ts

@@ -16,6 +16,10 @@
  * both run identities. The reference is the same node with the formula left
  * out of `fields`: the formula read must serve that row plus the formula.
  *
+ * [#22344] The one-row branch also names `id` in the projection it hands the
+ * engine (its `output.id` is read off the row), so the row it serves carries
+ * `id` beside the named fields; the list branch serves the named fields only.
+ *
  * Composition: the real stack the other `*.integration.test.ts` files in this
  * package boot — `ObjectKernel`, `ObjectQLPlugin`, `driver-sql` on
  * better-sqlite3 `:memory:`, and the real `AutomationServicePlugin`.
@@ -129,16 +133,19 @@ describe('flow get_record serves exactly its `fields` when one is a formula (#22
 
   const CASES: Array<[RunAs, Branch]> = [['system', 'one'], ['system', 'list'], ['user', 'one'], ['user', 'list']];
 
+  /** The one-row branch's `id` (#22344); the list branch adds nothing. */
+  const idOf = (branch: Branch): Record<string, unknown> => (branch === 'one' ? { id: 'line_1' } : {});
+
   it.each(CASES)('runAs %s, %s: the formula read serves the named fields plus the formula, nothing else', async (runAs, branch) => {
     const row = await served(runAs, branch, ['name', 'total_price']);
-    expect(Object.keys(row).sort()).toEqual(['name', 'total_price']);
+    expect(Object.keys(row).sort()).toEqual([...Object.keys(idOf(branch)), 'name', 'total_price']);
     // 3 × 7 — computed from two columns the node never named.
     expect(row.total_price).toBe(21);
   });
 
   it.each(CASES)('runAs %s, %s: it equals the same read without the formula, plus the formula value', async (runAs, branch) => {
     const without = await served(runAs, branch, ['name', 'quantity']);
-    expect(without, 'control: a projection with no formula is the named columns').toStrictEqual({ name: 'Widget', quantity: 3 });
+    expect(without, 'control: a projection with no formula is the named columns').toStrictEqual({ ...idOf(branch), name: 'Widget', quantity: 3 });
     const withFormula = await served(runAs, branch, ['name', 'quantity', 'total_price']);
     expect(withFormula).toStrictEqual({ ...without, total_price: 21 });
   });

packages/services/service-automation/src/builtin/get-record-id-projection.integration.test.ts

@@ -0,0 +1,187 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * A `get_record` node answers `output: { record, id, object }` on its
+ * single-row (`findOne`) branch, and a later node reads that `id` as
+ * `{<node>.id}`. The `id` must be the row's id whatever the author's `fields`
+ * projection names.
+ *
+ * The SQL drivers return exactly the columns a projection names. The node used
+ * to hand the author's `fields` to the engine unchanged, so a projection
+ * without `id` read the row with no `id` column: `output.id` was `undefined`,
+ * and a downstream `update_record` filtering on `{read.id}` was refused for an
+ * empty template. The node now names `id` in the projection it hands the
+ * engine on that branch, and only there:
+ *  - a projection that already names `id` is handed over as written (no
+ *    second `id`);
+ *  - no projection, or an empty one, stays a whole-row read (no projection is
+ *    invented);
+ *  - the `find` branch (`limit > 1`) declares no top-level `id` and is not
+ *    touched.
+ *
+ * `record` carries the `id` the engine returned, so `{rec.id}` through the
+ * node's `outputVariable` agrees with `{read.id}`; the row's other unnamed
+ * columns stay out.
+ *
+ * Composition: the real stack the other `*.integration.test.ts` files in this
+ * package boot: `ObjectKernel`, `ObjectQLPlugin`, `driver-sql` on
+ * better-sqlite3 `:memory:`, and the real `AutomationServicePlugin` for the
+ * flow-level case. The node-level cases run the registered `get_record`
+ * executor itself over the same engine, behind a recorder that keeps the
+ * projection each read handed it.
+ */
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { ObjectKernel } from '@objectstack/core';
+import { ObjectQLPlugin, type ObjectQL } from '@objectstack/objectql';
+import { SqlDriver } from '@objectstack/driver-sql';
+import { AutomationServicePlugin } from '../plugin.js';
+import { AutomationEngine } from '../engine.js';
+import { registerCrudNodes } from './crud-nodes.js';
+
+const SYS = { isSystem: true } as const;
+
+const OBJECT = {
+  name: 'pin_get_record_id',
+  label: 'Pin get_record id',
+  fields: {
+    title: { name: 'title', label: 'Title', type: 'text' },
+    status: { name: 'status', label: 'Status', type: 'text' },
+  },
+};
+
+function makeLogger(): any {
+  const l: any = { info() {}, warn() {}, error() {}, debug() {}, trace() {}, fatal() {} };
+  l.child = () => l;
+  return l;
+}
+
+type Executor = { execute: (node: any, variables: Map<string, unknown>, context: any) => Promise<any> };
+
+describe('get_record answers the row id as output.id whatever its fields projection names', () => {
+  let kernel: ObjectKernel;
+  let ql: ObjectQL;
+  let automation: AutomationEngine;
+  let rowId: string;
+  /** The projection each engine read was handed, in order. */
+  const handed: Array<{ method: string; fields: unknown }> = [];
+  let getRecord: Executor;
+
+  beforeAll(async () => {
+    kernel = new ObjectKernel({ logger: { level: 'fatal' } });
+    await kernel.use(new ObjectQLPlugin());
+    await kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' }));
+    await kernel.bootstrap();
+    ql = kernel.getService<ObjectQL>('objectql');
+    automation = kernel.getService<AutomationEngine>('automation');
+
+    const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
+    await driver.connect();
+    ql.registerDriver(driver, true);
+    ql.registry.registerObject(OBJECT as any, 'pin-get-record-id', 'pin-get-record-id');
+    await ql.syncSchemas();
+
+    const row: any = await ql.insert(OBJECT.name, { title: 'alpha', status: 'open' }, { context: SYS });
+    rowId = row.id;
+    expect(typeof rowId, 'control: the engine assigned the row an id').toBe('string');
+
+    // The executor itself, over the same engine: a recorder in front of the
+    // engine keeps the projection each read was handed.
+    const recorder = new Proxy(ql as any, {
+      get(target, key) {
+        const value = Reflect.get(target, key, target);
+        if (key === 'findOne' || key === 'find') {
+          return (object: string, query: any) => {
+            handed.push({ method: key, fields: Array.isArray(query?.fields) ? [...query.fields] : query?.fields });
+            return value.call(target, object, query);
+          };
+        }
+        return typeof value === 'function' ? value.bind(target) : value;
+      },
+    });
+    const executors = new Map<string, Executor>();
+    const capture = new Proxy(new AutomationEngine(makeLogger()), {
+      get(target, key, receiver) {
+        if (key === 'registerNodeExecutor') return (e: any) => { executors.set(e.type, e); };
+        return Reflect.get(target, key, receiver);
+      },
+    });
+    registerCrudNodes(capture as AutomationEngine, { logger: makeLogger(), getService: (n: string) => (n === 'data' ? recorder : undefined) } as any);
+    getRecord = executors.get('get_record')!;
+    expect(getRecord, 'control: the get_record executor was registered').toBeDefined();
+  }, 60_000);
+
+  afterAll(async () => {
+    try { await kernel?.shutdown(); } catch { /* best-effort teardown */ }
+  });
+
+  /** Run the node once and return its result and the projection the engine was handed. */
+  async function readOne(config: Record<string, unknown>): Promise<{ result: any; fields: unknown }> {
+    handed.length = 0;
+    const result = await getRecord.execute(
+      { id: 'read', type: 'get_record', config: { objectName: OBJECT.name, filter: { title: 'alpha' }, ...config } },
+      new Map(),
+      { runAs: 'system', flowName: 'pin_get_record_id' },
+    );
+    expect(result.success, `the read failed: ${JSON.stringify(result)}`).toBe(true);
+    expect(handed, 'the node reads through findOne exactly once').toHaveLength(1);
+    expect(handed[0].method).toBe('findOne');
+    return { result, fields: handed[0].fields };
+  }
+
+  it('control: the SQL driver returns exactly the columns a projection names, so a projection without id reads no id', async () => {
+    const direct: any = await ql.findOne(OBJECT.name, { where: { title: 'alpha' }, fields: ['title'], context: SYS });
+    expect(direct).toEqual({ title: 'alpha' });
+  });
+
+  it('fields that omit id: output.id is the row id, and record carries it beside the named columns', async () => {
+    const { result, fields } = await readOne({ fields: ['title'] });
+    expect(result.output.id).toBe(rowId);
+    expect(result.output.record).toEqual({ id: rowId, title: 'alpha' });
+    expect(fields).toEqual(['title', 'id']);
+  });
+
+  it('fields that name id: handed to the engine as written, with id once', async () => {
+    const { result, fields } = await readOne({ fields: ['id', 'title'] });
+    expect(fields).toEqual(['id', 'title']);
+    expect(result.output.id).toBe(rowId);
+    expect(result.output.record).toEqual({ id: rowId, title: 'alpha' });
+  });
+
+  for (const [label, config] of [['no fields', {}], ['empty fields', { fields: [] }]] as const) {
+    it(`control, ${label}: the whole row is read and the output is the engine's own answer`, async () => {
+      const whole: any = await ql.findOne(OBJECT.name, { where: { title: 'alpha' }, context: SYS });
+      expect(whole.status, 'control: a whole-row read carries the unnamed columns').toBe('open');
+      const { result, fields } = await readOne(config);
+      expect(fields, 'no projection is invented').toEqual((config as { fields?: unknown }).fields);
+      expect(result.output).toEqual({ record: whole, id: rowId, object: OBJECT.name });
+    });
+  }
+
+  it('a downstream update_record filtering on {read.id} updates the row the narrowed get_record read', async () => {
+    await ql.insert(OBJECT.name, { title: 'beta', status: 'open' }, { context: SYS });
+    const def = {
+      name: 'pin_get_record_id_flow', label: 'pin_get_record_id_flow', type: 'autolaunched', runAs: 'system',
+      variables: [{ name: 'rec', type: 'object', isOutput: true }],
+      nodes: [
+        { id: 'start', type: 'start', label: 'Start' },
+        { id: 'read', type: 'get_record', label: 'Read', config: { objectName: OBJECT.name, filter: { title: 'beta' }, fields: ['title'], outputVariable: 'rec' } },
+        { id: 'close', type: 'update_record', label: 'Close', config: { objectName: OBJECT.name, filter: { id: '{read.id}' }, fields: { status: 'closed' } } },
+        { id: 'end', type: 'end', label: 'End' },
+      ],
+      edges: [
+        { id: 'e1', source: 'start', target: 'read' },
+        { id: 'e2', source: 'read', target: 'close' },
+        { id: 'e3', source: 'close', target: 'end' },
+      ],
+    };
+    automation.registerFlow(def.name, def as any);
+    const res: any = await automation.execute(def.name, {});
+    expect(res.success, `run failed: ${JSON.stringify(res.error ?? res)}`).toBe(true);
+    const beta: any = await ql.findOne(OBJECT.name, { where: { title: 'beta' }, context: SYS });
+    expect(beta.status).toBe('closed');
+    expect(res.output.rec).toEqual({ id: beta.id, title: 'beta' });
+    const alpha: any = await ql.findOne(OBJECT.name, { where: { title: 'alpha' }, context: SYS });
+    expect(alpha.status, 'the update reached only the row the node read').toBe('open');
+  });
+});

packages/services/service-automation/src/builtin/get-record-stored-metadata-family.integration.test.ts

@@ -235,11 +235,16 @@ describe('flow get_record serves the stored-metadata family the way the data doo
     expect(JSON.parse(output.metadata)).toEqual(JSON.parse(control.metadata));
   });
 
-  it('a projection naming the body without its type is served projected, with exactly the columns named', async () => {
+  // [#22344] The single-row branch names `id` in the projection it hands the
+  // engine (its `output.id` is read off the row), so the served row carries the
+  // columns named plus `id`; the `type` column the family's projection adds is
+  // still taken back off.
+  it('a projection naming the body without its type is served projected, with exactly the columns named and the row id', async () => {
     const control = await doorRow('sys_metadata');
     const { output } = await runFlow(familyReadFlow('fam_projection', 'sys_metadata', 'system', 'one', ['name', 'metadata', 'checksum']));
     expectServedLikeTheDoor(output, control, 'the run output');
-    expect(Object.keys(output).sort()).toEqual(['checksum', 'metadata', 'name']);
+    expect(Object.keys(output).sort()).toEqual(['checksum', 'id', 'metadata', 'name']);
+    expect(output.id).toBe('meta_pin_flow_ds');
     expect(JSON.parse(output.metadata)).toEqual(JSON.parse(control.metadata));
     expect(output.checksum).toBe(control.checksum);
   });