fix(pymthouse): emit composite app.pmth_ bearer for DMZ signer#421
Conversation
PR #210 expects composite API keys on the remote signer DMZ. Replace the legacy user-JWT forward with per-user composite key mint at validate time, and forward composite PYMTHOUSE_API_KEY values directly without the signer-session exchange hop. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR changes ChangesComposite API key signer resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant PymthouseAdapter
participant SignerRouting
participant PymthouseKeysBff
participant ExchangeService
Caller->>PymthouseAdapter: resolveSignerEndpoint()
PymthouseAdapter->>SignerRouting: getSignerRouting()
alt apiKeyCfg present, composite key
PymthouseAdapter->>PymthouseAdapter: use apiKey directly (Bearer apiKey)
else apiKeyCfg present, bare key
PymthouseAdapter->>ExchangeService: exchangeApiKeyForSignerSession(apiKeyCfg)
ExchangeService-->>PymthouseAdapter: session.accessToken, signerUrl
else no apiKeyCfg
PymthouseAdapter->>PymthouseKeysBff: createPymthouseApiKey(externalUserId, "naap-validate-signer")
PymthouseKeysBff-->>PymthouseAdapter: apiKey
end
PymthouseAdapter-->>Caller: url + Authorization: Bearer <token>
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web-next/src/lib/billing/pymthouse-adapter.ts (1)
368-376: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the stale method contract comment above this flow.
The method doc still describes forwarding a minted user JWT, but this branch now forwards a composite API key. That auth contract should match the implementation.
🤖 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 368 - 376, The method contract comment for this auth flow is stale and still describes forwarding a minted user JWT, but the `createPymthouseApiKey` branch now returns a composite API key in the `Authorization` header. Update the doc comment above this logic in `pymthouse-adapter.ts` so it matches the current behavior in the `validateSigner`-style flow and clearly states that the bearer token is the composite `app_XXX.pmth_YYY` key, not a user JWT.
🤖 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 319-340: The `getSignerRouting()` lookup in `PymthouseAdapter` is
being done too early, which makes bare `pmth_` keys fail before they can use the
exchange path. Update the flow in the signer URL selection method so
`apiKey.includes('.pmth_')` is checked first, and only call
`this.client().getSignerRouting()` for the composite-key direct-DMZ branch; keep
bare `pmth_` keys routed through `exchangeApiKeyForSignerSession()` unchanged.
- Around line 368-372: The signer-resolution path in pymthouse-adapter’s
remote-signer flow is minting a new long-lived API key each time
`createPymthouseApiKey()` is called, which can accumulate unreused bearer
credentials. Update the logic around the remote-signer branch and
`createPymthouseApiKey` usage so it reuses an existing key when possible, or
adds a clear TTL/revocation/rotation contract instead of always creating a fresh
active key on every resolution.
---
Nitpick comments:
In `@apps/web-next/src/lib/billing/pymthouse-adapter.ts`:
- Around line 368-376: The method contract comment for this auth flow is stale
and still describes forwarding a minted user JWT, but the
`createPymthouseApiKey` branch now returns a composite API key in the
`Authorization` header. Update the doc comment above this logic in
`pymthouse-adapter.ts` so it matches the current behavior in the
`validateSigner`-style flow and clearly states that the bearer token is the
composite `app_XXX.pmth_YYY` key, not a user JWT.
🪄 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: c9f05be2-1996-4618-881b-0f01d4410065
📒 Files selected for processing (3)
apps/web-next/src/lib/billing/pymthouse-adapter.test.tsapps/web-next/src/lib/billing/pymthouse-adapter.tsapps/web-next/src/lib/pymthouse-signer-exchange-config.ts
Addresses CodeRabbit review on #421: - Regression fix: move getSignerRouting() INSIDE the composite `.pmth_` fast-path. A bare `pmth_` key resolves its DMZ url via exchangeApiKeyForSignerSession() and never needed routing, so running the routing lookup first could spuriously throw "no remote signer DMZ url" for bare-key callers. Extracted resolveDmzUrl() to dedupe url extraction/validation. - Documented the legacy mint-per-call fallback as a tracked follow-up: key reuse/TTL/revoke needs new persistent secret storage (Builder API returns the secret only at creation); prod uses the composite fast-path and never mints here. - Added a regression test asserting the bare-pmth_ path skips getSignerRouting() and still resolves via the exchange's own signerUrl (24/24 pass). Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
|
@coderabbitai review Both actionable comments addressed in b678d12:
Adapter suite: 24/24 pass. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
cd /tmp
gh pr view 421 --repo livepeer/naap --json commits,headRefName,baseRefName 2>&1 | head -50
gh pr diff 421 --repo livepeer/naap > /tmp/pr421.diff 2>&1
wc -l /tmp/pr421.diffLength of output: 2502 🏁 Script executed: #!/bin/bash
set -euo pipefail
cd /tmp
sed -n '1,429p' /tmp/pr421.diff | grep -n "^diff\|^+++\|^---"
echo "---- adapter diff section ----"
awk '/diff --git a\/apps\/web-next\/src\/lib\/billing\/pymthouse-adapter\.ts/,/diff --git a\/apps\/web-next\/src\/lib\/billing\/pymthouse-adapter\.test\.ts/' /tmp/pr421.diffLength of output: 7911 🏁 Script executed: #!/bin/bash
set -euo pipefail
cd /tmp
sed -n '1,292p' /tmp/pr421.diff | grep -n "getSignerRouting\|regression\|bare"Length of output: 1876 Confirmed both fixes are correctly implemented in b678d12:
Diff and helper extraction ( ✅ Action performedReview finished.
|
…t superseded/dead mintUserSignerJwtForExternalUser is coded, merged (#406/#410) and reached prod behind per_key_remote_signer, but is now orphaned: neither main nor feat/opaque-session-signer-endpoint call it. Superseded by composite-bearer (#421, Fix A) and api-key signer-session exchange (#412/#424). Plan file NAAP-SIGNER-JWT-EXCHANGE-PLAN.md does not exist; SDK mintUserSignerToken grant was abandoned upstream (500). Co-authored-by: Cursor <cursoragent@cursor.com>
…on doc (#430) * fix(billing): remove dead user-JWT signer mint + correct opaque-session doc The billed pymthouse `/generate-live-payment` webhook is JWT/composite-key only: it rejects a bare opaque `pmth_` session with 401 "not a JWT", while the unbilled `/sign-orchestrator-info` accepts it (looser auth) — which masked the asymmetry. main's `resolveSignerEndpoint()` already forwards the correct composite `app_<24hex>_pmth_<secret>` bearer (or exchanges a bare key for a signer JWT), so this change: - Deletes the orphaned Fix B `mintUserSignerJwtForExternalUser()` + `UserSignerJwt` (zero non-test call sites; superseded by #421/#412/#424) and its unit tests. - Rewrites the `resolveSignerEndpoint()` doc comment, which still referenced the removed user-JWT mint, to accurately describe the composite-bearer / api-key-exchange credential resolution and the billed-webhook asymmetry. - Drops the now-dangling `mintUserSignerJwtForExternalUser` mock + assertions from the adapter test. No behavior change: the composite-bearer forwarding and the `PER_KEY_REMOTE_SIGNER_FLAG` gating are untouched; no feature-flag defaults changed. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(e2e): Run 57 — billed E2E on LR-orch via composite-bearer path (PR #430) Verify PR #430 (forward composite app_..._pmth_... bearer, not opaque session) end-to-end against the Live Runner orchestrator (liverunner-staging-1): - validate → 200, signerSession {url,headers} with COMPOSITE bearer (PR #430 design) - signer /sign-orchestrator-info on LR-orch → 200 (composite accepted, not "not a JWT") - billed /generate-live-payment on LR-orch → 400 "missing or zero priceInfo" (LR zero-pricing config, John/orch infra — NOT an auth/#430 issue) - generation → 400 "Could not verify job creds" (unpaid, downstream of pricing) - byoc-staging-1 control (same composite bearer) → 200 + real image (stack intact) Verdict: PR #430 composite-bearer fix HOLDS on the LR path; remaining blocker is LR-orch zero-pricing (unchanged from Run 55/55b). Adds LR/composite probe scripts. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(audit): trace unit_kind billing-unit lifecycle + OpenMeter unit-metering gap unit_kind is display-only (advertised on /capabilities, back-filled for AI caps); it is never sent to orch, never in the create_signed_ticket event, and never read by the collector or OpenMeter meters (which sum network fee grouped by pipeline/ model only). Documents producer->transport->collector->consumer map with file:line and per-hop owners. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add Storyboard signer routing (Daydream vs pymthouse) architecture Document the production decision flow for how a Storyboard generation request selects the Daydream signer (signer.daydream.live, type:lv2v) vs the pymthouse per-key DMZ signer (type:byoc). The authoritative switch is SIGNER_FROM_VALIDATE + AUTH_VALIDATE_URL in the SDK service (_effective_signer), keyed on a naap_ bearer + validate signerSession; the python-gateway only derives payment type from the signer hostname. Includes a Mermaid diagram, per-hop file:line citations, and the current prod state (per-key path OFF; Daydream/lv2v is live). Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add pymthouse-e2e skill for the billed E2E test path Reusable skill/instructions doc (Claude Code or Cursor) covering the validate → signer → payment → orchestrator → metering flow, env-var config (placeholders only, no secrets), per-scenario run commands, result interpretation, troubleshooting, and coverage caveats. * docs: make liverunner-staging-1 (LR-orch) a first-class E2E scenario Add a dedicated LR-orch section (env, commands, per-stage expected results per Run 57), a byoc-staging-1 control contrast, and clarify the env table + interpreting section that LR zero-pricing is the expected, known blocker (John/orch-infra) while PR #430 auth holds. * docs: make pymthouse-e2e a true hand-off skill for agents Add a "For the AI agent" runbook at the top: step 0 prompts the user for required inputs (naap key OR composite bearer + signer URL), then the agent sets up env, resolves signer credentials, runs the default happy path, and emits a PASS/FAIL report. Adds a mandatory-vs-optional inputs checklist, a NAAP_KEY -> validate credential-derivation path, a report template table, and a warning that run57 has no GATEWAY_SRC default. No secrets embedded. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add BYOC-to-live-runner migration analysis + merge strategy Evidence-based HTML report covering byoc-staging-1 deploy state, PR/branch merge-state, live-runner parity matrix, minimal-merge recommendation, phased retire plan, and regression risks. Analysis only; no code changes. * docs(audit): resolve byoc-staging-1 orch branch/commit/image ambiguity Pinned image livepeer/go-livepeer:feat-remote-signer-byoc-v2 maps to branch feat/remote-signer-byoc-v2 @ be5a669e (built 2026-05-13, sha256:9f1abba6), NOT the local dev branch fix/byoc-e2e-v1-and-type-byoc @ 8ddd08ea (17 ahead / 119 behind). Adds build mapping (docker/metadata-action type=ref,event=branch), Docker Hub digest evidence, and the gcloud/docker-inspect command to confirm the live VM (deploy-byoc.sh does not wire ORCH_IMAGE, so the pin is docs-only). Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add Storyboard MCP performance & capacity investigation Test-based root-cause report (HTML). Findings: slowness / "out of capacity" is still happening, concentrated in image-to-video — seedance-i2v-fast runs 176-181s (advertised ~30s) and fails ~2/3 of the time against a ~180s SDK /inference abort ceiling. Root cause is per-capability Livepeer orchestrator/GPU capacity (no warm orchestrator), amplified by the hard timeout. t2v and ffmpeg are healthy. Includes latency/failure tables, server telemetry, ranked hypotheses, worst offenders, and recommendations. Total test spend $1.62. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add Storyboard MCP performance-bias skill / adaptive agent proposal Review-ready proposal comparing Option A (explicit two-mode "prefer faster iteration" vs default "prefer better quality" skill) against Option B (self-learning adaptive agent). Feasibility verdict is conditioned on the perf investigation root cause (Livepeer orchestrator/GPU capacity): a skill can bias model/settings/throughput and stream previews but cannot create capacity. Includes the two-mode lever matrix mapped to exact MCP params, one-file preference-memory design, A-vs-B tradeoff table, phased MVP->full recommendation, and metrics/test plan reusing the perf harness. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(skills): add storyboard-perf-mode two-mode perf skill (Option A MVP) Implements Phase 0 (route-around + measure) and Phase 1 (Option A MVP) from STORYBOARD-PERF-SKILL-PROPOSAL.md: explicit prefer-quality (default) / prefer-fast modes with a remembered preference, a lever matrix mapping each mode to exact user-storyboard MCP tool + params, warm-cap i2v routing (pixverse-i2v, never seedance-i2v-fast), session_id tagging, and measurement hooks via get_perf_report / get_cost_report. Phase 2 (self-learning) is intentionally not built. Preference memory: ~/.storyboard/perf-preference.json via scripts/perf_pref.py (get/set/reset). Skill lives under repo skills/ (committable) since .cursor/ is gitignored; activation note included. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(perf): add Storyboard PR #685 post-fix E2E assessment Ship-and-verify report for the seedance-i2v-fast → pixverse-i2v fast-animate re-route (PR #685, merged 951eb7e0). Live head-to-head: i2v fast path ~33%→100% success, ~176-181s→~40s p50, ~2x cheaper per delivered asset. Notes deploy status (merged; prefer_fast path pending redeploy) and spend (~$2.01). Co-authored-by: Cursor <cursoragent@cursor.com> * docs(billing): add per-unit (per-ticket) metering design proposal Investigates the signer -> OpenMeter -> pymthouse metering pipeline and documents that per-unit metering is not implemented today: cost is a fee-floor (1 uUSD ceil) over a seconds-as-flat-unit quantity, so image megapixels, video seconds, and TTS chars all meter identically. Proposes a minimal 3-hop fix: add billing_unit_kind + billing_unit_quantity to the create_signed_ticket event (unit from the cap price row, quantity from the request payload), carry them through the collector, add a usage_units SUM meter grouped by unit_kind, and compute pymthouse cost as quantity x per-unit price reconciled against the network fee. Includes per-unit requirement matrix, schema, phased backward-compatible rollout, test plan, and owners. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: network pricing/metering/billing architecture across agent, byoc signer, live-runner Add NETWORK-PRICING-ARCHITECTURE.html: a code-grounded examination of pricing across the agent (user-facing USD), the byoc-staging-1 signer (on-chain tickets), and the live-runner orchestrator, plus an extensible one-orch->many-runners design. Builds on/reconciles the prior per-unit metering, BYOC->live-runner migration, and signer-routing artifacts with fresh file:line evidence, a per-cap pricing config schema, and the agent-vs-on-chain (single-source-of-truth) recommendation. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(pricing): consolidate to descriptor price segment as single source of truth Establish the capability descriptor's offering.price as the ONE source of truth for per-cap price + unit; demote price.json/pricing-table-deploy.json to a build-time generated artifact. Add Part 0.5 (schema quote, field-by-field gap table, consumer table, consolidated flow diagram, DO/DONT), reconcile the Part 1 agent-pricing diagram and Part 4b config example, and flag the unit_kind (+ quantity_source, price_scaling) gaps the PriceSchema must add to fully replace price.json. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(pricing): add PR-cycle execution plan + design-review section Add PRICING-EXECUTION-PLAN.html: 10 self-contained, flag-gated (default OFF), observe->enforce PRs with per-PR test / prod-validation / rollback / owner, the reconcile-by-construction invariant, a gap-closure matrix, phased rollout, and end-to-end prod validation. Append a concise "Design review findings + execution plan" section to NETWORK-PRICING-ARCHITECTURE.html linking the plan, stating the invariant, and listing the dead-code-removal tasks. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(pricing): append independent code-verified review to execution plan Fresh-eyes review of PRICING-EXECUTION-PLAN.html, verified against source in storyboard-a3 and livepeer/go-livepeer. Verdict: NEEDS-REWORK with 4 blockers (invariant scaling term vs pixels_per_unit, lossy descriptor-as- source vs pricing-table.json margin/tier model, closed unit_kind enum drops live `tokens`, PR9 removing static-pricing.json fallback) plus 2 majors and minors. Architecture/sequencing sound; fixes are bounded. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(pricing): add A/B/C fee-locus assessment to NETWORK-PRICING-ARCHITECTURE Append section 7 "Where should fee/quantity compute live? (A vs B vs C)": per-fact (price/quantity/fee) layer-knowledge table grounded in remote_signer.go, orchestrator.go and gateway byoc.py; the on-chain trust/consistency analysis (ExpectedPrice vs orch re-derivation, #3993); the pre- vs post-inference metering point; and the verdict — hybrid of A (signer derives fee, gateway reports quantity) + the price-config half of C (runner owns per-unit price in its descriptor). Reject B and full C. PR4/PR5 stand unchanged. Additive edit only. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add scratch notes analyzing go-livepeer PR #3992 live pricing Analyze what PR #3992 actually ships (decimal USD runner price with unit={hour,720p}, signer type=live elapsed-seconds billing) vs the assumed type=fixed per-request model. Maps billing unit_kinds to coverage, confirms per-call/per-image still need the quantity design (PR4/PR5) and that tokens (B3) remains the residual gap. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(pricing): consolidated rewrite resolving review blockers B1-B4/M1-M2 + fold #3992/compute-placement Resolves the independent review's four blockers and two majors in-plan and folds in the compute-placement verdict (hybrid A + price-config half of C) and the PR #3992 fixed/live-pricing analysis. Plan status moves NEEDS-REWORK -> APPROVE-able. - B1: correct invariant to fee_wei = (price_per_unit/price_scaling) x pixels_per_unit x qty (verified vs convert.py:56-67); F4 no longer calls per-price scaling speculative. - B2: state two SoTs in one direction (build-time pricing-table.json cost/margin/tier -> runtime descriptor); convert.py generates the descriptor price, does not "write back". - B3: add tokens to unit_kind enum + extractors + parity matrix + E2E (audit: enum complete). - B4: keep static-pricing.json runtime fallback; PR9 removes only duplicate authoring. - M1: shared quantity-extractor spec + cross-language golden vectors (Py/Go/TS). - M2: PR2 reconciles generate-registry.ts (CAP_REGISTRY_PRICING) second registry path. - Compute-placement: gateway reports QUANTITY, signer derives FEE; ExpectedPrice==advertised guard + qty cross-check. - #3992: adopt runner decimal-USD price schema (PR3) + signer type-branched fee (PR5/PR8); per-call/per-image ride quantity mechanism; capture live_payment_processor pixel bug and global-vs-per-cap max-price caveats as risks. - Minor: verified sanitizeUsageLabel real (:383), mint helper absent from product code; refreshed signer line cites. - Architecture doc: minimal consistency edits (tokens row, #3992 pointer, SoT direction). Co-authored-by: Cursor <cursoragent@cursor.com> * docs(pricing): add §8 — Storyboard-independent PRs, regression class, and upstream already-exists vs still-needed recon Additive section to PRICING-EXECUTION-PLAN.html documenting which pricing PRs Storyboard can ship entirely within livepeer/storyboard (PR1/PR2 NO-REGRESSION, PR10 low display-only risk, PR9 upstream-gated by PR3), the standalone payoff, and a verified go-livepeer/gateway already-exists vs still-needed table (ja/live-runner #3938, ja/live-pricing #3992, byoc-staging-1, lr-orch). Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add upstream pricing scope for owners (John/Josh/Rick) PR-format scope doc from the agent's POV covering the three upstream changes needed to close pricing end-to-end: go-livepeer PR3/PR5/PR8 (orch aggregation + signer stamp + fee-from-quantity), python-gateway PR4 (payload->quantity), pymthouse PR6/PR7 (usage_units meter + cost). Includes per-owner "already exists, don't rebuild" callouts, the reconciliation invariant, and cross-owner ordering. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(pricing): add PRICING-DELIVERY-SUMMARY.html Skimmable delivery summary of the pricing/billing/perf effort with gh-verified status per artifact: what's shipped/built/merged vs open, expected value per item, and the upstream PRs + infra gates left for Josh/Rick/John. Corrects #430 to OPEN (not merged). Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
resolveSignerEndpointwith compositeapp_XXX.pmth_YYYkeys (PR fix: set DATABASE_URL_UNPOOLED for prisma db push #210 DMZ contract).PYMTHOUSE_API_KEYvalues directly to the DMZ (skip broken signer-session exchange for composite format).pmth_keys still use the existing api-key signer-session exchange path.Test plan
pymthouse-adapter.test.ts(23 tests)signerSession.headers.AuthorizationisBearer app_98575870….pmth_…(not JWT)IncompleteRead(85,108)(John)Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes