refactor(core,runtime,rest): one ExecutionContext assembler, two named anonymous entries (#6216) - #7259
Conversation
…wo named anonymous entries (#6216) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 3 package(s): 36 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also reference the affected code. These are read-only:
|
…eld-set pin for the shared assembler (#6216) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
github-actions
Bot
added
size/xl
documentation
) The shared assembler imported `scopesToAgentPermissionSets` / `MCP_OAUTH_SCOPE_ACTIONS` from `@objectstack/spec/ai` as VALUES. Every package whose vitest config aliases `@objectstack/core` to its source must then resolve that subpath too, and five do not (metadata, driver-memory, driver-sql, plugin-dev, plugin-hono-server) — driver-memory's suite died with 'ENOTDIR: not a directory, open .../spec/src/index.ts/ai'. The scope vocabulary is MCP-domain knowledge, so it is interpreted at the only door that speaks it: `OAuthTokenProvenance` now carries the already-derived `scopePermissions` + `delegatesActions`, and the assembler still decides — once, for every face — what that ceiling replaces on the envelope. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
|
Note for the reviewer, and a tooling observation worth recording: the PR body and both filed findings (#7279, #7280) were each written ending with the required attribution block and in all three cases the stored body comes back without it. Read-back confirms the text is absent at rest, not merely hidden — while the same block survives intact in issue comments (every comment on #6216 still carries it, including this one). So the strip happens on the body write path, not on read and not on comments. Recording it here so the convention is satisfied where it can be, and so the next agent does not spend the same time diagnosing it. Generated by Claude Code |
|
PM review — PASS. Marked ready and enqueued. Identity-lane PM seat (#6022), session CI on head The residual was disclosed — and this seat closed the question the disclosure left openThe report states the one measured delta honestly: the shared assembler omits undefined-valued keys where the legacy literals sometimes spelled them, so the entire on-the-wire diff is a single key ( That caveat is not theoretical in this repo, so I checked rather than accepting "semantically inert" on its face: Type closure: shown, with an honest limit that this seat accepts rather than waves throughProbe A (add an optional field to But under Probe A the runtime and rest typechecks stayed GREEN, which the author predicted in writing beforehand and refused to round up to the ruling's wording ("lands in every face at compile time"). That refusal is the right call and I am recording my judgement explicitly rather than letting it slide: the ruling's purpose was that the #6071 drift class become unrepresentable, and with a single assembler there is no per-face assembly left to forget — which achieves the purpose more directly than per-face compile errors would, and Probe B shows propagation to the faces exactly when a new field needs a per-face value. Not a Converging surfaced a second real divergence, and it was handled by value rather than by switch
Two things that raise this above a passing review
Also correct: three of the four reverse-verification pins are deliberately GREEN under the author's own mutation, stated as a result rather than dressed as reds — they exist to catch a future behaviour change, and their green against restored legacy code is the proof this PR changed nothing. Deliberately deferred, and rightlyThe stdio MCP face ( Tooling, confirmed independently by this seat: the trailing Generated by Claude Code |
2 participants
Fixes #6216
Implements the maintainer ruling of 2026-08-08 (comment 5227240904), Option A — one shared assembler, explicit dual mode, fail-closed default.
What changed
resolveAuthzContextalready made AUTHORIZATION resolution single-sourced. The step after it — turning the resolved envelope into theExecutionContextthat reaches enforcement — was still one hand-written copy per transport, and that duplication produced a measured defect family: #6071 (the REST copy never setprincipalKind, so every enforcement judgment reading it was silently never-true on that face) and #6206 / #6551 (a droppedaccessible_org_ids, and thegroupposture's Layer 0 wall reads it directly — real 403s).New module
packages/core/src/security/assemble-execution-context.ts, with the anonymous divergence as named API instead of drift:assembleExecutionContext(input)— the fail-closed default entry. No resolved principal, no context; the surface answers 401. REST's contract, unchanged.assembleExecutionContextOrGuest(input)— the explicit guest entry. No resolved principal produces today's dispatcher guest envelope (principalKind: 'guest',positions: ['guest']), whose consumers are live (plugin-security/explain-engine.tsreads guest for itsEXTERNALposture floor). Adopted only by a surface whose product semantics serve anonymous principals.EntryExecutionContextFieldisExclude< keyof ExecutionContext, NonEntryExecutionContextField >, andExecutionContextEntryFieldsmakes every one of those keys required (the value may beundefined; the decision may not be omitted).packages/runtime/src/security/resolve-execution-context.ts(dispatcher / MCP door) now takes the guest entry;packages/rest/src/rest-server.tscomputeExecCtxtakes the fail-closed default.The share-link surfaces already consume whole envelopes (#6552 / #6647) and were deliberately not re-plumbed.
The remaining divergences are values, not switches
Converging turned up a second real divergence beyond the anonymous face, and it is load-bearing:
accessToken. The dispatcher has always carriedauthz.accessToken; REST's literal never had the key. It is not inert —packages/objectql/src/engine.tsbuildSessionfeeds it toHookContext.session.accessToken, a published hook surface (packages/spec/src/data/hook.zod.ts). A naive convergence would have started exposing the better-auth session token to hooks on every REST-driven write. SoaccessTokenis a required input of the assembler: the dispatcher passesauthz.accessToken, REST passesundefinedwith the reason written at the call site. Same foroauth(the/mcpdoor alone). Required inputs, not booleans — a face cannot silently omit one, and what it withholds it withholds on the record.Evidence 1 — no behaviour change, measured both ways
A green suite proves nothing here; the suite was green before. Two independent measurements:
(a) Frozen-transcription parity matrix (
packages/core/src/security/assemble-execution-context.test.ts). The pre-#6216 assembly of each face is transcribed verbatim fromorigin/main@0caf122f0, frozen, and every shape either face serves is assembled both ways and compared: 3 authz shapes (anonymous / minimal human / full human with tenant+email+token+tabs+posture) times 6 OAuth shapes (none, agentdata:read, agentdata:write, agent withactions:execute, bearer withoutazp, bearer for a different user) times 4 localization shapes times 2 request-locale shapes, plus the REST matrix. 179 tests, all green. The transcriptions are a pin, not a live second implementation — nothing imports them, and the day they need editing is the day a face's output changed.(b) On the wire, against the real pipeline. Both face files were restored from
origin/mainwith the new tests kept, and the tests re-run through the realcomputeExecCtxand the realresolveExecutionContext. Result below.Evidence 2 — the type closure actually bites (shown, not asserted)
Probe A — add a field to
ExecutionContext. AddedprobeField6216: z.string().optional()toExecutionContextSchema, rebuilt@objectstack/spec, rantscon@objectstack/core. Core went from 98 pre-existing errors (its ledgered DEBT number, unchanged by this PR) to 100, and both new errors are the closure firing, independently:Line 252 is the assembly literal; line 149 is the order-list exhaustiveness constant.
pnpm --filter @objectstack/core buildfails on the same two errors (Error: error occurred in dts build), so the bite is visible to CI's build job, not only to a localtsc. The runtime pin went red too, naming the field:Probe B — add a required per-face input. Added a required
probeInput6216: stringtoExecutionContextAssemblyInputand rebuilt core. Both faces went red:Honest limit of the guarantee. The ruling says a new
ExecutionContextfield "must land in every face at compile time". What Probe A measures is that it lands at the one place where every face's value is decided — and Probe A's runtime and rest typechecks stayed green, which was predicted in writing before the run. That is the design working, not a gap: with one assembler the faces no longer assemble, so there is no per-face site left to forget. A new field propagates to the faces (Probe B) exactly when it needs a per-face value. Stated plainly rather than rounded up to the ruling's phrasing.The one measured residual
The shared assembler omits
undefined-valued keys; both legacy literals sometimes spelled them. Restoring the legacy REST face turned exactly one new assertion red, and the whole diff was one key:tenantIdwas present-with-undefinedfor a tenant-less principal. This is invisible toctx.xreads, toJSON.stringifyand to spreading the envelope; it is visible toObject.keysandin. No value moved on either face, for any shape. It is pinned rather than waved off — see the "measured residual" block in the core test.Reverse verification — direction predicted before running
Predicted in writing (scratchpad, before the run): restore both face files from
origin/main, keep the new tests. 1 red (thetenantIdpresence residual on REST), 3 green — because the other new pins assert unchanged behaviour and legacy code has that behaviour by construction.accessTokentenantIdonlytenantIdonlyaccessTokenstill withheldMeasured equals predicted, including the three deliberately-green ones. A guard that stays green under its author's own mutation is the result here, not a gap: these pins exist to catch a future change of behaviour, and their green under the legacy code is the proof that this PR did not change it.
One thing I did not predict: the first run of the REST key-set pin failed against my own code, because I guessed the golden set wrong (
currencyinstead ofemail— the test's ql stub returns asys_userrow with an email, and no currency is configured). The pin caught its author. Corrected before anything was pushed.Gates
pnpm --filter @objectstack/core testpnpm --filter @objectstack/rest testpnpm --filter @objectstack/runtime testpnpm --filter @objectstack/runtime --filter @objectstack/rest typecheckeslinton all 7 touched filescheck:authz-resolvercheck:org-identifiercheck:tenant-chokepointcheck:required-contextscheck:role-word/check:empty-changeset/check:spec-parsed-alias/check:slot-lookupcheck:type-check-coveragecheck:nul-bytes+ targeted control-byte self-scanChangeset:
.changeset/execution-context-single-assembler.md.Deliberately not done
packages/mcp/src/plugin.ts) is a fourth hand-written assembly that dropstabPermissionsandaccessToken. Converging it would change its output, on a surface the ruling did not weigh. Filed as finding: a THIRD hand-written ExecutionContext assembly survives in the stdio MCP plugin, and it dropstabPermissions/accessToken#7279.context.authGateis read by REST'senforceAuthbut never declared onExecutionContextSchema, so it sits oneas anyoutside the closure this PR builds. Filed as finding:context.authGateis read by REST'senforceAuthbut is not declared onExecutionContextSchema— an undeclared field outside every closure gate #7280.