feat(spec): HookContext.session 补声明 positions / preserveAudit(#5605)

.changeset/hook-context-session-positions-preserve-audit.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/spec": minor
+---
+
+feat(spec): `HookContext.session` declares `positions` and `preserveAudit` (#5605)
+
+Two keys the engine has been **producing** all along, that consumers have been
+**reading** and the docs have been **teaching**, were missing from the contract:
+`HookContextSchema.session` declared `userId` / `actor` / `organizationId` /
+`accessToken` / `isSystem` / `skipTriggers` / `skipAutomations` and nothing else.
+Both are now declared, per the maintainer ruling on #5605.
+
+This is the mirror of the `session.roles` retirement (#5050). That key was
+declared-never-produced, so it was removed; these two are
+produced-never-declared, so they are added. Same `session` block, opposite
+drift, opposite fix.
+
+**What was broken.** `HookContextSchema` is deliberately not `.strict()` — it is
+the runtime shape the engine hands a handler, and strictness there would make
+every engine-side enrichment a breaking change for anyone parsing a context they
+were given. The cost of that tolerance is that an undeclared key is **stripped
+in silence**:
+
+- `HookContextSchema.parse(ctx)` — the exact call the generated reference page
+  documents as the way to consume a context — returned a session with the
+  caller's `positions` and the import's `preserveAudit` dropped on the floor.
+- A handler typed the way the automation docs teach, `(ctx: HookContext)`, could
+  not read either key: `ctx.session?.positions` was a `TS2339`. The two
+  `kernel/runtime-services` pages that teach
+  `positions: ctx.session?.positions` compiled only because they annotate `ctx`
+  as `any` — copying both the code and the documented annotation did not build.
+
+**`positions`** (`string[]`, optional) is the ADR-0090 D3 placement vocabulary,
+copied verbatim from `ExecutionContext.positions` by ObjectQL's `buildSession()`.
+Its `.describe()` states the boundary the ruling asked for, because the boundary
+is the whole reason this key needed a decision rather than a patch: it is
+**readable context, never an authorization input**. A hook may forward it as the
+sharing service's evaluation context, tailor a message, or log it. A hook must
+not make the access decision itself by testing it — privilege is judged by the
+security service on the execution context (capability grants `permissions`,
+placements `positions`, and the derived posture). A hook re-deciding access from
+this array decides, somewhere with no access to the grant model, something that
+was already decided; that is structurally the mistake the `roles` tombstone
+exists to prevent, one vocabulary later.
+
+**`preserveAudit`** (`boolean`, optional) is the #3493 historical-import flag:
+server-set, opt-in, absent on normal writes, and read by the built-in audit hook
+to keep a caller-supplied `updated_at`/`updated_by` instead of stamping the
+import instant. It has a live consumer, so it could only ever be declared.
+
+Purely additive — both keys are optional, the shape stays non-strict, and no
+existing context, handler or stored metadata changes. Contexts are built per
+operation and never persisted, so there is nothing to migrate.

packages/spec/src/data/hook.test.ts

@@ -975,3 +975,170 @@ describe('session.roles retirement (#5050, ADR-0049)', () => {
     expect(built.userId).toBe('user_123');
   });
 });
+
+/**
+ * `HookContext.session.positions` / `.preserveAudit` declaration (#5605,
+ * maintainer ruling A of 2026-08-06).
+ *
+ * The MIRROR of the retirement above, and the reason both blocks live in this
+ * file: `roles` was declared-never-produced (delete it), these two are
+ * produced-never-declared (declare them). Same `session` object, opposite
+ * drift, opposite fix — which is why #5605 was filed apart from #5050 rather
+ * than folded into it.
+ *
+ * What was actually broken before the declaration, on `origin/main`:
+ *
+ *   - PARSE — `HookContextSchema` is deliberately non-strict (see the
+ *     `hook.zod.ts` header), so both keys were silently STRIPPED. Measured
+ *     before the change: parsing a session of
+ *     `{ userId, positions, preserveAudit }` returned `{"userId":"u1"}`. That
+ *     is not hypothetical: the generated reference page documents
+ *     `HookContextSchema.parse(data)` as the way to consume a context, so a
+ *     consumer following the docs dropped the caller's positions on the floor.
+ *   - TSC — a handler typed the way `content/docs/automation/index.mdx` teaches,
+ *     `(ctx: HookContext)`, could not read either key: two TS2339s, on
+ *     `ctx.session?.positions` and `ctx.session?.preserveAudit`. The two
+ *     `runtime-services` pages that teach `positions: ctx.session?.positions`
+ *     only look fine because they annotate `ctx` as `any` — copy the code AND
+ *     the documented annotation and it did not compile.
+ *
+ * REVERSE VERIFICATION, direction predicted first: delete either declaration
+ * from `hook.zod.ts` and this block fails BOTH ways — the parse assertions go
+ * red (the key is stripped, so `toHaveProperty` fails), and
+ * `pnpm --filter @objectstack/spec typecheck` reports TS2339 on the typed
+ * reads below. No `@ts-expect-error` is involved here and none should be
+ * added: the fact under test is that documented code COMPILES, so the pin is
+ * a positive typed read, and its failure mode on revert is a hard type error
+ * rather than an unused-directive TS2578.
+ *
+ * Boundary, restated because it is the whole reason the ruling needed a
+ * maintainer: `positions` is readable context, NOT an authorization input.
+ * The `.describe()` says so and the assertion below pins that it keeps saying
+ * so — the next author (or the next AI) reaching for
+ * `session.positions.includes(...)` as an access check is exactly what the
+ * `roles` tombstone above was written to stop.
+ */
+describe('session.positions / session.preserveAudit declaration (#5605)', () => {
+  it('PRESERVES `positions` through a parse instead of stripping it', () => {
+    const context = HookContextSchema.parse({
+      object: 'account',
+      event: 'beforeUpdate',
+      input: {},
+      session: { userId: 'user_123', positions: ['sales_manager', 'org_admin'] },
+      ql: {},
+    });
+
+    expect(context.session).toHaveProperty('positions');
+    expect(context.session?.positions).toEqual(['sales_manager', 'org_admin']);
+  });
+
+  it('PRESERVES `preserveAudit` through a parse (#3493 has a live consumer)', () => {
+    const context = HookContextSchema.parse({
+      object: 'account',
+      event: 'beforeInsert',
+      input: {},
+      session: { userId: 'user_123', preserveAudit: true },
+      ql: {},
+    });
+
+    expect(context.session).toHaveProperty('preserveAudit');
+    expect(context.session?.preserveAudit).toBe(true);
+  });
+
+  it('keeps both OPTIONAL — a normal write produces neither', () => {
+    // `buildSession()` writes `preserveAudit` only under the historical-import
+    // opt-in, and `positions` is absent whenever the execution context carried
+    // none. Declaring them must not start requiring them.
+    //
+    // ⚠️ HONEST NOTE — this one is a COMPANION, not a pin. It asserts absence,
+    // and absence is also what a stripped (undeclared) key produces, so it
+    // stayed green under the reverse verification while its four siblings went
+    // red. It is kept because "declaring them did not make them required" is a
+    // real regression it would catch (a missing `.optional()` turns it red),
+    // but it is not evidence that the declaration exists — do not read it as
+    // such. The pins are the two preserve tests, the `buildSession()` shape,
+    // and the tsc read below.
+    const context = HookContextSchema.parse({
+      object: 'account',
+      event: 'beforeInsert',
+      input: {},
+      session: { userId: 'user_123' },
+      ql: {},
+    });
+
+    expect(context.session?.positions).toBeUndefined();
+    expect(context.session?.preserveAudit).toBeUndefined();
+  });
+
+  it('accepts the exact session shape `buildSession()` builds', () => {
+    // Field-for-field the object ObjectQL assembles (engine.ts `buildSession`)
+    // for a historical import by an authenticated caller. Before #5605 this
+    // parse quietly returned a session two keys shorter than the one the
+    // engine handed the handler.
+    const built = {
+      userId: 'user_123',
+      organizationId: 'org_456',
+      positions: ['sales_manager'],
+      accessToken: 'token_abc123',
+      isSystem: true,
+      actor: 'svc:flow:import_history',
+      skipTriggers: true,
+      skipAutomations: true,
+      preserveAudit: true,
+    };
+
+    const context = HookContextSchema.parse({
+      object: 'account',
+      event: 'beforeInsert',
+      input: {},
+      session: built,
+      ql: {},
+    });
+
+    expect(context.session).toEqual(built);
+  });
+
+  it('type-checks the code the docs teach — `(ctx: HookContext)` reading both keys', () => {
+    // The TSC channel. Both reads were TS2339 before the declaration; this is
+    // `content/docs/kernel/runtime-services/sharing-service.mdx`'s snippet with
+    // the `any` annotation removed, which is what made the omission invisible
+    // there. Explicit annotations, so a widened or renamed declaration fails
+    // here too rather than being absorbed by inference.
+    const readCallerContext = (ctx: HookContext) => {
+      const positions: string[] | undefined = ctx.session?.positions;
+      const preserveAudit: boolean | undefined = ctx.session?.preserveAudit;
+      return { positions, preserveAudit };
+    };
+
+    // ...and the PRODUCER side: the literal `buildSession()` returns must be
+    // assignable to the declared session type.
+    const session: NonNullable<HookContext['session']> = {
+      userId: 'user_123',
+      positions: ['sales_manager'],
+      preserveAudit: true,
+    };
+
+    expect(readCallerContext({
+      object: 'account',
+      event: 'beforeUpdate',
+      input: {},
+      session,
+      ql: {},
+    })).toEqual({ positions: ['sales_manager'], preserveAudit: true });
+  });
+
+  it('carries the "not an authorization input" boundary in the `.describe()`', () => {
+    // The ruling's wording is load-bearing, not decoration: it is the only
+    // thing standing between this key and the next author using it as an
+    // access check. A `.describe()` reaches the generated reference page and
+    // every schema-driven surface, so pin that the boundary survives edits.
+    const sessionShape = HookContextSchema.shape.session.unwrap().shape;
+
+    const positionsDoc = sessionShape.positions.description ?? '';
+    expect(positionsDoc).toMatch(/security service/i);
+    expect(positionsDoc).toMatch(/not an authorization input/i);
+
+    const preserveAuditDoc = sessionShape.preserveAudit.description ?? '';
+    expect(preserveAuditDoc).toMatch(/not an authorization input/i);
+  });
+});

packages/spec/src/data/hook.zod.ts

@@ -405,6 +405,66 @@ export const HookContextSchema = lazySchema(() => z.object({
     isSystem: z.boolean().optional().describe('True when the call was made with an elevated system context (engine self-writes)'),
     skipTriggers: z.boolean().optional().describe('True when record-change automation (flow triggers) must be suppressed for this write — e.g. package seed replay. Lifecycle hooks still run.'),
     skipAutomations: z.boolean().optional().describe('True when metadata-bound automation hooks must be suppressed for this write — e.g. data import with "run automations" unchecked, or import undo. Implies skipTriggers; code-registered system hooks (audit, security) still run.'),
+    /**
+     * Position names held by the caller (ADR-0090 D3 vocabulary — the schema
+     * comment on `ExecutionContext.positions` spells it "Formerly `roles`").
+     * Copied verbatim from `ExecutionContext.positions` by ObjectQL's
+     * `buildSession()` (`packages/objectql/src/engine.ts`).
+     *
+     * ⚠️ **Descriptive, NOT an authorization input.** A hook may READ this to
+     * describe the caller — forwarding it as the sharing service's evaluation
+     * context (`services.sharing.canEdit(..., { positions })`, the shape both
+     * `content/docs/kernel/runtime-services/` pages teach), tailoring a
+     * message, logging — and nothing more. It grants nothing on its own, no
+     * security middleware keys on it here, and a hook must never make the
+     * access decision itself by testing it
+     * (`session.positions.includes('sales_manager')` is the anti-pattern).
+     * PRIVILEGE is judged by the security service on the ExecutionContext:
+     * capability grants (`permissions`), placements (`positions`) and the
+     * derived posture (ADR-0095 D3). A hook that re-decides access from this
+     * array is deciding, in a place with no access to the grant model,
+     * something already decided — structurally the same mistake as the
+     * `roles` tombstone below (#5050), one vocabulary later. That is why this
+     * key is declared with the boundary written down rather than left to be
+     * inferred from its name.
+     *
+     * Declared in #5605 (maintainer ruling A): it was PRODUCED by
+     * `buildSession()` and TAUGHT by two kernel doc pages while the contract
+     * omitted it — so `HookContextSchema.parse()` silently stripped it (this
+     * shape is deliberately non-strict, see the header) and a handler typed
+     * `(ctx: HookContext)` could not read it without TS2339. Produced-never-
+     * declared, the mirror of `roles`' declared-never-produced.
+     */
+    positions: z.array(z.string()).optional().describe(
+      'Position names held by the caller (ADR-0090 D3; formerly `roles`), copied from '
+      + 'ExecutionContext.positions. For hook READS only — e.g. forwarding to the sharing '
+      + 'service as evaluation context. Authorization is decided by the security service on '
+      + 'the ExecutionContext (permissions / positions / derived posture); this is NOT an '
+      + 'authorization input and a hook must not gate a write by testing it.',
+    ),
+    /**
+     * Historical-import audit-preservation flag (#3493). Set by
+     * `buildSession()` only when the write context carries it, so a normal
+     * write leaves it absent.
+     *
+     * Its one consumer is the built-in audit hook
+     * (`packages/objectql/src/plugin.ts`, `applyToRecord`): when true, a
+     * client-supplied `updated_at` / `updated_by` is PREFERRED and kept —
+     * reinstating the original timeline of imported history — instead of
+     * being overwritten with the import instant, symmetric with how
+     * `created_at` / `created_by` behave on insert. It also whitelists the
+     * audit/timestamp family through `stripReadonlyFields()`.
+     *
+     * Server-set and opt-in; like {@link positions} it authorizes nothing —
+     * it selects a stamping policy for a write the security service has
+     * already allowed.
+     */
+    preserveAudit: z.boolean().optional().describe(
+      'True when this write is a historical import that must KEEP its caller-supplied '
+      + 'updated_at/updated_by (and the readonly audit family) instead of being stamped with '
+      + 'the import instant (#3493). Server-set, opt-in, absent on normal writes; read by the '
+      + 'built-in audit hook. A stamping policy, not an authorization input.',
+    ),
     // `roles` REMOVED (#5050, ADR-0049 D2). It was DECLARED here, READ by two
     // dead exemption branches in plugin-approvals (the approval record lock and
     // the delegation write guard, both deleted in #4839 / PR #5049), and NEVER