fix(billing): mint opaque pmth_ signer session per ProviderInstance#403
Merged
Conversation
PymthouseAdapter.mintSignerSession called the SDK 0.4.3 PmtHouseClient.mintSignerSessionForExternalUser, which sets `resource` to the OIDC issuer. Current PymtHouse routes that to signer-JWT exchange (returns another JWT, no opaque `pmth_…` session), so the key-validation front door failed to produce a signer session (HTTP 503) for both legacy seat-bound and subscription-bound keys. Route the mint through the existing opaque-session workaround (upsert user → mint user JWT → token-exchange WITHOUT `resource`), and make it multi-app safe: a per-ProviderInstance adapter exchanges against THAT app's issuer/M2M creds via a new `signerExchange` option, while the global-env adapter continues to use the `PYMTHOUSE_*` env exchange. Additive and backward-compatible: the global path delegates to the same opaque exchange it already used; the per-instance path only engages when an adapter is built with both a client override and signerExchange. - pymthouse-client.ts: extract `exchangeUserJwtForOpaqueSignerSessionWith` (explicit config) + export `mintOpaqueSignerSessionForExternalUser`; `mintSignerSessionForExternalUser` now delegates with the global env. - pymthouse-adapter.ts: add `signerExchange` option; mint via the opaque workaround (per-instance override or global env). - provider-instance.ts: pass the instance's issuer + M2M creds as signerExchange when building a per-instance adapter. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
size/M
📝 WalkthroughWalkthroughThe PR adds explicit per-instance signer-exchange configuration for pymthouse sessions. It introduces a signer-exchange config type and explicit exchange helper, updates signer-session minting to accept explicit client/config inputs, and passes per-instance signer-exchange settings into the billing adapter. ChangesPymthouse signer exchange
Sequence Diagram(s)sequenceDiagram
participant buildAdapterForProviderInstance
participant PymthouseAdapter
participant mintOpaqueSignerSessionForExternalUser
participant PmtHouseClient
participant exchangeUserJwtForOpaqueSignerSessionWith
participant AuthorizationServer
buildAdapterForProviderInstance->>PymthouseAdapter: new PymthouseAdapter({ client, signerExchange })
PymthouseAdapter->>mintOpaqueSignerSessionForExternalUser: mintSignerSession(...)
mintOpaqueSignerSessionForExternalUser->>PmtHouseClient: upsertAppUser() and mintUserAccessToken()
mintOpaqueSignerSessionForExternalUser->>exchangeUserJwtForOpaqueSignerSessionWith: exchange(accessToken, signerExchange)
exchangeUserJwtForOpaqueSignerSessionWith->>AuthorizationServer: load issuerUrl and POST token exchange
AuthorizationServer-->>exchangeUserJwtForOpaqueSignerSessionWith: opaque signer session
exchangeUserJwtForOpaqueSignerSessionWith-->>mintOpaqueSignerSessionForExternalUser: SignerSessionToken
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web-next/src/lib/billing/pymthouse-adapter.ts`:
- Around line 71-74: The PymthouseAdapter currently allows a client override
without a matching signerExchange and then silently falls back to the global
mint path, which can mix provider usage with a different signer session. Update
PymthouseAdapter’s constructor and minting flow to enforce the invariant that
client and signerExchange must be provided together: either validate immediately
in the constructor or throw before the mint step when the pair is incomplete.
Use the PymthouseAdapter and its minting logic around the
clientOverride/signerExchange handling to locate the fix.
In `@apps/web-next/src/lib/pymthouse-client.ts`:
- Around line 97-99: The PmtHouse missing-configuration error in
pymthouse-client should stay classified as a server/provider availability issue
rather than a client error. Update the PmtHouseError thrown for the
PYMTHOUSE_NOT_CONFIGURED_MESSAGE path in the pymthouse-client logic to return
HTTP 503 while keeping the existing pymthouse_required code, so it matches the
behavior of the other PYMTHOUSE_* config readers and callers can handle it
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a39474ef-0d2f-4a65-8ebc-894e2400eab8
📒 Files selected for processing (3)
apps/web-next/src/lib/billing/provider-instance.tsapps/web-next/src/lib/billing/pymthouse-adapter.tsapps/web-next/src/lib/pymthouse-client.ts
Comment on lines
71
to
+74
| constructor(options: PymthouseAdapterOptions = {}) { | ||
| this.clientOverride = options.client; | ||
| this.isConfiguredOverride = options.isConfigured; | ||
| this.signerExchange = options.signerExchange; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed when client and signerExchange are not paired.
The option comment says signerExchange is required with a client override, but Line 227 silently falls back to the global env mint path if it is missing. That can bind usage/spend to one provider instance while minting signer sessions against another. Validate the invariant in the constructor or fail before minting.
Proposed fix
constructor(options: PymthouseAdapterOptions = {}) {
+ if (Boolean(options.client) !== Boolean(options.signerExchange)) {
+ throw new Error('PymthouseAdapter requires client and signerExchange to be configured together');
+ }
this.clientOverride = options.client;
this.isConfiguredOverride = options.isConfigured;
this.signerExchange = options.signerExchange;
} const session =
- this.clientOverride && this.signerExchange
+ this.clientOverride
? await mintOpaqueSignerSessionForExternalUser({
client: this.clientOverride,
- exchange: this.signerExchange,
+ exchange: this.signerExchange,
externalUserId: input.externalUserId,
...(input.email != null ? { email: input.email } : {}),
})Also applies to: 225-237
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web-next/src/lib/billing/pymthouse-adapter.ts` around lines 71 - 74, The
PymthouseAdapter currently allows a client override without a matching
signerExchange and then silently falls back to the global mint path, which can
mix provider usage with a different signer session. Update PymthouseAdapter’s
constructor and minting flow to enforce the invariant that client and
signerExchange must be provided together: either validate immediately in the
constructor or throw before the mint step when the pair is incomplete. Use the
PymthouseAdapter and its minting logic around the clientOverride/signerExchange
handling to locate the fix.
Comment on lines
97
to
99
| throw new PmtHouseError(PYMTHOUSE_NOT_CONFIGURED_MESSAGE, { | ||
| status: 400, | ||
| code: 'pymthouse_required', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return 503 for missing global PymtHouse configuration.
Line 98 changes the pymthouse_required path to HTTP 400, but existing PymtHouse config-readers treat missing required PYMTHOUSE_* server config as 503. Keep this consistent so callers classify it as provider/server unavailable.
Proposed fix
throw new PmtHouseError(PYMTHOUSE_NOT_CONFIGURED_MESSAGE, {
- status: 400,
+ status: 503,
code: 'pymthouse_required',
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new PmtHouseError(PYMTHOUSE_NOT_CONFIGURED_MESSAGE, { | |
| status: 400, | |
| code: 'pymthouse_required', | |
| throw new PmtHouseError(PYMTHOUSE_NOT_CONFIGURED_MESSAGE, { | |
| status: 503, | |
| code: 'pymthouse_required', | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web-next/src/lib/pymthouse-client.ts` around lines 97 - 99, The PmtHouse
missing-configuration error in pymthouse-client should stay classified as a
server/provider availability issue rather than a client error. Update the
PmtHouseError thrown for the PYMTHOUSE_NOT_CONFIGURED_MESSAGE path in the
pymthouse-client logic to return HTTP 503 while keeping the existing
pymthouse_required code, so it matches the behavior of the other PYMTHOUSE_*
config readers and callers can handle it consistently.
seanhanca
added a commit
that referenced
this pull request
Jun 25, 2026
… error Co-authored-by: Cursor <cursoragent@cursor.com>
7 tasks
Closed
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Surfaced while running the multi-app billing E2E (P0–P5) on an isolated preview: the key-validation front door (
POST /api/v1/keys/validate) returned HTTP 503 for both legacy seat-bound and subscription-boundnaap_keys because the signer-session mint failed.Root cause:
PymthouseAdapter.mintSignerSessioncalled the SDK 0.4.3PmtHouseClient.mintSignerSessionForExternalUser, which setsresourceto the OIDC issuer. Current PymtHouse routes aresource-bearing token-exchange to signer-JWT exchange (returns another JWT, no opaquepmth_…session), so the front door could not produce a usable signer session.This routes the mint through the existing opaque-session workaround (upsert user → mint user JWT → token-exchange without
resource) and makes it multi-app safe: a per-ProviderInstanceadapter exchanges against that app's issuer/M2M creds via a newsignerExchangeoption, while the global-env adapter keeps using thePYMTHOUSE_*env exchange.Changes
pymthouse-client.ts— extractexchangeUserJwtForOpaqueSignerSessionWith(userJwt, cfg)(explicit per-app config) and exportmintOpaqueSignerSessionForExternalUser({ client, exchange, … }).mintSignerSessionForExternalUsernow delegates to it with the global env client + config (behavior preserved).pymthouse-adapter.ts— addsignerExchange?: PymthouseSignerExchangeConfigoption;mintSignerSessionmints via the opaque workaround (per-instance override when aclient+signerExchangeare present, else global env).provider-instance.ts— pass the instance'sissuerUrl+ M2M client id/secret assignerExchangewhen building a per-instance adapter.Safety / regression
signerExchange(i.e. a configuredProviderInstance).pymthouse_bpp_validate/ native-key flags via the call sites.Test plan
POST /api/v1/keys/validatenow returns HTTP 200 withvalid: true,capabilities: ["*"], and an opaquepmth_…signerSessionfor both legacy seat-bound and subscription-bound keys (previously 503).pmth_…session.pymthouse-adapter/ billing).Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes