chore(pymthouse): upgrade builder-sdk to 0.4.3 and new API key handling#383
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
scope/shell
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughPymtHouse developer keys are switched from short-lived signer sessions to long-lived ChangesPymtHouse long-lived API key flow
Sequence Diagram(s)sequenceDiagram
actor User
participant StartRoute as POST /api/v1/auth/providers/pymthouse/start
participant KeysBFF as pymthouse-keys-bff
participant AppsAPI as PymtHouse Builder Apps API
participant ExchangeRoute as POST /api/pymthouse/keys/exchange
participant SignerJWT as PymtHouse Token Endpoint
User->>StartRoute: initiate pymthouse OAuth link
StartRoute->>KeysBFF: createPymthouseApiKey(externalUserId, email)
KeysBFF->>AppsAPI: POST /users (ensureAppUserProvisioned)
AppsAPI-->>KeysBFF: 200 / 409
KeysBFF->>AppsAPI: POST /keys
AppsAPI-->>KeysBFF: pmth_* apiKey + row
KeysBFF-->>StartRoute: apiKey string
StartRoute-->>User: access_token=pmth_* (long-lived, Bearer, SIGN_JOB_SCOPE)
Note over User,SignerJWT: Later — at streaming time
User->>ExchangeRoute: POST pmth_* key
ExchangeRoute->>SignerJWT: RFC 8693 token_exchange
SignerJWT-->>ExchangeRoute: short-lived signer JWT
ExchangeRoute-->>User: signer JWT for streaming
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web-next/src/app/api/v1/developer/keys/[id]/route.ts (1)
59-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEXPIRED PymtHouse keys are still returned by this endpoint.
The list endpoint (
GET /keys) filters out PymtHouse keys withstatus: 'EXPIRED', but this single-key endpoint returns them without checking. The plugin backend (server.tslines 709-713) does return 404 for EXPIRED PymtHouse keys in its GET /:id handler.Consider adding consistent filtering:
🔧 Suggested fix
if (!apiKey) { return errors.notFound('API key'); } + // Hide EXPIRED PymtHouse keys (consistent with list endpoint) + if ( + apiKey.billingProvider?.slug === 'pymthouse' && + apiKey.status === 'EXPIRED' + ) { + return errors.notFound('API key'); + } + return success({ key: { ...toSafeDevApiKey(apiKey), expiresAt: null, }, });🤖 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/app/api/v1/developer/keys/`[id]/route.ts around lines 59 - 68, The single-key endpoint in the route handler is returning API keys regardless of their expiration status, which is inconsistent with the list endpoint that filters out expired keys and the plugin backend that returns 404 for expired keys. After the existing check for apiKey existence, add an additional validation to check if the apiKey has a status of 'EXPIRED', and if so, return errors.notFound('API key') to maintain consistency with the other endpoints.
🧹 Nitpick comments (3)
apps/web-next/src/lib/pymthouse-signer-exchange-config.ts (1)
14-23: ⚡ Quick winValidate URL env values before returning exchange config.
PYMTHOUSE_SIGNER_URLandPYMTHOUSE_ISSUER_URLare currently accepted as any non-empty strings. Invalid URL values can pass config checks and fail later at request time. Validate/normalize them here and returnnullon invalid input so routes consistently surface misconfiguration.♻️ Proposed refactor
import { getPymthouseIssuerUrlFromEnv } from '`@pymthouse/builder-sdk/config`'; +function normalizeHttpUrl(raw: string): string | null { + try { + const url = new URL(raw); + if (url.protocol !== 'http:' && url.protocol !== 'https:') return null; + return url.toString().replace(/\/+$/, ''); + } catch { + return null; + } +} + function issuerOriginFromIssuerUrl(issuerUrl: string): string { return issuerUrl.replace(/\/api\/v1\/oidc\/?$/i, '').replace(/\/+$/, ''); } @@ export function resolvePymthouseSignerUrl(): string | null { const fromEnv = process.env.PYMTHOUSE_SIGNER_URL?.trim(); if (fromEnv) { - return fromEnv.replace(/\/+$/, ''); + return normalizeHttpUrl(fromEnv); } const issuerUrl = getPymthouseIssuerUrlFromEnv(); if (!issuerUrl) { return null; } @@ const issuerUrl = process.env.PYMTHOUSE_ISSUER_URL?.trim(); const m2mClientId = process.env.PYMTHOUSE_M2M_CLIENT_ID?.trim(); const m2mClientSecret = process.env.PYMTHOUSE_M2M_CLIENT_SECRET?.trim(); - if (!issuerUrl || !m2mClientId || !m2mClientSecret) { + const normalizedIssuerUrl = issuerUrl ? normalizeHttpUrl(issuerUrl) : null; + if (!normalizedIssuerUrl || !m2mClientId || !m2mClientSecret) { return null; } @@ return { - issuerUrl, + issuerUrl: normalizedIssuerUrl, m2mClientId, m2mClientSecret, allowInsecureHttp: process.env.PYMTHOUSE_ALLOW_INSECURE_HTTP === '1', signerUrl, };Also applies to: 35-50
🤖 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-signer-exchange-config.ts` around lines 14 - 23, The resolvePymthouseSignerUrl function and related URL resolution functions accept environment values as any non-empty strings without validating them are proper URLs. Add URL validation to the resolvePymthouseSignerUrl function to validate PYMTHOUSE_SIGNER_URL before returning it, and also validate the issuer URL obtained from getPymthouseIssuerUrlFromEnv and passed to issuerOriginFromIssuerUrl. Return null if any URL validation fails rather than returning potentially invalid URL strings, ensuring configuration errors are caught early instead of at request time.apps/web-next/src/lib/pymthouse-client.ts (1)
79-88: ⚡ Quick winAdd timeout to token exchange fetch call.
Same concern as the keys BFF: external API call without timeout could hang indefinitely.
♻️ Suggested fix
const response = await fetch(tokenEndpoint, { method: 'POST', headers: { Authorization: `Basic ${Buffer.from(`${env.m2mClientId}:${env.m2mClientSecret}`).toString('base64')}`, 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json', }, body: params.toString(), cache: 'no-store', + signal: AbortSignal.timeout(15_000), // 15s timeout });🤖 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 79 - 88, The fetch call to tokenEndpoint for token exchange lacks a timeout configuration, which could cause the request to hang indefinitely if the external API is unresponsive. Add an AbortController with a timeout to the fetch call by creating an AbortController instance, setting a timeout that aborts the signal after a reasonable duration (typically a few seconds), and passing the signal as part of the fetch options alongside the other headers and configuration. This ensures the request will fail fast if the external service does not respond within the expected timeframe.apps/web-next/src/lib/pymthouse-keys-bff.ts (1)
72-88: ⚡ Quick winConsider adding a timeout to external API calls.
The
fetchcalls to the PymtHouse Builder Apps API have no timeout. If the upstream service hangs, this could block the request indefinitely.♻️ Suggested approach using AbortSignal.timeout
const response = await fetch( `${appsOrigin()}/api/v1/apps/${encodeURIComponent(publicClientId)}/users`, { method: 'POST', headers: { Authorization: readM2mAuthHeader(), Accept: 'application/json', 'Content-Type': 'application/json', }, body: JSON.stringify({ externalUserId, email: email?.trim() || externalUserId, status: 'active', }), cache: 'no-store', + signal: AbortSignal.timeout(10_000), // 10s timeout }, );Apply the same pattern to the fetch in
createPymthouseApiKey(line 105).🤖 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-keys-bff.ts` around lines 72 - 88, The fetch call in the createPymthouseUser function lacks a timeout configuration, which could cause the request to hang indefinitely if the upstream service is unresponsive. Add a timeout to the fetch call using AbortSignal.timeout to ensure the request fails gracefully after a reasonable duration. Apply the same timeout pattern to the fetch call in the createPymthouseApiKey function as well to maintain consistency across all external API calls to the PymtHouse Builder Apps API.
🤖 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/pymthouse-client.ts`:
- Around line 114-131: In the parseSignerSessionExchange function call where
expires_in is being set, change the default fallback value from 0 to 300. The
expiresIn variable is being passed with a nullish coalescing operator that
defaults to 0 when the value is missing or invalid, but this disables token
caching in the SDK since a TTL of 0 is interpreted as immediate expiration.
Replace the fallback value from 0 to 300 seconds to allow the SDK's proactive
refresh mechanism (which refreshes at 80% of TTL) to function properly and
maintain the token reuse strategy.
---
Outside diff comments:
In `@apps/web-next/src/app/api/v1/developer/keys/`[id]/route.ts:
- Around line 59-68: The single-key endpoint in the route handler is returning
API keys regardless of their expiration status, which is inconsistent with the
list endpoint that filters out expired keys and the plugin backend that returns
404 for expired keys. After the existing check for apiKey existence, add an
additional validation to check if the apiKey has a status of 'EXPIRED', and if
so, return errors.notFound('API key') to maintain consistency with the other
endpoints.
---
Nitpick comments:
In `@apps/web-next/src/lib/pymthouse-client.ts`:
- Around line 79-88: The fetch call to tokenEndpoint for token exchange lacks a
timeout configuration, which could cause the request to hang indefinitely if the
external API is unresponsive. Add an AbortController with a timeout to the fetch
call by creating an AbortController instance, setting a timeout that aborts the
signal after a reasonable duration (typically a few seconds), and passing the
signal as part of the fetch options alongside the other headers and
configuration. This ensures the request will fail fast if the external service
does not respond within the expected timeframe.
In `@apps/web-next/src/lib/pymthouse-keys-bff.ts`:
- Around line 72-88: The fetch call in the createPymthouseUser function lacks a
timeout configuration, which could cause the request to hang indefinitely if the
upstream service is unresponsive. Add a timeout to the fetch call using
AbortSignal.timeout to ensure the request fails gracefully after a reasonable
duration. Apply the same timeout pattern to the fetch call in the
createPymthouseApiKey function as well to maintain consistency across all
external API calls to the PymtHouse Builder Apps API.
In `@apps/web-next/src/lib/pymthouse-signer-exchange-config.ts`:
- Around line 14-23: The resolvePymthouseSignerUrl function and related URL
resolution functions accept environment values as any non-empty strings without
validating them are proper URLs. Add URL validation to the
resolvePymthouseSignerUrl function to validate PYMTHOUSE_SIGNER_URL before
returning it, and also validate the issuer URL obtained from
getPymthouseIssuerUrlFromEnv and passed to issuerOriginFromIssuerUrl. Return
null if any URL validation fails rather than returning potentially invalid URL
strings, ensuring configuration errors are caught early instead of at request
time.
🪄 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: d609b884-db21-426a-8983-c3c5dc0b9985
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (20)
.vscode/settings.jsonapps/web-next/.env.local.exampleapps/web-next/package.jsonapps/web-next/src/app/api/pymthouse/keys/exchange/route.tsapps/web-next/src/app/api/signer/device/exchange/route.tsapps/web-next/src/app/api/v1/auth/providers/[providerSlug]/start/route.tsapps/web-next/src/app/api/v1/billing/pymthouse/config/route.tsapps/web-next/src/app/api/v1/billing/pymthouse/token/route.tsapps/web-next/src/app/api/v1/billing/pymthouse/usage/route.test.tsapps/web-next/src/app/api/v1/billing/pymthouse/usage/route.tsapps/web-next/src/app/api/v1/developer/keys/[id]/route.tsapps/web-next/src/app/api/v1/developer/keys/route.tsapps/web-next/src/lib/pymthouse-client.tsapps/web-next/src/lib/pymthouse-keys-bff.tsapps/web-next/src/lib/pymthouse-signer-exchange-config.tsdocs/pymthouse-integration.mdplugins/developer-api/backend/package.jsonplugins/developer-api/backend/src/server.tsplugins/developer-api/frontend/package.jsonplugins/developer-api/frontend/src/pages/DeveloperView.tsx
… implement new API key handling This update includes the following changes: - Upgraded the @pymthouse/builder-sdk dependency from version 0.1.0 to 0.4.3 across various package.json files. - Introduced new API endpoints for creating and exchanging PymtHouse API keys, allowing for long-lived pmth_* keys. - Updated existing routes to utilize the new API key handling logic, ensuring compatibility with the latest SDK features. - Added configuration settings for PymtHouse integration in the .env.local.example file. - Implemented new utility functions for managing API key exchanges and signer sessions. This upgrade enhances the overall functionality and security of the PymtHouse integration, aligning with the latest SDK capabilities.
eliteprox
force-pushed
the
feat/update-builder-sdk
branch
from
June 19, 2026 01:20
863491f to
a54d4eb
Compare
Reconcile builder-sdk 0.4.3 upgrade with usage-pull code already on main. Co-authored-by: Cursor <cursoragent@cursor.com>
- developer keys [id] GET now hides EXPIRED PymtHouse keys, matching the list endpoint and plugin backend (CodeRabbit outside-diff finding). - signer-session exchange falls back to a 5-minute TTL (was 0) when the provider omits expires_in, so SDK proactive refresh/reuse stays enabled. - add fail-fast timeouts to PymtHouse token-exchange and Builder Apps API fetches (no indefinite hangs). - validate/normalize PYMTHOUSE_SIGNER_URL / PYMTHOUSE_ISSUER_URL so misconfiguration surfaces at config time instead of at request time. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Review addressed + reconciled with
|
| Finding | Resolution |
|---|---|
Inline (Major) — pymthouse-client.ts expires_in: ?? 0 disables SDK token cache/refresh |
Fixed → ?? 300 (SDK default). Fallback only; real provider expires_in is used as-is. Replied on the thread. |
Outside-diff (Minor) — developer/keys/[id] GET returns EXPIRED PymtHouse keys, inconsistent with list endpoint + plugin backend |
Fixed — single-key GET now 404s EXPIRED PymtHouse keys (non-PymtHouse keys unaffected). |
Nitpick — pymthouse-client.ts token-exchange fetch has no timeout |
Fixed — AbortSignal.timeout(15_000). |
Nitpick — pymthouse-keys-bff.ts Builder Apps fetches have no timeout |
Fixed — AbortSignal.timeout(10_000) on both calls. |
Nitpick — signer-exchange-config.ts accepts any non-empty string as a URL |
Fixed — normalizeHttpUrl() validates/normalizes PYMTHOUSE_SIGNER_URL and PYMTHOUSE_ISSUER_URL, returning null on invalid input so misconfig surfaces at config time. |
Verification
tsc --noEmit(web-next): no new errors vsorigin/mainbaseline. The bump actually removes one pre-existing main error (UsageApiResponse.sourcenow exists in 0.4.3). All remaining TS errors are pre-existing onmainand tolerated by CI (continue-on-error).- Targeted vitest: 447 passed across
lib/billing,lib/dev-api,lib/gateway,lib/pymthouse-*(incl.pymthouse-adapter+ billing usage). Pre-push SDK tests: 275 passed. - All pymthouse feature flags (
provider_adapters,usage_pull, …) remain default OFF — this is a dependency bump, not a feature enablement, and is inert in production until flags are enabled.
The signer-session protocol reconciliation (dropping the local opaque-session exchange workaround once PymtHouse routing is aligned) remains a separate tracked follow-up.
@coderabbitai resolve
|
The verification steps (TypeScript clean compile, 447 Vitest passes, 275 SDK tests, feature flags remaining OFF) give good confidence the changes are safe. Resolving all open comments now. (∩｀-´)⊃━☆ﾟ.*･｡ﾟ ✅ Action performedComments resolved and changes approved. |
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
2 participants
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
@pymthouse/builder-sdkfrom0.1.0to0.4.3across web-next and the developer-api plugin packages./api/pymthouse/keys/exchange,/api/signer/device/exchange) and a billing config BFF endpoint, using the SDK's server-side exchange handlers.pmth_*keys via the Builder Apps user-keys API, with shared utilities inpymthouse-client.ts,pymthouse-keys-bff.ts, andpymthouse-signer-exchange-config.ts.docs/pymthouse-integration.md.Test plan
npm installat repo root succeeds with the pinned0.4.3SDKPYMTHOUSE_M2M_CLIENT_ID,PYMTHOUSE_M2M_CLIENT_SECRET, etc.)pmth_*key on first-time PymtHouse link (POST /api/v1/auth/providers/pymthouse/start)POST /api/pymthouse/keys/exchangeexchanges apmth_*key for a short-lived signer JWTPOST /api/signer/device/exchangeexchanges a device token for a signer sessionGET /api/v1/billing/pymthouse/configreturns signer URL for the Developer API UIPOST /api/v1/billing/pymthouse/tokenstill mints opaque signer sessions for authenticated usersGET /api/v1/billing/pymthouse/usagecontinues to proxy usage dataSummary by CodeRabbit
pmth_*API keys that persist until revoked.pmth_*API keys and device tokens to signer sessions.scope=menow includes retail usage data.pmth_*keys no longer show computed expiry, and onlyEXPIREDkeys are hidden/404’d.@pymthouse/builder-sdkto version0.4.3across packages.