|
| 1 | +--- |
| 2 | +"@objectstack/spec": major |
| 3 | +--- |
| 4 | + |
| 5 | +refactor(spec)!: retire `HookContext.session.roles` — declared, read by two dead branches, never produced (#5050) |
| 6 | + |
| 7 | +`session.roles` on the runtime hook context was ADR-0049's enforce-or-remove |
| 8 | +case with **neither end**: it was declared in `data/hook.zod.ts`, read by |
| 9 | +exactly two consumers, and produced by nobody. The two readers were |
| 10 | +plugin-approvals' admin exemptions — the approval record lock and the delegation |
| 11 | +write guard, each opening with `session.roles?.includes('admin')` — and both |
| 12 | +were deleted in #4839 (PR #5049) on the maintainer's ruling. The producer side |
| 13 | +was empty the whole time: ObjectQL's `buildSession()` builds the session field |
| 14 | +by field (`userId`, `organizationId`, `accessToken`, `isSystem`, `actor`, the |
| 15 | +skip flags) and has never written `roles`, and nothing else feeds a |
| 16 | +HookContext. So every read resolved `undefined`, |
| 17 | +and a guard keyed on it was dead code that merely LOOKED like an authorization |
| 18 | +decision — plus a second admin dialect competing with the one ADR-0090 D3 / |
| 19 | +ADR-0095 D3 sanction. |
| 20 | + |
| 21 | +Cross-repo consumer check ran in both directions before removing anything |
| 22 | +(#4895's discipline, after #4865's tombstone was disproven by objectui): |
| 23 | +`cloud` has zero `session.roles` while its hook consumers really do read |
| 24 | +`hookContext?.session?.userId` (`service-cloud/src/marketplace-visibility-plugin.ts`, |
| 25 | +`control-plane-org-scope-plugin.ts`) — a positive control in the same run; |
| 26 | +`objectui` has zero, and its `roles` are the `/auth/me` **user** payload, a |
| 27 | +different surface that is untouched. |
| 28 | + |
| 29 | +One neighbour is called out rather than folded in, because mistaking it for a |
| 30 | +producer would read as refuting the whole finding (#4865's lesson): an **action** |
| 31 | +body's `ctx.session` is a different, untyped object built by `runtime`'s |
| 32 | +`buildActionSession()`, and it does populate a `roles` key from `ec.positions`. |
| 33 | +It never becomes a HookContext and no schema types it, so it is neither evidence |
| 34 | +against this retirement nor fixed by it — it is filed and tracked apart. |
| 35 | + |
| 36 | +**Nothing observable changes.** A key nobody wrote and nothing read cannot alter |
| 37 | +a single decision — this is the declaration catching up with the runtime, not a |
| 38 | +behaviour change. |
| 39 | + |
| 40 | +FROM → TO: |
| 41 | + |
| 42 | +- `HookContext.session.roles` (`@objectstack/spec/data`) → removed. Delete the |
| 43 | + key. To gate a hook on the caller, read `ctx.session.userId` / |
| 44 | + `ctx.session.isSystem`; to judge PRIVILEGE, ask the security service, which |
| 45 | + evaluates capability grants (`permissions`), placements (`positions`) and the |
| 46 | + derived posture off the execution context (ADR-0095 D3) — never a role-name |
| 47 | + string comparison. |
| 48 | + |
| 49 | +The retirement kit: the key is **tombstoned**, not deleted, because |
| 50 | +`HookContextSchema` is deliberately not `.strict()` (strictness there would turn |
| 51 | +an engine-internal enrichment into a breaking change for anyone parsing a |
| 52 | +context they were handed, as `provenance` was in #3712) — a plain delete would |
| 53 | +have stripped it in silence, the #3733 / ADR-0104 failure. `retiredKey()` gives |
| 54 | +both channels instead: `tsc` types the key `never` at any producer, and a parse |
| 55 | +raises the prescription itself. There is **no** ADR-0087 D2 conversion and |
| 56 | +nothing for `os migrate meta` to rewrite: a HookContext is built per operation |
| 57 | +by the engine and never stored, so no `sys_metadata` row, example or template |
| 58 | +can carry the key — the `openApi31` (#4579) / `activationEvents` (#4657) shape, |
| 59 | +registered as the `hook-context-session-roles-retired` **semantic** migration at |
| 60 | +major 17 so the prescription still reaches `spec-changes.json`, the generated |
| 61 | +upgrade guide and the `spec_changes` MCP tool. The four export/def ratchets are |
| 62 | +unchanged by design: this narrows a nested key inside a surviving def, which |
| 63 | +`api-surface.json`, `authorable-surface.json` (whose walk records top-level keys |
| 64 | +per def), `api-surface-signatures.json` and `json-schema.manifest.json` are all |
| 65 | +blind to — the enum-narrowing reading of the two, not the whole-def one. |
0 commit comments