feat(sdk): add MCP authorization lifecycle

[samuv]

Summary

• add a session-bound TypeScript SDK lifecycle for MCP authorization presentation, recheck, cancellation, permission handling, and continuation
• treat an authorization-parked Run as a normal typed handoff, including chained authorization and exact-run continuation controls
• preserve lazy single-consumption semantics, request-option isolation, session affinity, transport parity, and server-owned authorization decisions
• close attachment, flow-termination, correlation, relay-disconnect, and late-verdict races identified during review
• publish the root, Node, and Deno API surface with generated reference, user guidance, examples, and real-wire coverage

Development stage

Implementation of the contract merged through Plan / Interface PR #1687.

Contract linkage

• Work classification: Architectural
• Decision record: ADR 0348
• Acceptance plan: TypeScript SDK MCP authorization lifecycle
• Plan / Interface PR: plan: TypeScript SDK MCP authorization lifecycle #1687 (merged)
• Approved plan head: 09f76b009691dc9cbb73b75ff7e12649b36e64f9
• Current implementation head: c4580cdd280d569ef080328f78d9afa7968628b0
• Lower stacked dependencies: merged PRs fix(sdk): decode daemon HTTP timestamp and duration objects #1686 and plan: TypeScript SDK MCP authorization lifecycle #1687
• Human waiver of spine: the directing user explicitly authorized the sequential implementation stack without waiting for plan: TypeScript SDK MCP authorization lifecycle #1687 to merge

Interface conformance

Interfaces match approved contract: Yes. The #1687 amendment makes exact-run attachment parking terminal, specifies the termination matrix and correlation boundaries, rejects empty authorization IDs locally, and documents the existing relay behavior without changing protobuf fields.

Review response

The changes-requested review on this PR is addressed:

• ADR 0348 acknowledges the authorization relay's disconnect transition.
• The concurrent-flow test starts and consumes independent flows concurrently.
• The parked-Run test immediately creates session.mcpAuthorization(...) on the same session.
• Follow-up review also fixed EOF-before-ask stranding, excluded plan asks from automatic disconnect cancellation, and suppresses automatic verdicts whose asks retire before dispatch.

Relationship to direct MCP onboarding

Merged PR #1689 and this PR have separate authority boundaries. PR #1689 administers host-local direct and global MCP profiles through mecated mcp; this PR consumes authorization.required handoffs from session-scoped ToolHive broker tools. The user guides, SDK TSDoc, generated API reference, and living architecture now state that boundary and link the two workflows.

Verification

• acceptance trace: 27/27 ACs, 0 failures
• TypeScript SDK: 329/329 tests passed before the main rebase
• focused lifecycle/attachment tests passed, including red-then-green race regressions
• post-rebase task sdk:docs:check, task site:build, and git diff --check passed
• GitHub CI is the authoritative full-matrix check for the rebased head

Plain task test and task docs remain host-limited on this macOS 27 machine because its installed TAPI/linker cannot parse the SDK .tbd architecture entries. The equivalent focused and CGO-disabled paths passed during development.

Issue relationship

Closes #1469

Stack

#1686 (merged) → #1687 (merged) → #1692

[kantord]

Fully or partially written by an AI agent (Claude Code /panel-review).

Panel review — PR #1692 (feat(sdk): add MCP authorization lifecycle)

Fixed point: origin/plan/sdk-mcp-authorization-lifecycle (PR base)
Diff: 36 files, +4987/-89, 11 commits

---

Spec — does this implement what was asked?

Source: docs/acceptance/sdk-mcp-authorization-lifecycle.md (27 ACs) + ADR 0348

Traced all 27 ACs against the actual implementation (sdk/typescript/src/{mcp-authorization,run,client,http,rpc-catalog}.ts, internal/adapter/server/grpc.go), not just test presence. The implementation is unusually faithful to the plan's exact wording — every "synchronously," "no compatibility probe until first consumption," and "mutually exclusive single-consumption" requirement was traced and confirmed correct. No missing ACs, no scope creep beyond the in-scope grpc.go fix.

• [advisory · Doc drift] docs/design/IMPLEMENTATION-NOTES.md states continuation permissions route through Session.controls(continuationRunId), but the code calls createRunControls(...) directly (functionally equivalent, same factory — just a wording mismatch).

---

Standards — does this follow project conventions?

Sources read: AGENTS.md, ADR 0348, ADR 0304, ADR 0347

• [important · Undisclosed server-behavior change] internal/adapter/server/grpc.go (relayMCPAuthorizationControl) gained new logic: a plain io.EOF on the control stream while parkedOnAsk now cancels the run (new test: TestMCPAuthorizationGRPCControlEOFCancelsStrandedPermissionContinuation). Before this diff that path did not cancel. This is a genuine change to gRPC relay/cancellation semantics — yet ADR 0348 §8 states explicitly: "No protobuf, Go API, server transition, event, snapshot, persistence, feature, or error code changes." ADR §7 narrates this exact behavior as if pre-existing, but the diff shows it wasn't implemented for the plain-EOF case until now. Either ADR 0348's "no server transition" claim needs correcting (with its own rationale), or this fix needs to be justified as a separate, disclosed change. As written, the ADR and the diff disagree about what "no server change" means — and the PR body's "Interfaces match approved contract: Yes" doesn't surface this.
• [advisory · Unverified doc-lifecycle] task docs / task site:build cleanliness (link-check, config-reference drift) wasn't independently verifiable from the diff alone — worth confirming ran clean, per AGENTS.md's "Changed any Markdown? Run task docs... always."

No git add -A usage, missing Co-Authored-By, or layering violations found.

---

Test adequacy — do the tests independently prove the contract?

Traced all 27 ACs to their named tests independently of the implementation. Most are solid: real discriminated-outcome assertions, real transport call-count/ordering checks, genuine planted malformed-frame cases (7 distinct malformed-authorization cases for AC3.3, 6 grammar-violation cases for AC4.1), and a genuinely real-wire e2e suite for AC5.5/AC6.1 (spawns the real server.NewHarnessServer/NewHTTPHandler over real gRPC-TCP/UDS/HTTP sockets).

• [important · AC5.1 not genuinely concurrent] mcp-authorization-recovery.test.ts:22-52 ("concurrent MCP authorization flows cannot cross consume or correlate") drives the two flows fully sequentially — first completes all three next() calls before second is touched. There's no actual interleaving (e.g. Promise.all([...])) exercising two in-flight requests sharing state at the same time. What's proven is payload-correlation validation, not independence under true concurrent execution as the AC claims.
• [important · AC1.3 partially covered] run.test.ts:154-236 solidly proves non-cancellation, busy-state clearing, and stream closing after a park — but never actually calls session.mcpAuthorization(...) after a park to verify the specific sub-clause "the same Session can immediately create its lifecycle handle." That claim is untested in the post-park context.

No blockers found; AC3.5 (single-consumption, both directions) was specifically checked and is fine.

---

Domain — what do the specialist reviewers say?

Panel: secure-code-reviewer, software-architect, code-duplication-reviewer, library-reuse-reviewer

Ship-blockers

• (none)

Cross-confirmed (1)

• [important · Medium] Byte-identical duplicate runResult() in run.ts:540-550 and mcp-authorization.ts:817-827 — the exact same "terminal result event → RunResult" rule, including a non-obvious usage: event.payload.usage ?? event.usage fallback.
  Sources: code-duplication-reviewer (rated important — trivial one-line export+import fix, do it now), software-architect (rated advisory, bundled into a broader "wait for a 3rd consumer" Rule-of-Three judgement about ask-responder plumbing).
  Judgement call: the two specialists disagree on urgency, not on the fact. Given it's a pure function, the type is already imported cross-file, and the fix is export + one import line, the low-risk/low-cost side favors just fixing it now rather than waiting.

Mechanical fixes (3)

• [important] 4th copy of a byte-identical single-consumption #claim() guard — run.ts:333, team.ts:170, plan.ts:169 (pre-existing), and new mcp-authorization.ts:375. Same business rule (claim-once, throw InvalidStateError), copy-pasted across 4 files. Extract one small createConsumptionClaim-shaped helper. (Sources: code-duplication-reviewer)
• [important] Hand-rolled deadline timer in mcp-authorization.ts:425-449 (#bindRequestLifetime) reinvents AbortSignal.timeout() composed via AbortSignal.any([...]) — the exact idiom already used 3 other places in this same package (http.ts, watch.ts, client.ts). Swap in the native primitive (~6-10 lines removed). One caveat: the manual timer is currently load-bearing for a fake-timers test, and timeoutMs is also independently forwarded to the transport, so there may be two competing enforcement paths on the real transports — not confirmed as an active bug, but worth a second look while touching this. (Sources: library-reuse-reviewer)
• [advisory] grpc.go's new EOF-cancels-stranded-ask branch inlines h.svc.cancelRegisteredRun(...) instead of going through the file's own named strand() closure that exists for exactly this concept (cancel a run stranded on an ask) — two independently-guarded call sites for the same policy invites drift on a future change. Fold both into one helper, e.g. cancelIfStranded(reason). (Sources: software-architect)

Judgement calls (1)

• [important] mcp-authorization.test.ts hand-rolls ~160 lines of its own LifecycleTransport-equivalent fixture (AuthorizationTransport, grpcSession(), httpClient(), etc.) instead of reusing mcp-authorization-control-fixture.ts, the shared fixture this same PR introduced and that its sibling test files (mcp-authorization-controls.test.ts, mcp-authorization-recovery.test.ts) correctly adopted. Already diverged once by accident (per-call header/trailer wiring present in one fake, absent in the other). Migrate onto the shared fixture (net ~-130 to -150 lines), adding header/trailer support to it if genuinely needed. (Sources: code-duplication-reviewer)

Polish (2)

• [advisory] CWE-362 — narrow same-session race in the new grpc.go EOF-cancel branch: parkedOnAsk reflects the last-observed event, so a resolving event landing right at EOF could cause an unnecessary self-cancel of the caller's own run. Same-session/same-caller only, no cross-tenant impact, and it's an explicit, tested, documented trade-off (the alternative — never cancelling — permanently strands the run, which is worse). No action needed; flagged for the record only. (Sources: secure-code-reviewer)
• [advisory] #protocol() error-wrapper helper duplicated once between run.ts:513 and mcp-authorization.ts:730 (3 lines, 2 sites) — below the extraction threshold today; revisit alongside the #claim extraction if a 3rd stateful flow class appears. (Sources: code-duplication-reviewer)

Gaps

• (none — no K8s/CI/IaC surfaces touched; devops-expert not applicable)

---

Summary

• Spec axis: 1 finding (0 blocker, 0 important, 1 advisory)
• Standards axis: 2 findings (0 blocker, 1 important, 1 advisory)
• Test-adequacy axis: 2 findings (0 blocker, 2 important, 0 advisory)
• Domain axis: 0 blocker, 4 important (1 cross-confirmed), 3 advisory

Most important single issue: the Standards finding — ADR 0348 asserts "no server transition" while grpc.go's cancellation semantics for a stranded permission-ask actually changed. Not a functional bug (the new behavior is correct and tested), but it's an accuracy gap in the durable decision record this repo treats as a real contract, and the PR's own "Interfaces match approved contract: Yes" doesn't surface it.

Each axis is orthogonal — none of these mask each other. The implementation itself is faithful to the acceptance plan and the security posture is sound; the open items are documentation accuracy, two test-proof gaps in Scenario 5 (concurrency/recovery), and mechanical duplication cleanup in a PR that adds the 4th copy of an existing pattern.

PANEL: ship_blockers=0 important=7 advisory=5 reviewer_failures=0

[kantord]

Fully or partially written by an AI agent (Claude Code /panel-review).

Full 4-axis panel review posted above: ship_blockers=0, important=7, advisory=5, reviewer_failures=0.

Requesting changes — not because of a functional defect (the implementation is faithful to all 27 ACs and the security posture is sound), but for one accuracy issue and two test-proof gaps worth closing before merge:

1. Standards (important): ADR 0348 §8 states "no ... server transition" changes, but internal/adapter/server/grpc.go's relayMCPAuthorizationControl gained new cancel-on-EOF-while-parked-on-ask behavior that didn't exist before. The behavior itself is correct and tested — the ADR text (or the PR's "Interfaces match approved contract: Yes" framing) should acknowledge it.
2. Test adequacy (important): AC5.1's "concurrent flows" test drives two flows sequentially, not concurrently — it doesn't prove the independence claim it names.
3. Test adequacy (important): AC1.3 never actually calls session.mcpAuthorization(...) after a park to prove "the same Session can immediately create its lifecycle handle."

The remaining 4 important / 5 advisory Domain findings (duplicated runResult(), a 4th copy of the #claim single-consumption guard, a hand-rolled timer duplicating AbortSignal.timeout(), and a test file reinventing a fixture this same PR introduced) are mechanical cleanup, not correctness risks — worth doing while this code is fresh, not necessarily blocking if the team prefers a fast-follow.

[samuv]

Addressed the three changes-requested review points on the current stacked head:

1. ADR 0348 now acknowledges the gRPC relay's cancel-on-disconnect transition for a stranded ordinary permission ask.
2. The concurrent authorization-flow test starts and consumes both flows concurrently, with independent correlation and transport calls.
3. The parked-Run handoff test immediately calls session.mcpAuthorization(...) on the same released session.

The follow-up fixed-point review also found and repaired two related relay races: EOF is remembered when it arrives before a later ordinary ask, and plan-originated asks are excluded from automatic disconnect cancellation. Automatic permission controls now use ask-scoped abort lifetimes so a retired ask cannot dispatch a late verdict.

Local focused validation is green, including 329 SDK tests, SDK lint/typecheck, server adapter tests, and repository lint. The stack is synced at be4d38c5721fb3c61a01261b0d62fa1bbae95e56; CI is running the full matrix.

[samuv]

Rebased this implementation onto current main at 73309dfa3fd56766b01b7eecea39a78873495b0f; the remote and local heads now match at c4580cdd280d569ef080328f78d9afa7968628b0.

I also reviewed the now-merged #1689 against this lifecycle. The features have separate authority boundaries: #1689 manages host-local direct/global MCP profiles and credentials through mecated mcp, while this PR consumes session-scoped ToolHive broker authorization.required handoffs. No implementation or cross-feature test gap was found. The rebased head adds reciprocal user-guide links plus SDK TSDoc, generated API reference, and living-architecture wording so that distinction is explicit.

Focused post-rebase checks passed:

• task sdk:docs:check
• task site:build
• git diff --check

The full matrix is running in CI.

[kantord]

Fully or partially written by an AI agent (Claude Code /panel-review).

Verified all three changes-requested items against the current head (c4580cdd2):

1. ADR 0348 now documents the gRPC cancel-on-disconnect-while-parked-on-ask transition instead of claiming "no server transition."
2. The AC5.1 concurrency test now genuinely overlaps two flows via Promise.all([...]) and asserts maxActiveStreams: 2, rather than driving them sequentially.
3. The AC1.3 test now calls session.mcpAuthorization(...) right after a park to prove the same released session can immediately create its lifecycle handle.

CI is green apart from k8s e2e (kind), which looks unrelated to this diff (no go.mod/go.sum changes anywhere in the PR, and perf is independently failing on main's last two pushes — reads as current CI infra flakiness, not a regression here).

The remaining Domain-axis duplication findings from the panel review (duplicated runResult(), a 4th copy of the #claim single-consumption guard, a hand-rolled timer vs AbortSignal.timeout()) are still open but were flagged as non-blocking mechanical cleanup, not part of the changes-requested reasons — fine as a fast-follow.

Approving.