feat: interactive elicitation (MRTR + legacy) and OAuth 2026-07-28 hardening - #256
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71b374ad4f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| serverName?: string | ||
| ): Promise<void> { | ||
| try { | ||
| await transport.finishAuth?.(response.code, response.iss); |
There was a problem hiding this comment.
Forward callback issuer through recording transport
When MCPORTER_RECORD is set, HTTP transports are wrapped in RecordTransport, whose finishAuth forwarder still accepts only authorizationCode and calls the inner transport with one argument. This new call therefore loses response.iss before it reaches the SDK in recorded OAuth flows, so 2026-07-28 callback issuer validation either fails as missing or is skipped depending on the transport; forward the second argument through the recorder.
Useful? React with 👍 / 👎.
| this.redirectUrlValue = redirectUrl; | ||
| this.logger = logger; | ||
| this.persistence = persistence; | ||
| validateClientMetadataUrl(definition.oauthClientMetadataUrl); |
There was a problem hiding this comment.
Close the callback server on metadata URL validation failure
At this point create() has already bound the callback server with server.listen(...); if a user configures a malformed oauthClientMetadataUrl, this validation throws out of the constructor and the already-listening callback server is never closed, leaving a live handle/port after createOAuthSession rejects. Move this validation before binding or close the server on constructor failure.
Useful? React with 👍 / 👎.
| const invalid = values.find((value) => !choices.includes(value)); | ||
| return invalid | ||
| ? { ok: false, message: `Choose comma-separated values from: ${choices.join(', ')}.` } | ||
| : { ok: true, value: values }; |
There was a problem hiding this comment.
Enforce multi-select min/max item bounds
For multi-select elicitation schemas that declare minItems or maxItems, this branch accepts whatever number of comma-separated values survived filtering as long as each choice is valid. A user can submit zero selections to a minItems: 1 prompt (for example by typing only commas) or too many selections for maxItems, and the server receives content that violates its requested schema instead of being re-prompted.
Useful? React with 👍 / 👎.
Completes the two capability gaps left after the SDK v2 migration (#255):
Elicitation / MRTR interactive input
elicitation/createand the 2026-07-28 MRTRinput_requiredauto-fulfilment path.createRuntimeaccepts anelicitationHandleroverride for programmatic use.OAuth hardening (2026-07-28 authorization requirements)
issand validates it via the SDK before the code is redeemed; mismatches abort with expected-vs-received issuer named.oauthClientMetadataUrlper-server config exposes a URL-based client identity where supported (SDK falls back to DCR). Verified the SDK already derivesapplication_type: nativefor loopback redirects.Proof:
pnpm checkclean; 927 passed / 3 skipped; autoreview clean (0.98) incl. TruffleHog. Part 3 of the MCP 2.0 series (#254, #255). Next: committed dual-era fixture servers + e2e.🤖 Generated with Claude Code