Skip to content

fix(xaa): address review nits from #1593 & use discoveryState#1721

Draft
pcarleton wants to merge 3 commits intomainfrom
pcarleton/pcp-113-xaa-nits
Draft

fix(xaa): address review nits from #1593 & use discoveryState#1721
pcarleton wants to merge 3 commits intomainfrom
pcarleton/pcp-113-xaa-nits

Conversation

@pcarleton
Copy link
Member

@pcarleton pcarleton commented Mar 20, 2026

Address review nits from PR #1593:

  • Use OAuthTokens type instead of inline return type in exchangeJwtAuthGrant
  • Use parseErrorResponse for proper OAuthError in both token exchange functions
  • Use z.coerce.number() for expires_in in IdJagTokenExchangeResponseSchema to match existing pattern

I also missed in that diff that we didn't end up refactoring to use the merged discoveryState, so this switches over to that.

…s_in

- Replace inline return type with OAuthTokens in exchangeJwtAuthGrant
- Use parseErrorResponse for proper OAuthError in both token exchange functions
- Use z.coerce.number() for expires_in in IdJagTokenExchangeResponseSchema
- Update tests to use real Response objects and check OAuthError type
@changeset-bot
Copy link

changeset-bot bot commented Mar 20, 2026

⚠️ No Changeset found

Latest commit: feeae1e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 20, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1721

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1721

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1721

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1721

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1721

commit: feeae1e

Remove individual saveAuthorizationServerUrl/saveResourceUrl methods
from OAuthClientProvider in favor of adding resourceUrl to
OAuthDiscoveryState. CrossAppAccessProvider now reads both URLs from
the discovery state, eliminating redundant save calls in auth().

The single saveDiscoveryState call is moved after selectResourceURL
so it can include the resolved resource URL.
@pcarleton pcarleton changed the title fix(xaa): address review nits from #1593 fix(xaa): address review nits from #1593 & use discoveryState Mar 20, 2026
CrossAppAccessProvider reads resourceMetadata.resource from the
discovery state instead of a separate resourceUrl field.
selectResourceURL throws if validation fails, so by the time
saveDiscoveryState is called the resource metadata is always valid.

This reverts the needsDiscoveryStateSave flag and restores the
original two inline saveDiscoveryState calls.
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.

1 participant