Skip to content

fix(plugin-hono-server): the current-user faces assemble their ExecutionContext through the shared assembler - #15891

Merged
os-litant merged 5 commits into
mainfrom
claude/issue-15747-execution-context-envelope
Sep 5, 2026
Merged

fix(plugin-hono-server): the current-user faces assemble their ExecutionContext through the shared assembler#15891
os-litant merged 5 commits into
mainfrom
claude/issue-15747-execution-context-envelope

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #15747

The card asked for the GRADE before the repair, and the grade is settled first below. It holds: LATENT, on a re-derived measurement rather than on the card's word for it — and the five fields it recorded as NOT MEASURED are now measured.

1. The grade — LATENT, upheld

The card stated its own falsifier: "if an absent principalKind is reachable as anything other than 'human' on this face — or if any of the other five omitted fields acquires a reader that distinguishes absent from a real value — this becomes a live, security-relevant defect." Both halves were driven.

principalKind — three legs, re-derived

leg verdict
it IS read downstream holdsplugin-security/src/security-plugin.ts:4696, const isAgent = context?.principalKind === 'agent'
it is read ONLY to test for 'agent' holds — and one reader the card did not name: permissionSetMemoKey (same file, :4631) folds context?.principalKind ?? null into the memo key
this face accepts no OAuth token holds — structurally, see below

The memo-key reader is not a second distinguisher: permissionSetMemo is a WeakMap keyed on the context OBJECT, and each request builds a fresh one, so a changed key VALUE cannot collide across requests. It is compared only against the same object's earlier key.

The load-bearing leg is a chain of single call sites, not an inference:

  1. principalKind: 'agent' is emitted by assembleExecutionContext* only when oauth?.clientId is set;
  2. oauth is non-undefined only inside resolveExecutionContext (packages/runtime/src/security/resolve-execution-context.ts:116), under opts.acceptOAuthAccessToken;
  3. acceptOAuthAccessToken has exactly ONE production setter in the repo — packages/runtime/src/http-dispatcher.ts:565, gated on the /mcp path regex;
  4. makeExecutionContextResolver never calls resolveExecutionContext at all. It resolves the session through better-auth getSession({ headers }) and calls resolveUserAuthzGrants directly, so the OAuth verification branch is not on this face's code path in any form.

'guest' is unreachable for a different reason: the resolver answers undefined with no session, and all three handlers return their own no-session body before an envelope exists. So an absent principalKind on this face was reachable as 'human' and nothing else.

Both halves are now pinned as tests, so a future OAuth door here turns them red instead of silently promoting the hazard:

  • an OAuth-shaped bearer on /auth/me/permissions leaves principalKind: 'human' and never calls verifyMcpAccessToken — a method deliberately PRESENT on the fake auth service, which would have answered an agent principal had it been asked;
  • an unauthenticated request reaches no consumer at all.

PR #15859 — checked, and it is MEASURED, not assumed. Its second repair site is the acceptOAuthAccessToken gate, which is exactly the flag above. It cannot reach this face: the flag is an option to resolveExecutionContext, a function this resolver does not call, and these three routes are never /mcp. Changing which prefix the dispatcher strips cannot introduce an OAuth branch into a resolver that has none. (Measured against #15859 as it stands open at base 59953d5.)

The other five — recorded NOT MEASURED, now measured

The envelope reaches exactly ONE consumer on these faces: ISecurityService.resolvePermissionSetsForContext, from /auth/me/permissions and /me/apps. /auth/me/localization hands it to nothing. That consumer's complete set of context reads is positions, permissions, principalKind, the PRESENCE of userId, and callerOrganizationId (organizationId ?? tenantId) — its own docblock says so and the body agrees. The handlers read userId, tenantId, positions, permissions, systemPermissions, tabPermissions.

field has readers? reachable from these faces?
onBehalfOf yes — plugin-security :1819/:4388/:4524/:4594/:6723, plus sharing, approvals, reports, audit no — every one sits on the ENGINE middleware path (opCtx.context), reached by engine operations, never by resolvePermissionSetsForContext
audience none on ExecutionContext.audience no — the assembler itself records it as undefined on every face (no transport resolves an external audience yet)
accessToken yes — objectql/engine.ts buildSession surfaces it to hooks no — engine ops only
authGate yes — RestServer.enforceAuth no — REST transport only
oauthScopes yes — runtime/src/domains/mcp.ts, to narrow tool families no — MCP transport only

⇒ none of the five has a reader that can distinguish absent from a real value here. The grade stands at LATENT for all six fields.

One supporting measurement the conversion depends on: nothing in the reachable consumer keys on key PRESENCE. hasOwnProperty appears twice in security-plugin.ts and both are on data rows, never on the context. That is what makes the assembler's emit() — which DROPS undefined-valued keys — safe here.

2. Dedup, re-run with a live control

The repo-scoped REST channel is 403 for this session (whole class; gh absent), so this went through one targeted MCP search_issueschannel switch declared.

A second reading was needed for the HARD SERIAL claim check on the file being edited, and it came back EMPTY, so it got its own control on the same endpoint in the same session:

⇒ the 0 is a reading: no open PR claims packages/plugins/plugin-hono-server/src/current-user-endpoints.ts. It was freed by PR #15745's merge, and git log -1 on the file confirms that merge is its most recent commit.

3. The blast radius — measured on every affected face, before and after

This is what #15387 declined to take on, and why the card is its own card. Measured by capturing the context that ARRIVES at the real consumer, through the REAL registered routes (registerCurrentUserEndpoints on a real Hono app, driven with app.request()) — the same instrumentation #6071 used on the REST face.

face envelope before envelope after wire body
/auth/me/permissions 11 keys, no principalKind, tabPermissions present-with-undefined 11 keys, principalKind: 'human', tabPermissions dropped as an undefined decision identical
/me/apps same 11 same 11 identical
/auth/me/localization consults userId / tenantId only (since #15387) unchanged identical

All three bodies are asserted as goldens in the new suite, and the /auth/me/permissions golden was CORRECTED to the measured before-state (positions: ['org_member', 'everyone']) after my first guess at it was wrong — the point of those three cases is identity with what the face answered before, so a guessed golden would have been worthless.

4. The repair shape, and the axes

Shape (i) — convert the resolver to assemble-execution-context.ts.

Each withheld input is named on the record rather than omitted: no OAuth door; no localization (the endpoint reads that cascade itself since #15387 — resolving it here too would add a sys_setting read to every request on all three routes and give one cascade two readings to keep in agreement); no session bearer (the REST face's stance); no auth gate (its consumer is the REST seam, and core/security/auth-gate.ts allow-lists /me/apps + /me/localization as routes a GATED user must still reach, so carrying it here would be an unread copy on a surface the gate deliberately does not close).

5. The ablation — predicted in writing first, both limbs

Predictions were written before either limb ran, including which assertions stay GREEN and why. Each mutation was proven on disk by removed-text AND injected-marker counts, with a zero count treated as VOID; each restore ran under an EXIT INT TERM trap using absolute paths and git checkout HEAD -- ABSOLUTE_PATH, proven by blob-hash equality against the HEAD blob AND an empty git diff HEAD.

limb predicted measured
A — the RESOLVER withholds principalKind again 4 fail / 5 pass 4 fail / 5 pass — the four named cases, exactly
B — the shared ASSEMBLER stops deriving principalKind 2 fail / 7 pass 2 fail / 7 pass — the two named cases, exactly

What stays GREEN under limb A is the finding restated as a test. The three wire bodies keep answering identically while the envelope is wrong — because the omission has no reachable consequence on these faces, which is precisely why this card is graded LATENT. An ablation that reddened a wire body would mean the grade was wrong, not that the pin was strong.

What stays GREEN under limb B is the other half. The two key-set equalities survive, because the reference envelope is built from the SAME assembler and face and reference lose the field together. Those cases assert agreement between this face and the shared assembler, not the presence of a named field; limb A is the limb that proves they bite. Predicting that in advance is what separates the two limbs from one limb run twice.

Source-vs-dist, proven positively rather than asserted. packages/plugins/plugin-hono-server/vitest.config.ts aliases @objectstack/core to ../../core/src/index.ts, so limb B mutated core SOURCE with packages/core/dist/ left un-rebuilt and still holding the unmutated expression (verified by grep at mutation time, 1 hit of the built form). The suite went RED ⇒ it resolves core to SOURCE. A GREEN limb B would have proved it was reading dist/ and voided the whole limb.

6. Verification — all on the final head f457224b9dd

  • Gate union: 55 of 55 families, taken from the Reconciliation line of node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack and harvested with --commands. The 44 bullet lines under "Local gates for this card" are the MATCHED block alone — the script's own reconciliation says that harvest is short by the 6 convention-triggered and 7 declared-whole-tree families. Exit codes captured before any pipe; 2 read as NOT WIRED and 3 as PREREQUISITE NOT MET, never as a pass. Re-derived after the change set was final.
  • The tree was behind origin/main at first derivation (STALE TREE banner); origin/main was merged in (never rebased) and the union re-derived on the merged head.
  • check:type-check-debt needs the workspace closure built, so pnpm exec turbo run build over ./packages/* + ./packages/*/* ran first — 71/71, exit 0.
  • Tests: @objectstack/plugin-hono-server 22 files / 247 assertions green, including the 6 pre-existing current-user suites; @objectstack/hono 2 files / 74 green (it aliases this package to SOURCE, so it exercises the change).
  • Typecheck: @objectstack/plugin-hono-server green, and the new test file was confirmed INSIDE the type-checked program via tsc -p tsconfig.test.json --listFiles (1 hit for the test file, 1 for the changed source, 0 errors) rather than inferred from a green script.
  • Downstream direction: makeExecutionContextResolver has ZERO in-repo consumers outside its own module (only hono-plugin.ts calls registerCurrentUserEndpoints), measured by repo-wide grep across packages/, examples/ and apps/.
  • Lint: the full repo sweep, pnpm lint = eslint . --no-inline-config, green in 88s under the shared verify lock (a shared-box figure, not an idle-box one) — no narrowing claimed or needed.
  • Control-character self-scan over the changed files: clean.

7. Clause ② (契約複審) — declared, from the DELIVERED diff

Mechanical / path limb — YES. packages/spec/src/** is not touched and the three endpoint BODIES are byte-identical, so nothing new crosses the wire. But makeExecutionContextResolver IS published (export * from './current-user-endpoints' on the package index), and this diff changes its published shape twice: the declared return type narrows from any to ExecutionContext | undefined, and the returned envelope gains principalKind while dropping tabPermissions when its value is undefined. In-repo consumers outside the module measure ZERO; consumers outside this repo cannot be measured from here, and the serverless host path composes pieces of this module directly (cloud#924). A narrowing on a published type with unmeasurable external consumers is not a clear call, so it is graded yes.

Non-mechanizable conformance limb — YES. The measured answer is that no verdict changes: isAgent was false for undefined and is false for 'human', the 'agent' arm is unreachable on this face, and the wire bodies are pinned identical. So this is a yes under the doctrine rather than under the measurement — the call is not clear, the faces are /auth/me/permissions and /me/apps, and the card carries security. A false yes self-corrects in a review round; a false no ships.

needs:contract-review applied to the card AND this PR together, with both label sets read back.

Changeset level. minor, not patch: the pr-automation "WHICH LEVEL" rule keeps patch for a fix( that changes no public surface, and this one does change a published package's public surface. No BREAKING banner and no ADR-0087 disposition are claimed — the runtime behaviour is unchanged on every measured path, and grading the type narrowing as breaking is a judgement left to the contract reviewer rather than pre-empted here.

8. Handed back, not acted on

None. Nothing out of scope was found that carried its own measurement.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N


Generated by Claude Code

…ionContext through the shared assembler

`makeExecutionContextResolver` built the envelope for `/auth/me/permissions`,
`/auth/me/localization` and `/me/apps` as a hand-rolled object literal cast
`as any` — sitting beside `assemble-execution-context.ts`, the module whose whole
purpose is to make a partial envelope unrepresentable by CLOSING the field set
with a type. It omitted six fields of that closed set: `principalKind`,
`onBehalfOf`, `audience`, `accessToken`, `authGate` and `oauthScopes`.

That is the #6071 drift class exactly — a field exists on `ExecutionContext`,
one transport carries it, another silently does not.

The resolver now calls `assembleExecutionContext`, the fail-closed default entry
(#6216 Option A), and passes every per-face divergence explicitly instead of
omitting it: no OAuth door, no localization (the endpoint reads that cascade
itself since #15387), no session bearer, no auth gate. The declared return type
narrows from `any` to `ExecutionContext | undefined`, so a field added to
`ExecutionContext` from here on fails to compile until this face decides it.

All three faces answer byte-identical bodies. `principalKind` becomes `'human'`,
which its only reachable reader on these routes — the security plugin's
`context?.principalKind === 'agent'` — already treated an absent value as.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json f50c394da55846da8d38f1e1efadbc24faa26ce2packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5cde623e85ab2811e788d57b3f81fc2704c88fc5 — the merge of head 8fa249a15becdc887f20dc10770528a275e163ac into base f50c394da55846da8d38f1e1efadbc24faa26ce2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5cde623e85ab2811e788d57b3f81fc2704c88fc5 && git checkout 5cde623e85ab2811e788d57b3f81fc2704c88fc5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f50c394da55846da8d38f1e1efadbc24faa26ce2 8fa249a15becdc887f20dc10770528a275e163ac && git checkout -B drift-repro f50c394da55846da8d38f1e1efadbc24faa26ce2 && git merge --no-ff 8fa249a15becdc887f20dc10770528a275e163ac

node scripts/docs-audit/affected-docs.mjs --json f50c394da55846da8d38f1e1efadbc24faa26ce2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@os-litant os-litant left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contract review (clause ②) · head f457224b9dd

Verdict: CHANGES REQUESTED — on exactly one item, the changeset declaration (section ② below). Every other leg was re-derived from the delivered diff and reads PASS; none of them needs a code change beyond the one line the changeset fix itself requires.

Posted as a COMMENT review: GitHub refuses APPROVE on a PR authored under this same account, and an agent seat never submits an approving review in any case. Governed-surface check: the three delivered files (.changeset/lucky-poems-invite.md, the new test, packages/plugins/plugin-hono-server/src/current-user-endpoints.ts) touch no governed surface. Nothing here strips or clears a label.

Implemented-by: claude/issue-15747-execution-context-envelope   (mode:subagent — the branch is the identity)
Reviewed-by:    session_01D47qPfEWVPmhguWgBZCi5N

The two identities differ, so this is not a SELF-REVIEW under the C4 rule.

Tier fuse, read before judging (contract-review reference, 降档保险丝): the two sanctioned readings for this turn — get_session.last_served_model and the harness-stamped model field on this turn's own transcript records — agree with each other and do not equal CONTRACT_REVIEW_TIER (single source: scripts/pm/dispatch-gates.mjs). Under that reading this seat may not self-clear needs:contract-review. The verdict below is CHANGES REQUESTED — the conservative direction — and it rests on a ratified ADR rule plus a gate run, so no clearing depends on it. The PASS readings on the other legs are handed to the at-tier re-review of the patch round as measurements to re-verify, not as a clearing.


① Derived judgments — both clause-② limbs, from the DELIVERED diff

Mechanical / path limb — YES, upheld. The dev's characterisation of the published-shape change is accurate and complete:

  • makeExecutionContextResolver is published: export * from './current-user-endpoints' (src/index.ts:4); @objectstack/plugin-hono-server 17.3.0 carries no private, and exports["."].types is ./dist/index.d.ts.
  • Shape change 1 — the type: the resolver the function returns narrows from (c: any) => Promise<any> to (c: any) => Promise<ExecutionContext | undefined>. Note the exported function itself carries no return annotation at either rev; the annotation moved on the inner closure resolveCtx (:576), and the exported symbol's declaration is inferred from it. That detail decides the gate outcome in ② below.
  • Shape change 2 — the envelope: +principalKind: 'human'; −tabPermissions only when resolveUserAuthzGrants merged none — packages/core/src/security/resolve-authz-context.ts:977 assigns grants.tabPermissions only when non-empty, and tabPermissions sits in the closed set (assemble-execution-context.ts:129), so a defined value is carried. No former key is lost and nothing new leaks in from grants: entryFields() reads exactly the ten authz fields the deleted literal copied (positions, permissions, systemPermissions, userId, tenantId, email, tabPermissions, posture, org_user_ids, accessible_org_ids) plus isSystem: false; every other closed-set field is undefined on this face and dropped by emit() (:264-271).
  • Wire bodies: pinned goldens in the new suite; all six required checks green on this head.
  • Is narrowing anyExecutionContext | undefined breaking for an external consumer? Yes — at compile time, and real rather than theoretical. The repo has already ruled on precisely this class: ADR-0087, addendum 2026-08-30 (#13080): "any is assignable to everything and admits every property read, so a consumer's code really can stop compiling." Concretely here: const ctx = await resolve(c); ctx.userId stops compiling under strictNullChecks (the undefined arm), and any consumer that leaned on any to read undeclared members or to assign the result where a different type is expected stops compiling. Runtime is unchanged on every path. Whether any such consumer exists is NOT MEASURED (below).

Non-mechanizable conformance limb — YES, upheld. The "yes under the doctrine rather than under the measurement" posture is correct, and the measurement behind it holds: the one reachable reader is const isAgent = context?.principalKind === 'agent' (security-plugin.ts:4696), false for undefined and for 'human' alike; the memo-key change from null to 'human' (permissionSetMemoKey, :4625-4634) cannot cross requests because permissionSetMemo is a WeakMap keyed on the context object; the 'agent' arm is absent from this face's code path (next section). No principal's verdict changes on any measured path.

The LATENT chain — the last link, verified

  • makeExecutionContextResolver never calls resolveExecutionContext — holds. Function body at head (:563-684) read in full: session via api.getSession({ headers }) (:585), grants via resolveUserAuthzGrants (:603), assembly via assembleExecutionContext (:662) with oauth: undefined literally (:673). Grep of the whole file for resolveExecutionContext|acceptOAuthAccessToken|verifyMcpAccessToken → one hit, the comment at :639. Control on the same file and pattern form (discriminates on "does the grep reach this file and match", not on the OAuth symbols): resolveUserAuthzGrants → 3 hits. The @objectstack/core import list is assembleExecutionContext, IDataEngine, resolveLocalizationContext, resolveUserAuthzGrants; resolveExecutionContext lives in @objectstack/runtime and is not imported at all.
  • Assembler: const agent = !anonymous && oauth?.clientId ? oauth : undefined (:293), principalKind: agent ? 'agent' : anonymous ? 'guest' : 'human' (:316); assembleExecutionContext answers undefined without a userId (:366), so 'guest' is unreachable here too.
  • acceptOAuthAccessToken repo-wide (non-test, non-dist): one production setter, packages/runtime/src/http-dispatcher.ts:565 (gated on the /mcp regex); the remaining hits are the option declaration (resolve-execution-context.ts:67), its reader (:116) and four comments. oauthPrincipal is assigned only inside the if (opts.acceptOAuthAccessToken) block (:116-135) and is what reaches the assembler as oauth: (:221).
  • #15859 (still open, head 4538d6289d5 on base 59953d5 — the state the dev measured against): its OAuth-gate site edits the regex on that one setter, which stays an option of a function this resolver does not call. Taken on the dev's measurement; state confirmed unchanged.

emit() drops undefined — no reachable consumer keys on PRESENCE

  • hasOwnProperty in security-plugin.ts: :1651 (row, field) and :2592 (r, 'owner_id') — data rows, as stated. :4618 is the comment explaining why the memo key deliberately does not JSON.stringify(context). No ' in context, Object.keys(context, Object.hasOwn, Reflect.has anywhere in the file.
  • The complete read set on the reachable path: the ISecurityService facade at :1526 is a pure pass-through → resolvePermissionSetsForContext (:4652) → memo key (positions, permissions, principalKind ?? null, userId ? 1 : 0) → unmemoized body (:4685-4752: context?.positions ?? [], context?.permissions ?? [], context?.principalKind === 'agent', context?.userId) → dbLoaderForContextcallerOrganizationId (:906, organizationId ?? tenantId). All value-based.
  • Handlers in current-user-endpoints.ts: execCtx.userId / tenantId / positions ?? [] (:981-983), execCtx.systemPermissions ?? [] (:1187), (execCtx as any).tabPermissions ?? {} (:1188). Grep for Object.keys(execCtx|JSON.stringify(execCtx|...execCtx|in execCtx|hasOwn → 0; control on the same file (execCtx. reads) → hits.

⇒ Dropping tabPermissions when undefined is a property of the consumers, not a coincidence of the tested inputs.

② Semver level — the finding

Verdict: breaking, with the disposition not-required (type-surface-only …#makeExecutionContextResolver). The delivered minor changeset with no **BREAKING** token and no marker is the exact shape ADR-0087's addendum names as a counter-example.

  • ADR-0087, addendum 2026-08-30 (#13080), "the sixth category": "A published TYPE-surface narrowing — a published SDK method whose declared return moves off any onto the contract it always answered — declares **BREAKING** truthfully." Then: "#8140, #11925, #12034 and #12104 are all … return-type narrowings, all shipped minor, all recording the no-entry disposition in prose, none carrying the token … a COUNTER-EXAMPLE, not a precedent" and "⛔ Dropping the token is no longer an available exit."
  • This diff is that class on all four predicates: (1) the package publishes; (2) packages/spec/** untouched; (3) no *.zod.ts, no packages/spec/src/contracts/**, no object definition moved; (4) erased at the merge base, concrete at HEAD — with the one wrinkle the gate itself reports below.
  • Gate output read, not guessed (scripts/check-adr-0087-registration.mjs --base origin/main --head HEAD, in a throwaway scratch worktree at f457224b9dd; two local probe commits, never pushed; worktree removed afterwards; refs/pr/15891 unchanged at f457224b9dd):
    • As delivered → exit 0: ✓ … this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen). The gate is silent because nothing was declared — that silence is the erosion the addendum describes, not a pass.
    • Probe A**BREAKING** token + <!-- adr-0087: not-required (type-surface-only packages/plugins/plugin-hono-server/src/current-user-endpoints.ts#makeExecutionContextResolver) … -->, code untouched → exit 1, refused on predicate 4: "makeExecutionContextResolver, which carries NO return annotation is still UNANNOTATED. This category is for a surface that MOVED OFF an erased type." The gate is a source-text reading of the named symbol; the narrowing sits on the inner closure, so the exported symbol looks unmoved.
    • Probe B — probe A plus one line, an explicit annotation on the exported function: export function makeExecutionContextResolver(ctx: CurrentUserEndpointsContext): (c: any) => Promise<ExecutionContext | undefined> { → exit 0: [BREAKING] not-required (type-surface-only) -- verified: …#makeExecutionContextResolver (unannotated -> (c: any)).
  • The bump level itself stays minor: scripts/check-changeset-no-major.mjs refuses a major bump (a breaking change does not burn a major version in the launch window). The live in-repo instance to copy is .changeset/driver-memory-update-upsert-honest-types.md"BREAKING for TypeScript consumers — a published TYPE-surface narrowing, the shape ADR-0087's 2026-08-30 addendum names … shipped as minor under the launch-window convention."
  • Why this category and not another: no-migration-prescription is the answer only when type-surface-only is unclaimable (the driver-memory instance touched packages/spec/**) and it is refused when the body carries a FROM/TO block; runtime-interface-only inherits that same refusal; registered / already-registered have no entry to name — nothing exists for objectstack migrate meta to rewrite, and a fabricated id would be false data in the one ledger the mechanism keeps true.

Required for the patch round (all in the two delivered files):

  1. .changeset/lucky-poems-invite.md: keep minor; add the **BREAKING** for TypeScript consumers … sentence stating FROM → TO ((c: any) => Promise<any>(c: any) => Promise<ExecutionContext | undefined>) and the one-line consumer fix (narrow the undefined arm; type any member previously read through any).
  2. Add the marker: <!-- adr-0087: not-required (type-surface-only packages/plugins/plugin-hono-server/src/current-user-endpoints.ts#makeExecutionContextResolver) <why> -->.
  3. Add the explicit return annotation on makeExecutionContextResolver (one line; ExecutionContext is already imported as a type in the file). Without it the gate refuses the claim (probe A); with it the gate verifies it (probe B). The annotation makes the published declaration explicit rather than inferred and changes nothing else, so it does not re-open the mechanical limb.
  4. Re-run pnpm check:adr-0087-registration and the gate union on the new head; the declared-breaking changeset also makes the CI step Require an ADR-0087 disposition on a declared-breaking changeset load-bearing.

③ Boundary flags

  • The one flag the dev raised — the changeset level, explicitly left to this review — is answered in ②.
  • open_questions: [], out_of_scope_findings: [] — nothing else to dispose.

NOT MEASURED (reported as such, never as a pass)

  • External consumers of makeExecutionContextResolver, including the serverless host composition named in cloud#924: the cloud repository is not attached to this session, and GitHub code search's must-hit control for that repository returned 0 with incomplete_results: true, so the org-wide search result (1 hit, this file) is not a reading for cloud. The objectui sibling checkout greps 0 for the symbol, and that one is a reading: two comment hits on current-user-endpoints.ts in the same sweep prove the grep reaches it; objectui carries no dependency on this package.
  • The emitted dist/index.d.ts was not built here; the before/after declaration shapes above follow from how declarations are emitted for an unannotated export, not from a built artifact.

Re-derived vs taken on trust

  • Re-derived at the PR head: every code fact cited above, the gate runs, the in-repo consumer count (zero outside the module; control registerCurrentUserEndpointshono-plugin.ts plus six suites), and the C4 identity pair.
  • Taken on trust: both ablation limbs and their on-disk mutation proofs, the 55/55 gate union from the Reconciliation line, the source-vs-dist proof, the before-state 11-key envelope (consistent with the deleted literal by inspection), and the #15859 interaction measurement (its state confirmed unchanged).

Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Seat correction: the tier fuse in the review above misfired — the review WAS at CONTRACT_REVIEW_TIER

domain:cli execution seat (#6024). The review states its tier reading came back below CONTRACT_REVIEW_TIER and therefore implies no clearing. ⛔ That reading is wrong, and the verdict is at tier. Correcting it here so the record is not left understating its own authority.

What happened: the reviewer read /root/.claude/projects/…/5c09f02c-…jsonl — the dispatching session's transcript — instead of its own subagent transcript. It then correctly reported that that file carries no claude-fable-5-1 stamp. It does not: that file is this seat, and this seat is claude-opus-5. The reviewer diagnosed the parent's tier and attributed it to itself.

Measured by this seat, on assistant records only (harness stamps, not Agent-tool inputs — and note those are different literals: the input alias is "model":"fable", the harness stamp is "model":"claude-fable-5-1"; the reviewer's own file contains 0 of the former and 76 of the latter):

transcript assistant-record stamps
this review (agent-a65070dfef24a2de3) claude-fable-5-1 × 76, nothing else
the four sibling contract reviews claude-fable-5-1 × 98 / 138 / 90 / 113, nothing else
control — an os-dev agent dispatched without the tier override claude-opus-5 × 253
the parent session the reviewer actually read claude-opus-5 × 2401

CONTRACT_REVIEW_TIER = 'claude-fable-5-1' (scripts/pm/dispatch-gates.mjs:8659). The control discriminates, so this is a reading rather than a grep that matches everything.

The verdict stands at tier. CHANGES REQUESTED is a tier verdict, and needs:contract-review correctly remains on both carriers (this PR and card #15747) until the patch round clears it.

⭐ Worth keeping despite the misfire: the reviewer was right to refuse to clear on a self-description, and right to post the conservative verdict anyway rather than stall. The protocol's rule — that a job's own account of its tier is not a reading — is exactly what caught four sibling reviews' self-reports and forced this seat to measure them independently. The rule worked; only the file path was wrong.

The finding itself is correct and is not softened by any of the above

minor without a **BREAKING** declaration is not an available exit here, and the reviewer did not infer that — it read the governing text: the ADR-0087 addendum of 2026-08-30 (#13080) names minor-without-**BREAKING** return-type narrowings as "a COUNTER-EXAMPLE, not a precedent" and says dropping the token "is no longer an available exit."

Narrowing (c: any) => Promise<any>Promise<ExecutionContext | undefined> is breaking for an external TypeScript consumer at compile time — the undefined arm under strictNullChecks, and undeclared member reads. Runtime is unchanged, which is why the bump stays minor; check-changeset-no-major.mjs forbids major outright.

And the fix was gate-probed before being asked for, which is what makes this actionable rather than a suggestion:

probe result
as delivered exit 0, "adds no declared-breaking changeset"silence, not a pass
A — BREAKING token + marker, code untouched exit 1: predicate 4 refuses, the exported symbol "carries NO return annotation … still UNANNOTATED"
B — A plus a one-line explicit return annotation exit 0, verified

⇒ The patch round needs three things, not two: the BREAKING sentence with FROM→TO and the consumer fix; the marker not-required (type-surface-only packages/plugins/plugin-hono-server/src/current-user-endpoints.ts#makeExecutionContextResolver); and the one-line explicit return annotation on the exported function — without which the marker itself is refused. Shape to copy: .changeset/driver-memory-update-upsert-honest-types.md.

Everything else in the review verified PASS: both clause-② limbs upheld; the LATENT chain's last link confirmed (the resolver body at :563-684 never calls resolveExecutionContext, the only file match being a comment at :639, with resolveUserAuthzGrants → 3 hits as the control); no presence-keying anywhere in the reachable read set, so emit() dropping undefined-valued keys is safe here rather than coincidentally safe; C4 pair correct (Implemented-by: branch ≠ Reviewed-by: session — not SELF-REVIEW); no governed surface touched; all six required CI checks green.

NOT MEASURED, declared rather than passed: external consumers including cloud#924 — that repo is not attached, and GitHub code search's must-hit control returned 0 with incomplete_results: true, so the org-wide single hit is not a reading. The objectui sibling checkout is 0 consumers and that one is a reading (its control hit).

The implementing agent is being sent back for the patch round.


Generated by Claude Code

…declare the type-surface narrowing as BREAKING

Contract review graded the return-type narrowing breaking, and it is: `any` is
assignable to everything and admits every property read, so an external
TypeScript consumer's code really can stop compiling on the `undefined` arm
under `strictNullChecks` and on reads of members `ExecutionContext` does not
declare. The bump stays `minor` — `check-changeset-no-major` refuses `major`
during the launch window, so the BREAKING banner and the ADR-0087 disposition
are the carriers, not the level.

The ADR-0087 addendum of 2026-08-30 (#13080) names `minor`-without-`BREAKING`
return-type narrowings as a counter-example rather than a precedent, and closes
dropping the token as an exit. The changeset now carries the banner with the
FROM/TO shape and the consumer's fix, plus the `type-surface-only` disposition.

The one code change is what that disposition's fourth predicate requires: the
narrowing lived only on the inner closure, leaving the EXPORTED symbol
unannotated at both revisions, so the marker itself was refused. The exported
`makeExecutionContextResolver` now declares
`(c: any) => Promise<ExecutionContext | undefined>`.

No behaviour change: all three current-user faces answer the same bodies, and
their goldens are untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N

@os-litant os-litant left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contract re-review (clause ②, scoped) · head 31431321cad

Verdict: CLEARED. The one item the prior review (#pullrequestreview-5121417846) blocked on — the changeset declaration — is resolved on this head, and the patch round changed nothing else. Every leg that review verified PASS is taken as settled and was not reopened. Clause ② may be stripped by the seat; nothing here strips or clears a label, and no merge, ready flip, enqueue or auto-merge is touched.

Posted as a COMMENT: GitHub refuses APPROVE on a PR authored under this same account, and an agent seat never submits an approving review in any case. Governed-surface check re-confirmed on this head: the three delivered files (.changeset/lucky-poems-invite.md, the new test, packages/plugins/plugin-hono-server/src/current-user-endpoints.ts) touch no governed surface.

Tree freshness — confirmed before any gate was believed

  • origin/main was fetched at the start (b31ebfe), and the PR head as pushed is 8 commits behind it (its merge parent ad0b3e7 is the merge base). A freshness re-read after the first gate run showed origin/main had moved again, to f9a3c32, so the gate was run a second time on a re-merged tree.
  • Both gate scripts are blob-identical at the PR head and at origin/main (check-adr-0087-registration.mjs = 8e9cbd0d…, check-changeset-no-major.mjs = 76eeeddd…), so the instrument is not stale in the dimension under test. The one commit that moved main during this review touched neither script.
  • The final gate reading was taken in a scratch worktree on a local, never-pushed merge of the PR head with origin/main @ f9a3c32: git merge-base --is-ancestor origin/main HEAD → 0 before and after the run, and git diff --stat origin/main HEAD = exactly the PR's three files. The same gate was also run against the PR head as pushed; the verdict is identical.

The gate, re-run here (not inherited)

node scripts/check-adr-0087-registration.mjs --self-test                      → exit 0  (325 assertions)
node scripts/check-adr-0087-registration.mjs --base origin/main --head HEAD   → exit 0
✓ check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.
    .changeset/lucky-poems-invite.md  [BREAKING]  not-required (type-surface-only) -- verified: packages/plugins/plugin-hono-server/src/current-user-endpoints.ts#makeExecutionContextResolver (unannotated -> (c: any))
node scripts/check-changeset-no-major.mjs --base origin/main --head HEAD      → exit 0
✓ This diff introduces no `major` bump.

Exit codes captured before any pipe. Controls in the same worktree, each a local probe commit restored afterwards (final tree: 0 dirty, source blob 6512f7a6… = the PR head's):

control expected measured
token-absent: gate against the OLD head f457224b9dd silence, not a pass exit 0, adds no declared-breaking changeset (1 non-breaking changeset(s) seen) — a different reading, so the instrument discriminates
probe R: exported return annotation stripped (file restored to old-head content; 1 old signature / 0 new, on disk) refuse on predicate 4 exit 1, [predicate 4: narrowed-from-erased] is false at HEAD: … is still UNANNOTATED
probe M: marker line deleted, **BREAKING** kept (0 markers / 1 banner, on disk) refuse for silence exit 1, declares a breaking change (BREAKING) but no adr-0087: disposition marker
probe X: minormajor refuse exit 1, ⛔ This PR introduces changeset(s) that declare a major bump

① The BREAKING sentence — the FROM claim checked at both revisions, not accepted from prose

  • Merge base ad0b3e7: export function makeExecutionContextResolver(ctx: CurrentUserEndpointsContext) { (:558, no return annotation); inner closure const resolveCtx = async (c: any): Promise<any | undefined> => (:571); envelope } as any; (:630); return resolveCtx; (:635). So the FROM text (ctx: CurrentUserEndpointsContext) => (c: any) => Promise<any | undefined> is literally what a declaration emit produces for that shape, and the as any cast is real.
  • "In practice any" — measured, not argued: a standalone reduction of the three shapes (base / f457224b9dd / this head) under TypeScript 6.0.2 (repo pins ^6.0.3) with --declaration emits (c: any) => Promise<any | undefined> for the base shape, and a --strict consumer of it compiles ctx.userId and ctx.notDeclared with no narrowing and no error — the receiver behaves as any. The same consumer against the f457224b9dd and head shapes fails on exactly the two arms the banner names: TS18048 'ctx' is possibly 'undefined' and TS2339 Property 'notDeclared' does not exist on type 'ExecutionContext'. The banner's consumer fix is the compiler's own message.
  • f457224b9dd already inferred (c: any) => Promise<ExecutionContext | undefined> for the export; the annotation on this head makes that declaration explicit and changes nothing else, so the mechanical limb is not reopened.
  • "A consumer who only calls registerCurrentUserEndpoints sees no change" holds: the whole PR diff (-U0, merge base → head) touches exactly one exported declaration, makeExecutionContextResolver. registerCurrentUserEndpoints(options: RegisterCurrentUserEndpointsOptions): boolean is unchanged, and the withRequestContext helper that carries ReturnType<typeof makeExecutionContextResolver> is a local const inside it, not an export.

② The marker — category and vocabulary

  • Present, well-formed, with its reason: <!-- adr-0087: not-required (type-surface-only packages/plugins/plugin-hono-server/src/current-user-endpoints.ts#makeExecutionContextResolver) … -->.
  • type-surface-only is the right category. ADR-0087 addendum 2026-08-30 (#13080) defines it as "a published SDK method whose declared return moves off any onto the contract it always answered", with four predicates the gate checks by name (published, no-spec-diff, no-metadata-surface-diff, narrowed-from-erased): the package publishes (17.3.0, no private), packages/spec/** is untouched, no shape surface moved, and predicate 4 verified as unannotated -> (c: any). The other categories stay closed for the reasons the prior review gave.
  • Judged from the gate's CATEGORIES const (scripts/check-adr-0087-registration.mjs:460-466): unpublished, already-registered, no-migration-prescription, runtime-interface-only, type-surface-only — five not-required categories plus the registered form, the six-form set. The header docblock (:59-66) lists only four not-required forms and omits this one — stale, #15915 — and was not used to judge the category. The gate's own vocabulary-drift check (#8299, CATEGORIES ↔ ADR in both directions) runs on the real assertInputs() path, which refuses to report a verdict at all if the two sets drift; a verdict was reported, and the 325-assertion self-test passed.

③ The annotation — and nothing else

  • Patch-round commit 7ba5f7c (parent f457224b9dd) changes two files: the changeset, and current-user-endpoints.ts +3/−1 — the one-line signature becoming the three-line annotated one. That is the entire code delta.
  • The head 31431321cad is a merge of 7ba5f7c with origin/main @ ad0b3e7: git show --remerge-diff on it is empty (no evil-merge content), and its diff against its first parent under packages/plugins/plugin-hono-server/ and .changeset/ is only the three changesets main gained. The new test file is byte-identical between f457224b9dd and this head. ExecutionContext was already imported as a type (:52), so the annotation adds no import.

Bump level — minor stays correct, and check-changeset-no-major is doing real work

  • pr-automation.yml WHICH LEVEL (:667 ff., maintainer ruling 2026-09-04 on #15294): a fix( that changes a published package's public surface is minor; during the launch window major stays refused and breaking-ness is carried by the BREAKING banner plus the ADR-0087 disposition. Both carriers are now present on the changeset.
  • The refusal is armed, not cited as cover: the guard stands down only in Changesets pre-mode, and .changeset/pre.json does not exist at this head or on origin/main (--list reads back mode: (absent or unreadable)); probe X above proved the enforce branch fires. It is wired live at pr-automation.yml:942 against $MERGE_BASE.

CI on this head (read, not reopened)

Check Changeset — the job hosting Require an ADR-0087 disposition on a declared-breaking changeset, now load-bearing — is success, as are all four Type Check jobs, Build Core, and every other completed job; Lint & Repo Gates and Test Core (1/6) were still in progress at read time.

Tier fuse — own file, read directly

This seat's own transcript, …/subagents/agent-ab984de2a074d02c2.jsonl (the file that carries this review's task prompt — not the dispatching session's transcript, which is what the prior review misread), parsed record by record: 48 of 48 type:"assistant" records carry a message.model stamp equal to CONTRACT_REVIEW_TIER (scripts/pm/dispatch-gates.mjs:9852, same value on origin/main and this head). Control, same parser, same run: a developer-seat transcript at 234/234 stamps of a different value; zero occurrences of the Agent-tool input alias literal in this file. This verdict is a tier verdict.

NOT MEASURED (reported as such, never as a pass)

  • External consumers of makeExecutionContextResolver (the serverless host composition, cloud#924): that repository is not attached; unchanged from the prior review.
  • The package's own emitted dist/index.d.ts was not built here; the before/after declaration shapes were measured on a standalone reduction of the three signatures under the same TypeScript major, not on the built artifact.
  • Lint & Repo Gates and Test Core (1/6) on this head: in progress at read time.

Settled, not reopened

Both clause-② limbs, the LATENT chain's last link, the absence of presence-keying in the reachable read set, the C4 independence pair, and the governed-surface check — verified PASS in the prior review and carried forward as settled.


Generated by Claude Code

@os-litant
os-litant marked this pull request as ready for review September 5, 2026 18:29
@os-litant
os-litant enabled auto-merge September 5, 2026 18:29
@os-litant
os-litant added this pull request to the merge queue Sep 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 5, 2026
@os-litant
os-litant added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 6615a02 Sep 5, 2026
34 checks passed
@os-litant
os-litant deleted the claude/issue-15747-execution-context-envelope branch September 5, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants