diff --git a/.changeset/action-session-positions-runtime-dual-emit.md b/.changeset/action-session-positions-runtime-dual-emit.md new file mode 100644 index 0000000000..be53e20f98 --- /dev/null +++ b/.changeset/action-session-positions-runtime-dual-emit.md @@ -0,0 +1,58 @@ +--- +"@objectstack/runtime": major +--- + +feat(runtime)!: action body `ctx.session` emits `positions` (canonical) alongside the deprecated `roles` (#5613) + +`buildActionSession()` — the one producer of the action-body `ctx.session` — now +emits the caller's position names under **both** `positions` (canonical) and +`roles` (deprecated alias), with the same array under both names. This is the +**runtime half** of #5613 phase 2 under the maintainer's contract-first ruling +("C skeleton + A semantics"); the spec half (#5779) declared the shape this now +produces, and phase 1 (#5697) declared the shape it produced before. + +**What was wrong.** The builder copied `ExecutionContext.positions` into a key +spelled `roles` — the one spelling ADR-0090 D3 bans — while its own docblock +claimed it "mirrors the hook `ctx.session` shape". That sentence stopped being +true at #5050, which retired `HookContext.session.roles` outright: a body author +met two different answers to one key name on one platform, and the comment +pointed at the wrong one. The key set reached no schema and no gate until #5697, +so nothing could see it drift. + +**Migration prescription — do this now.** + +- Read `ctx.session.positions`. It carries exactly the array `ctx.session.roles` + carried (`ExecutionContext.positions` — the rename is a rename, not a semantic + change), and it is the spelling the platform now uses everywhere: the + execution context, the sharing service, `ctx.user.positions`, and the hook + `ctx.session.positions` (#5605). +- `ctx.session.roles` still resolves for the length of the deprecation window + announced by the ADR-0087 semantic migration + `action-session-roles-to-positions`, and is then removed on the path + `session.tenantId` already walked (#3280 deprecated → #3290 removed in v11). A + body still reading it at that point sees `undefined` with nothing to catch the + change — which is why the read moves **inside** the window, not at its close. +- Do **not** migrate an access check by renaming it. `roles.includes('admin')` + rewritten as `positions.includes('admin')` migrates the defect: neither array + is an authorization input. Privilege is judged by the security service, which + evaluates capability grants, placements and the derived posture (ADR-0095). +- Presence semantics are unchanged: a context with no positions (or a non-array + `positions`) yields **neither** key — `'positions' in ctx.session` answers + `false` exactly when `'roles' in ctx.session` does — and a call with no + identity envelope still yields no session at all rather than `{}` (#3712). + +**Also breaking, for TypeScript consumers of the sandbox seam.** +`ScriptContext.session` (`@objectstack/runtime`, `sandbox/script-runner.ts`) was +`unknown` and is now the exported union `ScriptSession = ActionSession | +HookContext['session']` — the two declared producer shapes this one seam +actually carries. Code that read an arbitrary property off it must now +discriminate the body kind (or read one of the keys both shapes declare: +`userId`, `organizationId`, `positions`). It is deliberately **not** narrowed to +`ActionSession` alone: the seam really does carry hook sessions, and declaring +otherwise would re-create the "one key, two realities" defect this change +closes. + +The consistency between what the producer builds and what `ActionSessionSchema` +declares stays pinned in +`packages/runtime/src/action-session-shape-contract.test.ts`, and the observed +shape is verified through a real dispatch in `http-dispatcher.test.ts`. diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 4a72eb94dc..bcf6403978 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -739,38 +739,69 @@ export function enforceActionParams(deps: ActionExecutionDeps, } /** - * Build the action-body `ctx.session` from the request ExecutionContext, - * mirroring the hook `ctx.session` shape (#3280) so an action author reads - * the caller's active org under the SAME blessed name as a hook author. + * Build the action-body `ctx.session` from the request ExecutionContext. + * + * The shape this returns is DECLARED by {@link ActionSessionSchema} + * (`@objectstack/spec/ui`), for which this function is the ONLY producer — + * #5697 declared the shape as it stood, #5779 added the canonical `positions` + * key and demoted `roles` to a deprecated alias, and this function is the + * matching producer half (#5613). The consistency between what is declared and + * what is built is pinned in `action-session-shape-contract.test.ts`; that pin, + * not this comment, is what keeps the two from drifting. * * `organizationId` is the blessed name for the caller's active org — the * same value as the `organization_id` column and `current_user.organizationId` * (RLS). The deprecated `session.tenantId` alias (#3280) was removed in v11 * (#3290); the driver-layer `ExecutionContext.tenantId` it is sourced from is - * a distinct, configurable axis and stays. Returns undefined for a genuinely - * context-less / self-invoked call so a body can distinguish "no session" the - * same way hooks do. + * a distinct, configurable axis and stays. Returns `undefined` — never `{}` — + * for a genuinely context-less / self-invoked call, so a body can tell "no + * identity envelope at all" from "an anonymous caller" (#3712). + * + * ## `positions` is canonical, `roles` is its deprecated alias (#5613) + * + * Both keys carry the SAME array, `ExecutionContext.positions` — the ADR-0090 + * D3 vocabulary. `positions` is the spelling an action body should read. + * `roles` is emitted only for the length of the deprecation window announced by + * the ADR-0087 semantic migration `action-session-roles-to-positions`, and is + * then removed on the path `session.tenantId` already walked (#3280 deprecate → + * #3290 remove). Emitting both, with identical values, is precisely what makes + * the migration a change of key and nothing else — a body can be moved to + * `positions` and verified while the alias is still live. + * + * The conditional spread is unchanged, so absent still means the KEY IS ABSENT: + * a context with no positions (or a non-array `positions`) yields NEITHER key, + * and since a session is only built at all when the context carries a user or + * an org, neither key can ever appear alone. + * + * ⚠️ Under either spelling this array is NOT an authorization input. Privilege + * is judged by the security service — capability grants, placements, derived + * posture (ADR-0095) — never by a name-string comparison; rewriting + * `roles.includes('admin')` as `positions.includes('admin')` migrates the + * defect rather than the read. * - * [#5697] The shape this returns is now DECLARED — {@link ActionSessionSchema} - * in `@objectstack/spec/ui`, phase 1 of #5613's contract-first ruling — and the - * annotation here is that declaration, not a restatement of it. Nothing about - * what this builds changed; the consistency between the two is pinned in - * `action-session-shape-contract.test.ts`. + * ## NOT the hook `ctx.session` * - * ⚠️ Two sentences above are tracked as WRONG, deliberately left standing for - * #5613 phase 2 to correct together with the rename they belong to: "mirroring - * the hook `ctx.session` shape" has not been true since #5050 retired - * `HookContext.session.roles` (the hook session is a different key set from a - * different producer), and the `roles` key this builds carries `ec.positions`, - * i.e. the ADR-0090 D3 vocabulary under the spelling that ADR bans. Read the - * schema's docblock before relying on either. + * This docblock used to say it mirrors the hook `ctx.session` shape (#3280). + * That claim stopped being true at #5050, which retired + * `HookContext.session.roles` outright: the hook session is a DIFFERENT key set + * (`actor`, `accessToken`, `isSystem`, the skip flags) from a different producer + * (ObjectQL's `buildSession()`), so "same key, two realities" was exactly the + * hazard the sentence advertised as a convenience. The two surfaces do now + * agree on `positions` — same vocabulary, same "descriptive, never an + * authorization input" boundary (#5605 declared it hook-side) — which is the + * POINT of this rename, not a coincidence. */ export function buildActionSession(_deps: ActionExecutionDeps, ec: any): ActionSession | undefined { if (!ec || (ec.userId == null && ec.tenantId == null)) return undefined; return { ...(ec.userId != null ? { userId: String(ec.userId) } : {}), ...(ec.tenantId != null ? { organizationId: String(ec.tenantId) } : {}), - ...(Array.isArray(ec.positions) && ec.positions.length ? { roles: ec.positions } : {}), + // Dual-emitted for the #5613 deprecation window: `positions` canonical, + // `roles` the alias, ONE array under two names (same value, by + // construction — not two reads that could drift apart). + ...(Array.isArray(ec.positions) && ec.positions.length + ? { positions: ec.positions, roles: ec.positions } + : {}), }; } diff --git a/packages/runtime/src/action-session-shape-contract.test.ts b/packages/runtime/src/action-session-shape-contract.test.ts index 0e34b52fce..752caed44a 100644 --- a/packages/runtime/src/action-session-shape-contract.test.ts +++ b/packages/runtime/src/action-session-shape-contract.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#5697] The action-body `ctx.session` consistency pin — what + * [#5697 / #5613] The action-body `ctx.session` consistency pin — what * `buildActionSession()` BUILDS against what `ActionSessionSchema` * (`@objectstack/spec/ui`) DECLARES. * @@ -12,6 +12,15 @@ * runs the real producer — the `hook-input-shape-contract.test.ts` shape from * #5668, one surface over. * + * Phase 2 has now landed on both sides: the spec half (#5779) added the + * canonical `positions` key and demoted `roles` to a deprecated alias of it, + * and the runtime half (#5613) made the producer DUAL-EMIT them. So the key-set + * assertions below are no longer "what the builder happens to do" — they are + * the deprecation window itself, pinned: both keys, same array, for exactly as + * long as the ADR-0087 semantic migration `action-session-roles-to-positions` + * says. When that window closes, `roles` comes out of the expectations here in + * the same change that stops producing it. + * * It lives in `packages/runtime` for the same reason that one lives in * `packages/objectql`: the pin must EXECUTE the producer, and `packages/spec` * cannot import the runtime without inverting the dependency. @@ -37,16 +46,30 @@ const build = (ec: unknown) => buildActionSession(deps, ec as any); describe('#5697 — action `ctx.session` matches its declared contract', () => { it('builds exactly the declared keys, and the declaration covers all of them', () => { - const built = build({ userId: 'u_1', tenantId: 'org_acme', positions: ['sales_rep', 'org_admin'] }); + const positions = ['sales_rep', 'org_admin']; + const built = build({ userId: 'u_1', tenantId: 'org_acme', positions }); + + // FLIPPED by #5613's runtime half. Before it, the builder emitted only + // the deprecated `roles`; it now emits the canonical `positions` too, + // which is what opens the migration window the spec half (#5779) + // declared. Both keys, or this is not a window. + expect(Object.keys(built!).sort()).toEqual(['organizationId', 'positions', 'roles', 'userId']); - // `roles`, not `positions` — the deprecated spelling is what the - // builder emits today and what the contract therefore declares. This - // assertion is the one #5613 phase 2 flips. - expect(Object.keys(built!).sort()).toEqual(['organizationId', 'roles', 'userId']); + // The window's load-bearing property: SAME VALUE under both spellings, + // so migrating a body from `roles` to `positions` is a change of key + // and nothing else. Asserted against `ec.positions` on both sides + // rather than just key-to-key, so a builder that started deriving one + // of them from something else could not satisfy it. + expect((built as { positions?: string[] }).positions).toEqual(positions); + expect((built as { roles?: string[] }).roles).toEqual(positions); + expect((built as { roles?: string[] }).roles).toEqual((built as { positions?: string[] }).positions); // Non-strict parse: an UNDECLARED key would be silently stripped here, // so deep equality — not `.success` — is what proves the contract - // covers everything the producer emits. + // covers everything the producer emits. This is also what proves the + // spec half is actually in: before #5779 declared `positions`, a + // dual-emitting builder would fail HERE (key stripped) rather than on + // the key-set assertion above. expect(ActionSessionSchema.parse(built)).toEqual(built); expect(ActionSessionSchema.safeParse(built).success).toBe(true); }); @@ -66,15 +89,33 @@ describe('#5697 — action `ctx.session` matches its declared contract', () => { expect('tenantId' in built!).toBe(false); }); - it('carries `ec.positions` verbatim under the deprecated `roles` spelling', () => { + it('carries `ec.positions` verbatim under BOTH the canonical and the deprecated spelling', () => { const positions = ['sales_rep', 'org_admin']; const built = build({ userId: 'u_1', positions }); - // Phase 2 (#5613) renames the KEY; this pins that the VALUE is, and + // Phase 2 (#5613) renamed the KEY; this pins that the VALUE is, and // stays, the ADR-0090 D3 vocabulary — so the rename is a rename and // not a semantic change smuggled inside one. + expect((built as { positions?: string[] }).positions).toEqual(positions); expect((built as { roles?: string[] }).roles).toEqual(positions); expect(ActionSessionSchema.safeParse(built).success).toBe(true); }); + + it('emits the alias only alongside the canonical key — never `roles` on its own', () => { + // The direction that matters when the window CLOSES: the removal + // deletes `roles` from the producer and from the expectation above, + // and this assertion is what says the canonical key was never the + // thing that could go missing. A builder that regressed to alias-only + // would satisfy the value assertions and fail here. + for (const ec of [ + { userId: 'u_1', positions: ['org_admin'] }, + { tenantId: 'org_acme', positions: ['org_admin'] }, + { userId: 'u_1', tenantId: 'org_acme', positions: ['org_admin'] }, + ]) { + const keys = Object.keys(build(ec)!); + expect(keys).toContain('positions'); + expect(keys).toContain('roles'); + } + }); }); describe('#5697 — conditional-spread semantics: absent means the KEY is absent', () => { @@ -94,15 +135,22 @@ describe('#5697 — conditional-spread semantics: absent means the KEY is absent it('omits `userId` entirely for an org-scoped call with no user', () => { const built = build({ tenantId: 'org_acme', positions: ['org_admin'] }); expect('userId' in built!).toBe(false); - expect(Object.keys(built!).sort()).toEqual(['organizationId', 'roles']); + expect(Object.keys(built!).sort()).toEqual(['organizationId', 'positions', 'roles']); }); - it('omits `roles` for an empty or absent positions array', () => { + it('omits BOTH position spellings for an empty or absent positions array', () => { + // The dual emission is one conditional spread, so the "non-empty only" + // semantics is identical for the canonical key and the alias — neither + // appears as an empty array, and `'positions' in ctx.session` answers + // false exactly when `'roles' in ctx.session` does. expect(Object.keys(build({ userId: 'u_1', positions: [] })!)).toEqual(['userId']); expect(Object.keys(build({ userId: 'u_1' })!)).toEqual(['userId']); // A non-array `positions` is ignored rather than passed through — the // declared `string[]` would otherwise be a lie the parse catches. expect(Object.keys(build({ userId: 'u_1', positions: 'org_admin' })!)).toEqual(['userId']); + for (const ec of [{ userId: 'u_1', positions: [] }, { userId: 'u_1' }, { userId: 'u_1', positions: 'org_admin' }]) { + expect(ActionSessionSchema.parse(build(ec))).toEqual(build(ec)); + } }); }); @@ -114,9 +162,9 @@ describe('#5697 — no identity envelope yields NO session, never an empty one', ])('%s → undefined', (_label, ec) => { // #3712's distinction, on the action side: a body can tell "no identity // envelope at all" from "an anonymous caller" only because this is - // `undefined` rather than `{}`. The third row is why `roles` can never - // appear alone — positions without a user and without an org produce no - // session for it to appear on. + // `undefined` rather than `{}`. The third row is why neither position + // spelling can ever appear alone — positions without a user and without + // an org produce no session for them to appear on. expect(build(ec)).toBeUndefined(); }); }); diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 38d4679d4e..7378be410c 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -3643,6 +3643,28 @@ describe('HttpDispatcher — action body ctx.user identity (#2701)', () => { expect(session.tenantId).toBeUndefined(); }); + it('hands the body its positions under the canonical `positions`, with `roles` as the equal deprecated alias (#5613)', async () => { + // Verified through a REAL dispatch rather than by calling the builder — the + // ADR-0087 semantic migration `action-session-roles-to-positions` asks for + // exactly this shape of evidence: invoke an action as a caller holding + // positions, then assert what the BODY observed. + const { dispatcher, executeAction, ctx } = captureCtx({ + userId: 'user_42', + positions: ['sales_rep', 'org_admin'], + tenantId: 'org_acme', + }); + await dispatcher.handleActions('/lead/convert', 'POST', {}, ctx); + const session = actionSession(executeAction); + // The canonical key an action body should read (ADR-0090 D3 vocabulary). + expect(session.positions).toEqual(['sales_rep', 'org_admin']); + // The alias, dual-emitted with the SAME value for the length of the + // deprecation window — which is what makes migrating a body a change of + // key and nothing else. This assertion is removed by the change that + // stops producing `roles`, not before. + expect(session.roles).toEqual(session.positions); + expect(Object.keys(session).sort()).toEqual(['organizationId', 'positions', 'roles', 'userId']); + }); + it('falls back to a `system` principal for a SELF-INVOKED call — the anonymous door is 401 now (#5519)', async () => { // REPLACED, not re-spelled. This was driven with NO execution context — // the shape an anonymous HTTP request has — and asserted over the action diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index 3daf787d6f..b69c8a249e 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -146,6 +146,7 @@ export type { ScriptOrigin, ScriptResult, ScriptRunOptions, + ScriptSession, QuickJSScriptRunnerOptions, } from './sandbox/index.js'; diff --git a/packages/runtime/src/sandbox/index.ts b/packages/runtime/src/sandbox/index.ts index 596bf60d58..05a68bde8a 100644 --- a/packages/runtime/src/sandbox/index.ts +++ b/packages/runtime/src/sandbox/index.ts @@ -9,6 +9,7 @@ export type { ScriptOrigin, ScriptResult, ScriptRunOptions, + ScriptSession, } from './script-runner.js'; export { QuickJSScriptRunner, SandboxError } from './quickjs-runner.js'; export type { QuickJSScriptRunnerOptions } from './quickjs-runner.js'; diff --git a/packages/runtime/src/sandbox/script-runner.ts b/packages/runtime/src/sandbox/script-runner.ts index c133735a0f..5fbf969240 100644 --- a/packages/runtime/src/sandbox/script-runner.ts +++ b/packages/runtime/src/sandbox/script-runner.ts @@ -35,7 +35,23 @@ * touching call sites. */ -import type { HookBody, ScriptBody, ExpressionBody } from '@objectstack/spec/data'; +import type { HookBody, ScriptBody, ExpressionBody, HookContext } from '@objectstack/spec/data'; +import type { ActionSession } from '@objectstack/spec/ui'; + +/** + * The caller session a sandboxed body receives on `ctx.session` — the union of + * the two DECLARED producer shapes this one seam carries (#5613). + * + * It is a union rather than a single type because the seam is genuinely + * generic over both body kinds, and collapsing it to either one would be a + * contract lie in the other direction: a hook body's session is not an + * {@link ActionSession}, and an action body's is not a `HookContext['session']`. + * The two agree on exactly the keys they are documented to agree on — `userId`, + * `organizationId` and, since #5605 hook-side plus #5613 action-side, + * `positions` — so a consumer that reads only those needs no discrimination, + * which is the practical payoff of the rename. + */ +export type ScriptSession = ActionSession | HookContext['session']; /** * Identity / origin information used by the sandbox for diagnostics, capability @@ -73,24 +89,34 @@ export interface ScriptContext { * this interface is a single generic seam over both body kinds: * * - a HOOK body gets `HookContext.session` (`@objectstack/spec/data`) — - * `userId` / `actor` / `organizationId` / `accessToken` / `isSystem` / - * the skip flags, built by ObjectQL's `buildSession()`; - * - an ACTION body gets `ActionSession` (`@objectstack/spec/ui`, - * {@link ActionSessionSchema}) — `userId` / `organizationId` / `roles`, - * built by `buildActionSession()`. + * `userId` / `actor` / `organizationId` / `positions` / `accessToken` / + * `isSystem` / the skip flags, built by ObjectQL's `buildSession()`; + * - an ACTION body gets `ActionSession` (`@objectstack/spec/ui`) — + * `userId` / `organizationId` / `positions` and, for the length of the + * #5613 deprecation window, its deprecated alias `roles`; built by + * `buildActionSession()` (`../action-execution.ts`). * - * They are NOT the same object and never converge: `roles` exists on the - * action side only (and is deprecated there — #5613 phase 2 renames it to - * `positions`), while the hook side retired that key at #5050. + * They are NOT the same object and do not converge into one: `roles` exists + * on the action side only (deprecated there, removed hook-side at #5050), + * and the hook side carries an `actor` / skip-flag vocabulary the action side + * has no counterpart for. What they DO now share is the position vocabulary + * under one spelling — `positions` on both (#5605 hook-side, #5613 + * action-side) — which is the point of #5613's rename. * - * Left `unknown` rather than typed as the union on purpose (#5697, which is - * a zero-behaviour-change declaration): narrowing this field would force - * every consumer of the seam — `quickjs-runner`'s `installCtx`, the body - * runners, the hook path's own producers — to discriminate a body kind this - * type does not carry. Typing it belongs with whatever change is willing to - * pay that, not with declaring what the producers already build. + * Typed as {@link ScriptSession}, the union of those two DECLARED shapes, + * since #5613. It was `unknown` while #5697 declared the action half without + * changing behaviour; the cost that deferral named — that narrowing forces + * seam consumers to discriminate a body kind this type does not carry — was + * MEASURED here rather than re-assumed: the two writers (`body-runner`'s + * `buildSandboxContext` / `buildActionSandboxContext`) assign from an `any` + * engine context, and the only reader (`quickjs-runner`'s `installCtx`, via + * `setObjectJson`) takes `unknown`. So no site discriminates today, and the + * union is what makes a future site that needs to. It is deliberately NOT + * narrowed to `ActionSession` alone: this seam really does carry hook + * sessions, and declaring otherwise would be the same + * "one key, two realities" defect #5613 exists to close. */ - session?: unknown; + session?: ScriptSession; /** * The lifecycle event name the hook is firing for (e.g. `beforeInsert`, * `afterUpdate`). Required for hooks that subscribe to multiple events diff --git a/skills/objectstack-ui/SKILL.md b/skills/objectstack-ui/SKILL.md index 4c8a3c4892..a6f85668f4 100644 --- a/skills/objectstack-ui/SKILL.md +++ b/skills/objectstack-ui/SKILL.md @@ -1822,6 +1822,25 @@ RLS/FLS), so a body that must scope by org reads it from `ctx` explicitly. isolation axes as hooks — `organization_id` row-scoping vs environment / database-per-tenant; see the objectstack-data hooks reference.) +The caller's position names are on `ctx.session.positions` — the ADR-0090 D3 +spelling, the same one the hook `ctx.session`, `ctx.user.positions` and the +sharing service use: + +```typescript +// ✅ Canonical since #5613 +const positions = ctx.session?.positions ?? []; +``` + +> The key is **absent** (not empty) when the caller holds no positions, and the +> whole `ctx.session` is `undefined` for a call with no identity envelope. The +> pre-ADR-0090 alias of this same array is still emitted for one migration +> window and then removed — see `action-session-*-to-positions` in the protocol +> upgrade guide for the prescription. Migrate the READ to `positions`; do not +> migrate an access check by renaming it. **This array is not an authorization +> input**: `positions.includes('admin')` is the same defect under a blessed +> name. Ask the security service for privilege (capability grants, placements, +> derived posture — ADR-0095). + ### Opening in a New Tab (`openIn` / `opensInNewTab` / `newTabUrl`) There are **two** mechanisms here. Pick by whether the URL is static or computed: