Skip to content

Commit ef4efa8

Browse files
os-zhuangclaude
andauthored
feat(spec): declare HookContext.session.positions / .preserveAudit (#5605) (#5722)
Two keys the engine produces, consumers read and the docs teach were missing from HookContextSchema.session. Declared per the maintainer ruling (A) on #5605 — the mirror of the #5050 `session.roles` retirement: that key was declared-never-produced (removed), these two are produced-never-declared (added). Because the shape is deliberately non-strict, the omission was silent: `HookContextSchema.parse(ctx)` — the call the generated reference documents — stripped both keys, and a handler typed `(ctx: HookContext)` as the automation docs teach hit TS2339 on `ctx.session?.positions`. The two runtime-services pages that teach that exact read compile only because they annotate `ctx` as `any`. `positions` carries the ruling's boundary wording in its `.describe()`: readable context for hooks, never an authorization input — privilege is judged by the security service on the ExecutionContext (permissions / positions / derived posture), never by testing this array in a hook. Same discipline as the `roles` tombstone, which the new keys sit ABOVE so the generated reference's four-key inline summary does not surface the tombstone as `roles?: any`. `preserveAudit` documents its real consumer semantics: the #3493 historical-import flag read by the built-in audit hook to keep a caller-supplied updated_at/updated_by instead of stamping the import instant. Both optional and additive; contexts are built per operation and never stored, so there is nothing to migrate. Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9fe9c1d commit ef4efa8

3 files changed

Lines changed: 280 additions & 0 deletions

File tree

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

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

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,3 +975,170 @@ describe('session.roles retirement (#5050, ADR-0049)', () => {
975975
expect(built.userId).toBe('user_123');
976976
});
977977
});
978+
979+
/**
980+
* `HookContext.session.positions` / `.preserveAudit` declaration (#5605,
981+
* maintainer ruling A of 2026-08-06).
982+
*
983+
* The MIRROR of the retirement above, and the reason both blocks live in this
984+
* file: `roles` was declared-never-produced (delete it), these two are
985+
* produced-never-declared (declare them). Same `session` object, opposite
986+
* drift, opposite fix — which is why #5605 was filed apart from #5050 rather
987+
* than folded into it.
988+
*
989+
* What was actually broken before the declaration, on `origin/main`:
990+
*
991+
* - PARSE — `HookContextSchema` is deliberately non-strict (see the
992+
* `hook.zod.ts` header), so both keys were silently STRIPPED. Measured
993+
* before the change: parsing a session of
994+
* `{ userId, positions, preserveAudit }` returned `{"userId":"u1"}`. That
995+
* is not hypothetical: the generated reference page documents
996+
* `HookContextSchema.parse(data)` as the way to consume a context, so a
997+
* consumer following the docs dropped the caller's positions on the floor.
998+
* - TSC — a handler typed the way `content/docs/automation/index.mdx` teaches,
999+
* `(ctx: HookContext)`, could not read either key: two TS2339s, on
1000+
* `ctx.session?.positions` and `ctx.session?.preserveAudit`. The two
1001+
* `runtime-services` pages that teach `positions: ctx.session?.positions`
1002+
* only look fine because they annotate `ctx` as `any` — copy the code AND
1003+
* the documented annotation and it did not compile.
1004+
*
1005+
* REVERSE VERIFICATION, direction predicted first: delete either declaration
1006+
* from `hook.zod.ts` and this block fails BOTH ways — the parse assertions go
1007+
* red (the key is stripped, so `toHaveProperty` fails), and
1008+
* `pnpm --filter @objectstack/spec typecheck` reports TS2339 on the typed
1009+
* reads below. No `@ts-expect-error` is involved here and none should be
1010+
* added: the fact under test is that documented code COMPILES, so the pin is
1011+
* a positive typed read, and its failure mode on revert is a hard type error
1012+
* rather than an unused-directive TS2578.
1013+
*
1014+
* Boundary, restated because it is the whole reason the ruling needed a
1015+
* maintainer: `positions` is readable context, NOT an authorization input.
1016+
* The `.describe()` says so and the assertion below pins that it keeps saying
1017+
* so — the next author (or the next AI) reaching for
1018+
* `session.positions.includes(...)` as an access check is exactly what the
1019+
* `roles` tombstone above was written to stop.
1020+
*/
1021+
describe('session.positions / session.preserveAudit declaration (#5605)', () => {
1022+
it('PRESERVES `positions` through a parse instead of stripping it', () => {
1023+
const context = HookContextSchema.parse({
1024+
object: 'account',
1025+
event: 'beforeUpdate',
1026+
input: {},
1027+
session: { userId: 'user_123', positions: ['sales_manager', 'org_admin'] },
1028+
ql: {},
1029+
});
1030+
1031+
expect(context.session).toHaveProperty('positions');
1032+
expect(context.session?.positions).toEqual(['sales_manager', 'org_admin']);
1033+
});
1034+
1035+
it('PRESERVES `preserveAudit` through a parse (#3493 has a live consumer)', () => {
1036+
const context = HookContextSchema.parse({
1037+
object: 'account',
1038+
event: 'beforeInsert',
1039+
input: {},
1040+
session: { userId: 'user_123', preserveAudit: true },
1041+
ql: {},
1042+
});
1043+
1044+
expect(context.session).toHaveProperty('preserveAudit');
1045+
expect(context.session?.preserveAudit).toBe(true);
1046+
});
1047+
1048+
it('keeps both OPTIONAL — a normal write produces neither', () => {
1049+
// `buildSession()` writes `preserveAudit` only under the historical-import
1050+
// opt-in, and `positions` is absent whenever the execution context carried
1051+
// none. Declaring them must not start requiring them.
1052+
//
1053+
// ⚠️ HONEST NOTE — this one is a COMPANION, not a pin. It asserts absence,
1054+
// and absence is also what a stripped (undeclared) key produces, so it
1055+
// stayed green under the reverse verification while its four siblings went
1056+
// red. It is kept because "declaring them did not make them required" is a
1057+
// real regression it would catch (a missing `.optional()` turns it red),
1058+
// but it is not evidence that the declaration exists — do not read it as
1059+
// such. The pins are the two preserve tests, the `buildSession()` shape,
1060+
// and the tsc read below.
1061+
const context = HookContextSchema.parse({
1062+
object: 'account',
1063+
event: 'beforeInsert',
1064+
input: {},
1065+
session: { userId: 'user_123' },
1066+
ql: {},
1067+
});
1068+
1069+
expect(context.session?.positions).toBeUndefined();
1070+
expect(context.session?.preserveAudit).toBeUndefined();
1071+
});
1072+
1073+
it('accepts the exact session shape `buildSession()` builds', () => {
1074+
// Field-for-field the object ObjectQL assembles (engine.ts `buildSession`)
1075+
// for a historical import by an authenticated caller. Before #5605 this
1076+
// parse quietly returned a session two keys shorter than the one the
1077+
// engine handed the handler.
1078+
const built = {
1079+
userId: 'user_123',
1080+
organizationId: 'org_456',
1081+
positions: ['sales_manager'],
1082+
accessToken: 'token_abc123',
1083+
isSystem: true,
1084+
actor: 'svc:flow:import_history',
1085+
skipTriggers: true,
1086+
skipAutomations: true,
1087+
preserveAudit: true,
1088+
};
1089+
1090+
const context = HookContextSchema.parse({
1091+
object: 'account',
1092+
event: 'beforeInsert',
1093+
input: {},
1094+
session: built,
1095+
ql: {},
1096+
});
1097+
1098+
expect(context.session).toEqual(built);
1099+
});
1100+
1101+
it('type-checks the code the docs teach — `(ctx: HookContext)` reading both keys', () => {
1102+
// The TSC channel. Both reads were TS2339 before the declaration; this is
1103+
// `content/docs/kernel/runtime-services/sharing-service.mdx`'s snippet with
1104+
// the `any` annotation removed, which is what made the omission invisible
1105+
// there. Explicit annotations, so a widened or renamed declaration fails
1106+
// here too rather than being absorbed by inference.
1107+
const readCallerContext = (ctx: HookContext) => {
1108+
const positions: string[] | undefined = ctx.session?.positions;
1109+
const preserveAudit: boolean | undefined = ctx.session?.preserveAudit;
1110+
return { positions, preserveAudit };
1111+
};
1112+
1113+
// ...and the PRODUCER side: the literal `buildSession()` returns must be
1114+
// assignable to the declared session type.
1115+
const session: NonNullable<HookContext['session']> = {
1116+
userId: 'user_123',
1117+
positions: ['sales_manager'],
1118+
preserveAudit: true,
1119+
};
1120+
1121+
expect(readCallerContext({
1122+
object: 'account',
1123+
event: 'beforeUpdate',
1124+
input: {},
1125+
session,
1126+
ql: {},
1127+
})).toEqual({ positions: ['sales_manager'], preserveAudit: true });
1128+
});
1129+
1130+
it('carries the "not an authorization input" boundary in the `.describe()`', () => {
1131+
// The ruling's wording is load-bearing, not decoration: it is the only
1132+
// thing standing between this key and the next author using it as an
1133+
// access check. A `.describe()` reaches the generated reference page and
1134+
// every schema-driven surface, so pin that the boundary survives edits.
1135+
const sessionShape = HookContextSchema.shape.session.unwrap().shape;
1136+
1137+
const positionsDoc = sessionShape.positions.description ?? '';
1138+
expect(positionsDoc).toMatch(/security service/i);
1139+
expect(positionsDoc).toMatch(/not an authorization input/i);
1140+
1141+
const preserveAuditDoc = sessionShape.preserveAudit.description ?? '';
1142+
expect(preserveAuditDoc).toMatch(/not an authorization input/i);
1143+
});
1144+
});

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,66 @@ export const HookContextSchema = lazySchema(() => z.object({
405405
isSystem: z.boolean().optional().describe('True when the call was made with an elevated system context (engine self-writes)'),
406406
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.'),
407407
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.'),
408+
/**
409+
* Position names held by the caller (ADR-0090 D3 vocabulary — the schema
410+
* comment on `ExecutionContext.positions` spells it "Formerly `roles`").
411+
* Copied verbatim from `ExecutionContext.positions` by ObjectQL's
412+
* `buildSession()` (`packages/objectql/src/engine.ts`).
413+
*
414+
* ⚠️ **Descriptive, NOT an authorization input.** A hook may READ this to
415+
* describe the caller — forwarding it as the sharing service's evaluation
416+
* context (`services.sharing.canEdit(..., { positions })`, the shape both
417+
* `content/docs/kernel/runtime-services/` pages teach), tailoring a
418+
* message, logging — and nothing more. It grants nothing on its own, no
419+
* security middleware keys on it here, and a hook must never make the
420+
* access decision itself by testing it
421+
* (`session.positions.includes('sales_manager')` is the anti-pattern).
422+
* PRIVILEGE is judged by the security service on the ExecutionContext:
423+
* capability grants (`permissions`), placements (`positions`) and the
424+
* derived posture (ADR-0095 D3). A hook that re-decides access from this
425+
* array is deciding, in a place with no access to the grant model,
426+
* something already decided — structurally the same mistake as the
427+
* `roles` tombstone below (#5050), one vocabulary later. That is why this
428+
* key is declared with the boundary written down rather than left to be
429+
* inferred from its name.
430+
*
431+
* Declared in #5605 (maintainer ruling A): it was PRODUCED by
432+
* `buildSession()` and TAUGHT by two kernel doc pages while the contract
433+
* omitted it — so `HookContextSchema.parse()` silently stripped it (this
434+
* shape is deliberately non-strict, see the header) and a handler typed
435+
* `(ctx: HookContext)` could not read it without TS2339. Produced-never-
436+
* declared, the mirror of `roles`' declared-never-produced.
437+
*/
438+
positions: z.array(z.string()).optional().describe(
439+
'Position names held by the caller (ADR-0090 D3; formerly `roles`), copied from '
440+
+ 'ExecutionContext.positions. For hook READS only — e.g. forwarding to the sharing '
441+
+ 'service as evaluation context. Authorization is decided by the security service on '
442+
+ 'the ExecutionContext (permissions / positions / derived posture); this is NOT an '
443+
+ 'authorization input and a hook must not gate a write by testing it.',
444+
),
445+
/**
446+
* Historical-import audit-preservation flag (#3493). Set by
447+
* `buildSession()` only when the write context carries it, so a normal
448+
* write leaves it absent.
449+
*
450+
* Its one consumer is the built-in audit hook
451+
* (`packages/objectql/src/plugin.ts`, `applyToRecord`): when true, a
452+
* client-supplied `updated_at` / `updated_by` is PREFERRED and kept —
453+
* reinstating the original timeline of imported history — instead of
454+
* being overwritten with the import instant, symmetric with how
455+
* `created_at` / `created_by` behave on insert. It also whitelists the
456+
* audit/timestamp family through `stripReadonlyFields()`.
457+
*
458+
* Server-set and opt-in; like {@link positions} it authorizes nothing —
459+
* it selects a stamping policy for a write the security service has
460+
* already allowed.
461+
*/
462+
preserveAudit: z.boolean().optional().describe(
463+
'True when this write is a historical import that must KEEP its caller-supplied '
464+
+ 'updated_at/updated_by (and the readonly audit family) instead of being stamped with '
465+
+ 'the import instant (#3493). Server-set, opt-in, absent on normal writes; read by the '
466+
+ 'built-in audit hook. A stamping policy, not an authorization input.',
467+
),
408468
// `roles` REMOVED (#5050, ADR-0049 D2). It was DECLARED here, READ by two
409469
// dead exemption branches in plugin-approvals (the approval record lock and
410470
// the delegation write guard, both deleted in #4839 / PR #5049), and NEVER

0 commit comments

Comments
 (0)