refactor(runtime): 删除沙箱 ctx.user 的 ?? session?.user 死支 (#6316)

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<string, unknown>) => {
+      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<string, unknown>, ...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<string, unknown>) =>
+      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');
+    });
+  });
 });

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,

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