Skip to content

plan: TypeScript SDK MCP authorization lifecycle - #1687

Merged
samuv merged 4 commits into
mainfrom
plan/sdk-mcp-authorization-lifecycle
Sep 18, 2026
Merged

samuv merged 4 commits into
mainfrom
plan/sdk-mcp-authorization-lifecycle

Conversation

@samuv

@samuv samuv commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Stage

Plan / Interface

Summary

  • define a session-bound TypeScript SDK MCP authorization lifecycle
  • make authorization-parked Run completion a normal typed handoff
  • specify lazy, single-consumption recheck/cancel flows and exact continuation controls
  • define chained authorization, transport disconnect, recovery, and HTTP envelope behavior
  • treat a replayed exact-run authorization park as an attachment terminal
  • define flow termination, responder cancellation, late-verdict suppression, and post-terminal control behavior
  • state the correlation boundaries the current event wire can actually prove
  • correct the Converse terminal-shape documentation without changing protobuf fields
  • record ADR 0348 as the narrow ergonomic-resource exception to ADR 0304

Human decisions

The directing user approved the public object model, initial Run handoff, lazy dispatch,
automatic-control options, bounded recovery contract, attachment terminal behavior,
termination matrix, and correlation boundary recorded in the plan.

Verification

  • bash .claude/skills/to-acceptance-plan/scripts/check-acceptance-plan.sh docs/acceptance/sdk-mcp-authorization-lifecycle.md — 27 ACs, passed
  • bash .claude/skills/to-acceptance-plan/scripts/check-acceptance-plan-test.sh — passed
  • CGO_ENABLED=0 task docs — passed
  • advisory devil's-advocate review folded into the contract

Relates to #1469

@samuv samuv self-assigned this Sep 17, 2026
@samuv
samuv force-pushed the plan/sdk-mcp-authorization-lifecycle branch from 667078e to deb4716 Compare September 18, 2026 00:13
@samuv
samuv changed the base branch from main to impl/sdk-http-wkt-json-compatibility September 18, 2026 00:13
@samuv
samuv added this pull request to stack #1693 September 18, 2026 00:13
@samuv
samuv force-pushed the plan/sdk-mcp-authorization-lifecycle branch from deb4716 to 9741773 Compare September 18, 2026 08:33
Base automatically changed from impl/sdk-http-wkt-json-compatibility to main September 18, 2026 08:54
@samuv
samuv force-pushed the plan/sdk-mcp-authorization-lifecycle branch from 9741773 to 6ef30bc Compare September 18, 2026 08:54

@jhrozek jhrozek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought this was fine. The machine insisted on bringing these up:

1. Medium — attachment recovery does not handle the new parked terminal shape

Plan: docs/acceptance/sdk-mcp-authorization-lifecycle.md:234 ADR: docs/adr/0348-typescript-sdk-mcp-authorization-lifecycle.md:184

The plan recommends explicit Session.attach(continuationRunId) as a recovery option. However, the existing attachment:

stops iteration only on result ( sdk/typescript/src/watch.ts:591 );
marks the run no longer live only on result ( sdk/typescript/src/watch.ts:671 ).

A continuation can instead finish by parking on another authorization.required , with no result . Attachment can yield that event but remain live === true and keep watching rather than finish.

Requested change: Explicitly choose between extending attachment to recognize authorization parking, or documenting that applications must detect that event and close the attachment themselves. Add a recovery test for disconnect → attach → replay chained authorization park. “Where those existing APIs support it”
leaves this important case unresolved.

2. Medium — flow termination and subsequent control behavior are underspecified

Plan: docs/acceptance/sdk-mcp-authorization-lifecycle.md:230

AC5.2 groups caller abort, deadline, iterator return, transport loss, and client close under “reject or end according to the normalized contract.” It does not specify their observable differences or what happens to outstanding permission responders and later control calls.

For example: while next() waits for an event and an automatic permission responder is pending, the caller returns the iterator. Must the pending read finish normally or reject? Is the responder’s signal aborted? Can its late verdict still dispatch? Can resolveAsk() or cancelContinuation() be called after the
flow ends?

These are public lifecycle decisions, not implementation details. Existing SDK behavior is not a sufficient default: Run currently has a no-op iterator return() ( sdk/typescript/src/run.ts:276 ), whereas attachment closes its resources in finally ( sdk/typescript/src/watch.ts:595 ).

Requested change: Add a small termination matrix specifying:

pending next() / result() outcome;
responder cancellation and late-verdict suppression;
post-terminal manual-control behavior;
registration and transport cleanup.

Pin the race cases with executable tests.

3. Medium — correlation acceptance promises more than the event carries

Plan: docs/acceptance/sdk-mcp-authorization-lifecycle.md:228

AC5.1 says an event from another session, authorization, call, or continuation cannot be accepted as this flow’s correlation. But:

EventCommon  has no session ID ( sdk/typescript/src/events.ts:342 );

authorization payloads contain authorization and call IDs, but no session ID ( sdk/typescript/src/events.ts:153 );
the handle accepts only an authorization ID, so it has no independently known call ID against which to validate the first authoritative frame.

The SDK can enforce session affinity on the request, isolate iterators, compare the authorization ID, and pin the call/run IDs learned from the response. It cannot independently verify every claimed origin from these frames.

Requested change: Separate the guarantees explicitly: session ownership is server-enforced through the session-affined request; authorization ID is checked against the handle; call and continuation IDs are pinned from authoritative frames and checked thereafter. Make the tests assert those actual boundaries
without widening the wire.

Smaller corrections

AC2.1 should specify local rejection of an empty authorization ID, matching the ADR’s “non-empty” requirement ( docs/adr/0348-typescript-sdk-mcp-authorization-lifecycle.md:54 ).
Correct the Converse proto documentation, which still promises closure after terminal result ( contracts/proto/mecatl/v1/harness.proto:100 ). This requires no wire-field change, but the plan should distinguish that from “protobuf: None.”

The empty-run-ID control response, repeated resolution event, and gRPC-versus-HTTP disconnect distinction match the existing server implementation. I would keep those decisions, along with lazy dispatch, separate permission request options, and the explicit no-retry policy.

No files changed; this was a plan and supporting-code review, not an implementation test run.

@samuv

samuv commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review in 2f9a05c5b and 09f76b009:

  • exact-run attachment now treats a valid replayed chained authorization park as terminal and the plan requires a disconnect → attach → replay test;
  • Scenario 5 now includes an explicit termination matrix for next()/result(), iterator return, caller abort, deadline, transport loss, client close, responder/control cancellation, late-verdict suppression, post-terminal controls, and cleanup;
  • correlation now reflects the observable wire boundary: session ownership is server-enforced, the handle authorization ID and original call ID are checked, the repeated original resolution retains those IDs, continuation frames use the pinned run ID, and a chained authorization has its own call ID;
  • AC2.1 now requires local empty-ID rejection;
  • the Converse source comment and regenerated bindings document both terminal shapes without changing protobuf fields;
  • Proposed ADR 0348 now records those decisions and accurately acknowledges the gRPC relay's EOF-on-stranded-permission-ask behavior.

The acceptance checker still reports 27/27 ACs and CGO_ENABLED=0 task docs passes. The executable SDK repairs are being added in the stacked Implementation PR #1692.

Signed-off-by: Samuele Verzi <samu@stacklok.com>
Signed-off-by: Samuele Verzi <samu@stacklok.com>
Signed-off-by: Samuele Verzi <samu@stacklok.com>
Signed-off-by: Samuele Verzi <samu@stacklok.com>
@samuv
samuv force-pushed the plan/sdk-mcp-authorization-lifecycle branch from 09f76b0 to 28868b2 Compare September 18, 2026 14:45
@samuv
samuv merged commit ad1cfe3 into main Sep 18, 2026
36 checks passed
@samuv
samuv deleted the plan/sdk-mcp-authorization-lifecycle branch September 18, 2026 15:26
This was referenced Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants