From 23253eb06d736a46a94f320d727d00c9e38b86f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 14:15:59 +0000 Subject: [PATCH 1/2] =?UTF-8?q?refactor(runtime):=20=E5=88=A0=E9=99=A4?= =?UTF-8?q?=E6=B2=99=E7=AE=B1=E4=B8=A4=E4=B8=AA=E5=86=99=E5=85=A5=E6=96=B9?= =?UTF-8?q?=E7=9A=84=20`=3F=3F=20session=3F.user`=20=E6=AD=BB=E6=94=AF=20(?= =?UTF-8?q?#6316)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/runtime/src/sandbox/body-runner.ts | 26 +++++++++++++++++-- packages/runtime/src/sandbox/script-runner.ts | 21 ++++++++------- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/packages/runtime/src/sandbox/body-runner.ts b/packages/runtime/src/sandbox/body-runner.ts index 3b8be04276..2a6e7bf01b 100644 --- a/packages/runtime/src/sandbox/body-runner.ts +++ b/packages/runtime/src/sandbox/body-runner.ts @@ -318,7 +318,19 @@ function buildSandboxContext(engineCtx: any, ql: any): ScriptContext { // Preserve `undefined` for `previous` on insert events so hooks can // reliably distinguish create (`!ctx.previous`) from update/delete. previous: unwrapProxyToPlain(previousRaw), - user: engineCtx?.user ?? engineCtx?.session?.user, + // `engineCtx.user` is the ONLY source, and the `?? engineCtx?.session?.user` + // limb that used to follow it was removed in #6316 (same family as #5906 + // above, and as #4984): `HookContext['session']` declares no `user` key + // (`packages/spec/src/data/hook.zod.ts`) and its sole producer — + // ObjectQL's `buildSession()` (`packages/objectql/src/engine.ts`), which + // every HookContext assembly site in the engine calls — builds the session + // field by field and writes none. The limb resolved `undefined` on every + // real path, so deleting it changes no behaviour; what it changed was the + // reading, which advertised a second data source that has never existed. + // Keep it deleted: a `session.user` that some future engine "might" set is + // a contract to DECLARE on `HookContextSchema.session`, not to anticipate + // with consumer-side tolerance here (PD #12). + user: engineCtx?.user, session: engineCtx?.session, event: typeof engineCtx?.event === 'string' ? engineCtx.event : undefined, object: typeof engineCtx?.object === 'string' ? engineCtx.object : undefined, @@ -343,7 +355,17 @@ function buildActionSandboxContext(actionCtx: any, ql: any): ScriptContext { return { input: unwrapProxyToPlain(actionCtx?.params ?? {}), previous: undefined, - user: actionCtx?.user ?? actionCtx?.session?.user, + // Same removal as the hook face above (#6316), measured on this face's own + // shapes: `ActionSession` (`packages/spec/src/ui/action-params.zod.ts`) + // declares `userId` / `organizationId` / `positions` / `roles` and no + // `user`, and its sole producer `buildActionSession()` + // (`../action-execution.ts`) writes exactly those four — for both action ctx + // assembly sites (`action-execution.ts` MCP `run_action`, `domains/actions.ts` + // REST `/actions`). The third `executeAction` caller, ObjectQL's + // `ScopedRepo.execute()`, passes neither `user` nor `session`, so `ctx.user` + // stays `undefined` there — unchanged by this removal, and the correct + // semantics: that path carries no caller identity. + user: actionCtx?.user, session: actionCtx?.session, object: typeof actionCtx?.object === 'string' ? actionCtx.object : undefined, recordId, diff --git a/packages/runtime/src/sandbox/script-runner.ts b/packages/runtime/src/sandbox/script-runner.ts index 7569888719..f0cd3bf0dd 100644 --- a/packages/runtime/src/sandbox/script-runner.ts +++ b/packages/runtime/src/sandbox/script-runner.ts @@ -89,15 +89,18 @@ export type ScriptSession = ActionSession | HookContext['session']; * same over-claim in a spec-shaped disguise. `ActorUser extends EvalUser`, so * the action arm still carries the ADR-0068 contract on the path that has it. * - * The `?? …session?.user` fallback chain both writers carry (`body-runner.ts` - * `:315` / `:340`) forces no THIRD arm — measured, not presumed: neither - * session shape reaching this seam declares a `user` key - * (`HookContext['session']`, `ActionSession`) and neither producer writes one - * (`buildSession()` in objectql, `buildActionSession()` in - * `../action-execution.ts`), so that arm is unreachable on every real path — - * the #4984 dead-limb family. It is left in place here because this change - * types a seam and does not get to re-decide a runtime expression; the limb is - * filed separately. + * No THIRD arm for a session-carried user, and there is no longer a runtime + * expression suggesting one. Both writers in `body-runner.ts` used to spell + * `?? …session?.user`; #5521 measured that limb unreachable — neither session + * shape reaching this seam declares a `user` key (`HookContext['session']`, + * `ActionSession`) and neither producer writes one (`buildSession()` in + * objectql, `buildActionSession()` in `../action-execution.ts`) — and left it + * alone, because typing a seam does not get to re-decide a runtime expression. + * #6316 re-ran that sweep across every producer on both faces, confirmed it, + * and deleted both limbs (the #4984 dead-limb family). So the union's arms are + * the two REAL producer shapes and nothing else; if a session ever should + * carry a user, DECLARE it on the session contract rather than restoring a + * consumer-side `??` here (PD #12). * * `undefined` is a member (via `HookContext['user']`'s own optionality, exactly * as in {@link ScriptSession}) and it is a REAL value on this seam, not just From 7ad7eb38e4fa32fb68349b042e2d4b946c7a107d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 17:25:54 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(runtime):=20=E9=92=89=E4=BD=8F=20ctx.u?= =?UTF-8?q?ser=20=E5=8F=AA=E4=BB=8E=20engine/action=20ctx=20=E7=9A=84=20us?= =?UTF-8?q?er=20=E9=94=AE=E5=8F=96=E5=80=BC=20(#6316)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../runtime/src/sandbox/body-runner.test.ts | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/packages/runtime/src/sandbox/body-runner.test.ts b/packages/runtime/src/sandbox/body-runner.test.ts index 7fd077b1b0..2819c60f69 100644 --- a/packages/runtime/src/sandbox/body-runner.test.ts +++ b/packages/runtime/src/sandbox/body-runner.test.ts @@ -202,6 +202,60 @@ describe('hookBodyRunnerFactory', () => { expect(seen[0]).toEqual({ input: '{}', previous: 'null' }); }); }); + + // [#6316] `ctx.user` is seeded from `engineCtx.user` and from nothing else. + // `buildSandboxContext` used to spell `engineCtx?.user ?? engineCtx?.session?.user`; + // the second limb was unreachable, because `HookContext['session']` declares + // no `user` key and its sole producer — `buildSession()` in objectql, called + // by every HookContext assembly site in the engine — writes none. + // + // ⚠️ Read these two cases for what they each pin. The POSITIVE one is not a + // regression guard for this change: the truth key sat FIRST in the old chain, + // so it was green before the deletion and is green after — deleting an + // unreachable limb cannot move it, by construction. The NEGATIVE one carries + // all the weight, and it is the one that goes RED if the limb is restored. + // Its context is deliberately SYNTHETIC — no producer can build a session + // carrying `user`, which is the whole finding — so it pins the RULE + // ("`session.user` is not a data source") rather than any behaviour a real + // path exhibits. That is the point: the rule is what a future edit would + // break, and types cannot catch it here (both writers take `any`). + describe('seeds ctx.user from the engine key only', () => { + const probeUser = (engineCtx: Record) => { + const fn = hookBodyRunnerFactory(runner, { ql: {}, appId: 'crm' })({ + name: 'probe_user', + object: 'contact', + events: ['beforeInsert'], + body: { + language: 'js', + source: 'return { seen: JSON.stringify(ctx.user ?? null) };', + capabilities: [], + }, + } as any); + const ctx = { input: {} as Record, ...engineCtx } as any; + return fn!(ctx).then(() => ctx.input.seen as string); + }; + + it('reads `user` — the key `buildUser()` produces on every hook dispatch', async () => { + expect(await probeUser({ user: { id: 'u_1', name: 'Ada' } })).toBe( + '{"id":"u_1","name":"Ada"}', + ); + }); + + it('does NOT fall back to `session.user` — no producer writes that key', async () => { + // A session shape no producer can build, planted so the removed limb + // would have something to find. With the limb gone the body sees no user + // at all; `session.userId` is the spelling that carries the caller here. + expect( + await probeUser({ session: { userId: 'u_1', user: { id: 'u_1', name: 'Ada' } } }), + ).toBe('null'); + }); + + it('leaves ctx.user undefined when the context carries neither key', async () => { + // ObjectQL's `ScopedRepo.execute()` is the real shape of this case on the + // action face; on the hook face it is a context-less programmatic call. + expect(await probeUser({})).toBe('null'); + }); + }); }); describe('actionBodyRunnerFactory', () => { @@ -388,4 +442,56 @@ describe('actionBodyRunnerFactory', () => { expect(updates).toEqual([{ id: 'deal_1', stage: 'won' }]); }); }); + + // [#6316] The action face of the same removal. `ActionSession` declares + // `userId` / `organizationId` / `positions` / `roles` and no `user`, and its + // sole producer `buildActionSession()` writes exactly those four — for both + // action ctx assembly sites (MCP `run_action` in `action-execution.ts`, REST + // `/actions` in `domains/actions.ts`). As on the hook face above, the + // negative case is the one that goes red if `?? actionCtx?.session?.user` is + // restored; the positive case cannot move either way. + describe('seeds ctx.user from the action-context key only', () => { + const probeUser = (actionCtx: Record) => + actionBodyRunnerFactory(runner, { ql: {}, appId: 'crm' })({ + name: 'whoami', + object: 'crm_deal', + body: { + language: 'js', + source: 'return JSON.stringify(ctx.user ?? null);', + capabilities: [], + }, + })!(actionCtx); + + it('reads `user` — the ActorUser every dispatch site builds', async () => { + expect(await probeUser({ user: { id: 'u_1', displayName: 'Ada' } })).toBe( + '{"id":"u_1","displayName":"Ada"}', + ); + }); + + it('does NOT fall back to `session.user` — ActionSession has no such key', async () => { + // The four keys `buildActionSession()` really writes, plus a planted + // `user` the removed limb would have read. Only the planted one is + // ignored; a body that needs the caller reads `ctx.session.userId`. + expect( + await probeUser({ + session: { + userId: 'u_1', + organizationId: 'org_1', + positions: ['sales'], + roles: ['sales'], + user: { id: 'u_1', displayName: 'Ada' }, + }, + }), + ).toBe('null'); + }); + + it("leaves ctx.user undefined on ObjectQL's `ScopedRepo.execute()` shape", async () => { + // The engine's `repo.execute(name, params)` reaches `executeAction` with + // `{ ...params, userId, tenantId, roles }` — neither `user` nor `session`. + // Both limbs missed it before this change and the surviving one misses it + // now: `ctx.user` is `undefined` either way, which is the correct + // semantics (that path carries no caller identity). + expect(await probeUser({ userId: 'u_1', tenantId: 'org_1', roles: ['sales'] })).toBe('null'); + }); + }); });