fix(approvals): a schedule-triggered run can write its own locked record (#3712)

.changeset/approval-dead-run-record-lock.md

@@ -20,8 +20,8 @@ Both halves are now closed.
 
 **Prevention — the owning run may write its own record.** The automation engine
 stamps `flowRunId` onto the run context at setup, alongside `runAs`, and it
-travels with every data node's ObjectQL context into `ctx.session`. The lock hook
-exempts a write whose `flowRunId` matches the pending request's `flow_run_id`.
+travels with every data node's ObjectQL context into `ctx.provenance`. The lock
+hook exempts a write whose `flowRunId` matches the pending request's `flow_run_id`.
 It is keyed on run identity rather than elevation on purpose: a `runAs:'user'`
 run stays fully RLS-scoped while it writes. `flowRunId` is pure provenance —
 server-constructed like `isSystem`, never client-supplied, evaluated by no
@@ -49,9 +49,9 @@ entries under one id, so every suspend-then-finish run — every approval, scree
 and wait flow — reported itself as `paused` forever, both on the Runs
 observability surface and to this sweep.
 
-Residual, deliberately not changed here: a `runAs:'user'` run with no trigger
-user (a schedule) passes no ObjectQL context at all, so it carries no
-`flowRunId` and is still subject to the lock. Manufacturing a context just to
-carry the run id would flip that run from its documented unscoped fail-open
-(#1888) to baseline-member RLS — a separate, larger change. The sweep is what
-recovers that shape.
+One shape was left out here and closed separately in #3712: a `runAs:'user'` run
+with no trigger user (a schedule) resolved no ObjectQL context at all, so it
+carried no `flowRunId` and stayed subject to the lock. It now passes a
+provenance-only context — the run id and nothing the security middleware keys on
+— so it is attributable without acquiring a principal, and its documented
+unscoped posture (#1888) is unchanged.

.changeset/approval-lock-schedule-run-provenance.md

@@ -0,0 +1,50 @@
+---
+"@objectstack/service-automation": patch
+"@objectstack/plugin-approvals": patch
+"@objectstack/objectql": patch
+"@objectstack/spec": patch
+---
+
+fix(approvals): a schedule-triggered run can write its own locked record (#3712)
+
+#3456 let the run that opened a pending approval write its own target record,
+keyed on `flowRunId`. It worked for every run that resolves an identity and
+missed the one that doesn't: an effective `runAs:'user'` run with **no trigger
+user** — a schedule being the canonical case — passed no ObjectQL context at
+all, so nothing carried the run id and the run still died on its own
+`RECORD_LOCKED`.
+
+The blocker was never the lock. It was that "no identity" and "no context" were
+the same thing on the wire, so a run could not say *who it was* without also
+claiming *what it was allowed to do*.
+
+**A run with no principal now passes provenance alone.**
+`resolveRunDataContext` returns `{ flowRunId }` — no `userId`, no `positions`,
+no `permissions`, not even `isSystem: false`. Every principal gate keys on one
+of those fields (the elevation short-circuit on `isSystem`, the ADR-0103
+engine-owned write guard and the ADR-0090 D12 delegated-admin gate on `userId`,
+the empty-principal fall-open on all three), so this context authorizes
+**identically to no context at all**. The run keeps the documented #1888
+unscoped posture, its loud `[runAs]` warning, and the
+`flow-schedule-runas-unscoped` build-time lint. Nothing about what it may touch
+changed — only that it can now be attributed.
+
+**Provenance moved out of the hook session, into `ctx.provenance`.** `session`
+answers *who is calling* and is absent when no identity envelope was supplied —
+a distinction real gates depend on (the attachment access gate skips bare-kernel
+writes on exactly that test). Folding a run id into `session` would have forced
+an identity-less run to present an empty session, silently turning "no caller"
+into "an anonymous caller" and narrowing the #1888 fail-open for attachments
+alone. `HookContext.provenance.flowRunId` says what produced the write; the
+approvals lock reads it there.
+
+Also relaxes `BaseEngineOptionsSchema.context` to a partial envelope
+(`ExecutionContextInput`). `positions`/`permissions`/`isSystem` carry parse-time
+defaults, which made them *required* on a caller-supplied option and asserted
+something untrue — that every data-engine context carries a principal. Callers
+have always passed slices (`{ isSystem: true }` for a system read); the type now
+says so.
+
+Migration: nothing to change unless you read the run id inside a hook. If you
+wrote `ctx.session.flowRunId`, read `ctx.provenance.flowRunId` instead — the
+field never shipped under the old name.

content/docs/kernel/contracts/data-engine.mdx

@@ -94,10 +94,15 @@ interface EngineQueryOptions {
   search?: FullTextSearch;                   // Full-text search
   expand?: Record<string, QueryAST>;         // Recursive relation loading
   distinct?: boolean;                        // SELECT DISTINCT
-  context?: ExecutionContext;                 // Identity, tenant, transaction
+  context?: ExecutionContextInput;            // Identity, tenant, transaction — any subset
 }
 ```
 
+`ExecutionContextInput` is `ExecutionContext` with every field optional — supply
+what you have, the engine reads what it needs. A system read passes
+`{ isSystem: true }`; an automation run with no resolvable identity passes only
+its run id (`{ flowRunId }`), a context that deliberately carries no principal.
+
 ### FilterCondition (where)
 
 Filters use the canonical **`where` + MongoDB-style `$op` object syntax** from `FilterConditionSchema`:
@@ -191,7 +196,7 @@ const tasks = await engine.insert('task', [
 ```typescript
 interface DataEngineInsertOptions {
   returning?: boolean;          // Return inserted record(s)? Default: true
-  context?: ExecutionContext;    // Identity, tenant, transaction
+  context?: ExecutionContextInput;  // Identity, tenant, transaction — any subset
 }
 ```
 
@@ -218,7 +223,7 @@ interface EngineUpdateOptions {
   upsert?: boolean;             // Insert if not found? Default: false
   multi?: boolean;              // Update multiple records? Default: false
   returning?: boolean;          // Return updated record(s)? Default: false
-  context?: ExecutionContext;
+  context?: ExecutionContextInput;
 }
 ```
 
@@ -268,7 +273,7 @@ await engine.delete('task', {
 interface EngineDeleteOptions {
   where?: FilterCondition;      // Filter to identify records (WHERE)
   multi?: boolean;              // Delete multiple records? Default: false
-  context?: ExecutionContext;
+  context?: ExecutionContextInput;
 }
 ```
 
@@ -306,7 +311,7 @@ interface EngineAggregateOptions {
   where?: FilterCondition;                 // Pre-aggregation filter (WHERE)
   groupBy?: string[];                      // GROUP BY fields
   aggregations?: AggregationNode[];        // Aggregation definitions
-  context?: ExecutionContext;
+  context?: ExecutionContextInput;
 }
 
 interface AggregationNode {

content/docs/references/data/hook.mdx

@@ -38,6 +38,7 @@ const result = HookContext.parse(data);
 | **result** | `any` | optional | Operation result (After hooks only) |
 | **previous** | `Record<string, any>` | optional | Record state before operation |
 | **session** | `{ userId?: string; organizationId?: string; roles?: string[]; accessToken?: string; … }` | optional | Current session context |
+| **provenance** | `{ flowRunId?: string }` | optional | Server-stamped write provenance (never client-supplied, never an authorization input) |
 | **transaction** | `any` | optional | Database transaction handle |
 | **ql** | `any` | ✅ | ObjectQL Engine Reference |
 | **api** | `any` | optional | Cross-object data access (ScopedContext) |

packages/objectql/src/engine.test.ts

@@ -527,6 +527,77 @@ describe('ObjectQL Engine', () => {
         });
     });
 
+    /**
+     * #3712 — write PROVENANCE (what produced the write) is surfaced separately
+     * from the SESSION (who is calling), because a writer can have the first and
+     * none of the second: a schedule-triggered flow run resolves no principal but
+     * still owns its writes. Folding the two together would have forced such a run
+     * to present an empty session, silently converting "no caller" into "an
+     * anonymous caller" for every hook that skips when `ctx.session` is absent.
+     */
+    describe('hook provenance is separate from the hook session (#3712)', () => {
+        beforeEach(async () => {
+            engine.registerDriver(mockDriver, true);
+            await engine.init();
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} } as any);
+        });
+
+        const capture = () => {
+            const seen: { session?: any; provenance?: any } = {};
+            engine.registerHook('beforeInsert', async (ctx: any) => {
+                seen.session = ctx.session;
+                seen.provenance = ctx.provenance;
+            }, { object: 'task' });
+            return seen;
+        };
+
+        it('a provenance-only context yields provenance and NO session', async () => {
+            const seen = capture();
+
+            await engine.insert('task', { title: 'x' }, { context: { flowRunId: 'run_1' } as any });
+
+            expect(seen.provenance).toEqual({ flowRunId: 'run_1' });
+            // The whole point: `!ctx.session` still means "no identity envelope
+            // was supplied", so caller-gating hooks keep skipping for this write.
+            expect(seen.session).toBeUndefined();
+        });
+
+        it('an identity-bearing run carries both', async () => {
+            const seen = capture();
+
+            await engine.insert('task', { title: 'y' }, { context: { userId: 'u1', flowRunId: 'run_2' } as any });
+
+            expect(seen.provenance).toEqual({ flowRunId: 'run_2' });
+            expect(seen.session).toMatchObject({ userId: 'u1' });
+            // Provenance is NOT identity — it must not reappear inside the session.
+            expect(seen.session).not.toHaveProperty('flowRunId');
+        });
+
+        it('no context at all yields neither', async () => {
+            const seen = capture();
+
+            await engine.insert('task', { title: 'z' });
+
+            expect(seen.provenance).toBeUndefined();
+            expect(seen.session).toBeUndefined();
+        });
+
+        it('an anonymous transport request still yields a session (it has a caller)', async () => {
+            // The shape resolveExecutionContext builds for a signed-out HTTP
+            // request: no userId, but a resolved (empty) principal. That is an
+            // anonymous CALLER, not the absence of one — hooks must keep gating it.
+            const seen = capture();
+
+            await engine.insert('task', { title: 'w' }, {
+                context: { isSystem: false, positions: [], permissions: [] } as any,
+            });
+
+            expect(seen.session).toBeDefined();
+            expect(seen.session.userId).toBeUndefined();
+            expect(seen.provenance).toBeUndefined();
+        });
+    });
+
     describe('execution context via the trailing options arg (read methods)', () => {
         // Regression: reads took context inside the query while writes took it in
         // a trailing options arg — so `find(obj, q, { context })` silently dropped