fix(billing): remove dead user-JWT signer mint + correct opaque-session doc#430
Merged
Conversation
…on 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>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoves the user-scoped signer JWT API and updates signer endpoint resolution tests and documentation. Adds billed BYOC E2E audit records, signer-routing and runbook documentation, executable payment probes, Live Runner diagnostics, and a BYOC-to-Live-Runner migration report. ChangesPymthouse signer flow
Billed E2E diagnostics
BYOC-to-Live-Runner migration analysis
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
#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>
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
BILLED-E2E-BLOCKER-AUDIT.md (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the reported Markdownlint violations.
BILLED-E2E-BLOCKER-AUDIT.md#L23-L23: escape or reformat leading#255.BILLED-E2E-BLOCKER-AUDIT.md#L81-L81: add a fence language such astext.BILLED-E2E-BLOCKER-AUDIT.md#L186-L186: add the missing table cell or move the resolved B5 row to the fixed-items table.BILLED-E2E-BLOCKER-AUDIT.md#L237-L239: escape leading#255and specify the code-block language.BILLED-E2E-BLOCKER-AUDIT.md#L1159-L1159,BILLED-E2E-BLOCKER-AUDIT.md#L1169-L1169, andBILLED-E2E-BLOCKER-AUDIT.md#L1176-L1176: surround tables with blank lines.🤖 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 `@BILLED-E2E-BLOCKER-AUDIT.md` at line 23, Resolve all Markdownlint violations in BILLED-E2E-BLOCKER-AUDIT.md: at lines 23 and 237-239 escape leading “#255”; at line 81 specify a language for the fenced block; at line 186 add the missing table cell or move the resolved B5 row into the fixed-items table; and at lines 1159, 1169, and 1176 add blank lines before and after the affected tables.Source: Linters/SAST tools
USER-E2E-DEMO-RESULTS.md (1)
223-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the reported markdownlint warnings before merge.
The file has repeated MD040 missing fence languages, MD058 table-spacing warnings, plus MD018/MD038 formatting issues. Add languages to fenced blocks and normalize the affected tables/code spans.
🤖 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 `@USER-E2E-DEMO-RESULTS.md` at line 223, Resolve the markdownlint findings in USER-E2E-DEMO-RESULTS.md: add an appropriate language identifier to every fenced code block flagged by MD040, normalize spacing around the affected tables for MD058, and correct heading/code-span formatting for MD018 and MD038. Preserve the documented content while ensuring the file passes markdownlint.Source: Linters/SAST tools
🤖 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 `@BILLED-E2E-BLOCKER-AUDIT.md`:
- Around line 3-5: Update the audit document header metadata: rename the
existing Date field to Started, preserve its original Run 47 start date, and add
a Last updated field with 2026-07-20 to reflect the appended Run 57 findings.
In `@scripts/run53-multicap-probe.py`:
- Around line 109-146: Broaden the exception handling around the
payment-generation and parsing flow after the generate-live-payment request so
URLError, malformed JSON, missing payment data, and protobuf decoding failures
are captured per capability instead of aborting the sweep. Record the failure in
row, append and print the row, then continue to the next capability, while
preserving the existing successful payment-matching logic and final results
output.
In `@USER-E2E-DEMO-RESULTS.md`:
- Around line 1-9: Redact the committed USER-E2E-DEMO-RESULTS.md report by
removing operator identity, credential-adjacent details, production URLs and
infrastructure identifiers, wallet/application metadata, and bearer or token
fragments, including the referenced sections. Retain only non-sensitive evidence
and replace removed diagnostics with an access-controlled evidence ID reference.
- Around line 5044-5046: Update the verdict logic in run57-lr-auth-vs-pay.py to
parse the structured JSON error code and message together with the HTTP status
before classifying failures. Remove substring-only checks for “not a jwt” and
“priceinfo”, and map verdicts only when the structured fields match the expected
auth or LR-configuration errors; preserve unrelated failures as their actual
status/error rather than misclassifying them.
- Around line 4530-4532: Update the curl reproduction command in the “Reproduce”
section to avoid passing M2M credentials through the command line; provide
authentication via a temporary 0600-protected netrc/config file or curl
configuration supplied through stdin, while preserving the existing request URL
and options.
- Around line 5051-5055: Update the LR-orch results section around the “Run 56”
mention: either add a valid link or artifact for Run 56, or rephrase the
statement to describe the observed opaque-session rejection without referencing
an unavailable run. Keep the surrounding authentication findings unchanged.
---
Nitpick comments:
In `@BILLED-E2E-BLOCKER-AUDIT.md`:
- Line 23: Resolve all Markdownlint violations in BILLED-E2E-BLOCKER-AUDIT.md:
at lines 23 and 237-239 escape leading “#255”; at line 81 specify a language for
the fenced block; at line 186 add the missing table cell or move the resolved B5
row into the fixed-items table; and at lines 1159, 1169, and 1176 add blank
lines before and after the affected tables.
In `@USER-E2E-DEMO-RESULTS.md`:
- Line 223: Resolve the markdownlint findings in USER-E2E-DEMO-RESULTS.md: add
an appropriate language identifier to every fenced code block flagged by MD040,
normalize spacing around the affected tables for MD058, and correct
heading/code-span formatting for MD018 and MD038. Preserve the documented
content while ensuring the file passes markdownlint.
🪄 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: 56c579cd-1085-4faa-9b55-8a42a45340e5
📒 Files selected for processing (7)
BILLED-E2E-BLOCKER-AUDIT.mdUSER-E2E-DEMO-RESULTS.mdscripts/run50-direct-signer-probe.pyscripts/run53-multicap-probe.pyscripts/run55-lr-generic-diag.pyscripts/run55-lr-orchinfo-diag.pyscripts/run57-lr-auth-vs-pay.py
Comment on lines
+3
to
+5
| **Date:** 2026-07-17 (Run 47 — live probes + code review) | ||
| **Method:** Live HTTP probes against prod/staging endpoints, `gh` PR status, gateway commit `1bf13cd` source review, gateway venv `submit_byoc_job` (certifi), Storyboard MCP regression. | ||
| **Honesty rule:** This doc states what **failed today**, not what passed last week. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the document’s freshness metadata.
The header still says 2026-07-17 (Run 47), while this audit now includes Run 57 on July 20, 2026. Rename this field to Started and add Last updated: 2026-07-20 so readers do not mistake the append-only audit for a stale snapshot.
🤖 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 `@BILLED-E2E-BLOCKER-AUDIT.md` around lines 3 - 5, Update the audit document
header metadata: rename the existing Date field to Started, preserve its
original Run 47 start date, and add a Last updated field with 2026-07-20 to
reflect the appended Run 57 findings.
Comment on lines
+109
to
+146
| # generate-live-payment | ||
| payload = { | ||
| "orchestrator": base64.b64encode(info.SerializeToString()).decode("ascii"), | ||
| "type": "byoc", | ||
| "capabilities": base64.b64encode(byoc_caps.SerializeToString()).decode("ascii"), | ||
| } | ||
| req = urllib.request.Request( | ||
| f"{signer_origin}/generate-live-payment", | ||
| data=json.dumps(payload).encode(), | ||
| headers={"Content-Type": "application/json", "Livepeer-Capability": cap, **headers}, | ||
| method="POST", | ||
| ) | ||
| try: | ||
| with urllib.request.urlopen(req, timeout=60) as r: | ||
| pay = json.loads(r.read()) | ||
| row["paygen_http"] = 200 | ||
| except urllib.error.HTTPError as e: | ||
| row["paygen_http"] = e.code | ||
| row["paygen_err"] = e.read().decode()[:200] | ||
| results.append(row) | ||
| print(json.dumps(row)); print("-" * 60) | ||
| continue | ||
|
|
||
| raw = base64.b64decode(pay["payment"]) | ||
| payment = lp_rpc_pb2.Payment() | ||
| payment.ParseFromString(raw) | ||
| exp = payment.expected_price if payment.HasField("expected_price") else None | ||
| row["ExpectedPrice"] = _price(exp) | ||
| row["sender"] = "0x" + payment.sender.hex() | ||
| # price match orch (requested cap) vs payment expected price | ||
| of = _frac(orch_price) | ||
| ef = _frac(exp) | ||
| row["price_match_orch"] = (of == ef) if (of is not None and ef is not None) else None | ||
| # payment expected price vs advertised base × overhead | ||
| if ef is not None and adv_base is not None: | ||
| row["match_advertised_x_overhead"] = (ef == adv_base * Fraction(101, 100)) | ||
| results.append(row) | ||
| print(json.dumps(row)); print("-" * 60) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Payment-generation parsing isn't guarded, unlike the orch-info step above.
Lines 97-107 wrap get_orch_info in a broad try/except so one bad capability doesn't kill the sweep. The payment step only catches urllib.error.HTTPError (lines 121-130); a connection error (URLError), non-JSON body, or a 200 response missing pay["payment"] will raise uncaught at lines 123/132-134 and abort the entire multi-capability loop, losing accumulated results and the summary/JSON dump at the end.
Proposed fix
try:
with urllib.request.urlopen(req, timeout=60) as r:
pay = json.loads(r.read())
row["paygen_http"] = 200
except urllib.error.HTTPError as e:
row["paygen_http"] = e.code
row["paygen_err"] = e.read().decode()[:200]
results.append(row)
print(json.dumps(row)); print("-" * 60)
continue
+ except Exception as exc:
+ row["paygen_err"] = str(exc)[:200]
+ results.append(row)
+ print(json.dumps(row)); print("-" * 60)
+ continue
- raw = base64.b64decode(pay["payment"])
+ raw = base64.b64decode(pay.get("payment", ""))
+ if not raw:
+ row["paygen_err"] = "response missing 'payment' field"
+ results.append(row)
+ print(json.dumps(row)); print("-" * 60)
+ continue📝 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
| # generate-live-payment | |
| payload = { | |
| "orchestrator": base64.b64encode(info.SerializeToString()).decode("ascii"), | |
| "type": "byoc", | |
| "capabilities": base64.b64encode(byoc_caps.SerializeToString()).decode("ascii"), | |
| } | |
| req = urllib.request.Request( | |
| f"{signer_origin}/generate-live-payment", | |
| data=json.dumps(payload).encode(), | |
| headers={"Content-Type": "application/json", "Livepeer-Capability": cap, **headers}, | |
| method="POST", | |
| ) | |
| try: | |
| with urllib.request.urlopen(req, timeout=60) as r: | |
| pay = json.loads(r.read()) | |
| row["paygen_http"] = 200 | |
| except urllib.error.HTTPError as e: | |
| row["paygen_http"] = e.code | |
| row["paygen_err"] = e.read().decode()[:200] | |
| results.append(row) | |
| print(json.dumps(row)); print("-" * 60) | |
| continue | |
| raw = base64.b64decode(pay["payment"]) | |
| payment = lp_rpc_pb2.Payment() | |
| payment.ParseFromString(raw) | |
| exp = payment.expected_price if payment.HasField("expected_price") else None | |
| row["ExpectedPrice"] = _price(exp) | |
| row["sender"] = "0x" + payment.sender.hex() | |
| # price match orch (requested cap) vs payment expected price | |
| of = _frac(orch_price) | |
| ef = _frac(exp) | |
| row["price_match_orch"] = (of == ef) if (of is not None and ef is not None) else None | |
| # payment expected price vs advertised base × overhead | |
| if ef is not None and adv_base is not None: | |
| row["match_advertised_x_overhead"] = (ef == adv_base * Fraction(101, 100)) | |
| results.append(row) | |
| print(json.dumps(row)); print("-" * 60) | |
| # generate-live-payment | |
| payload = { | |
| "orchestrator": base64.b64encode(info.SerializeToString()).decode("ascii"), | |
| "type": "byoc", | |
| "capabilities": base64.b64encode(byoc_caps.SerializeToString()).decode("ascii"), | |
| } | |
| req = urllib.request.Request( | |
| f"{signer_origin}/generate-live-payment", | |
| data=json.dumps(payload).encode(), | |
| headers={"Content-Type": "application/json", "Livepeer-Capability": cap, **headers}, | |
| method="POST", | |
| ) | |
| try: | |
| with urllib.request.urlopen(req, timeout=60) as r: | |
| pay = json.loads(r.read()) | |
| row["paygen_http"] = 200 | |
| except urllib.error.HTTPError as e: | |
| row["paygen_http"] = e.code | |
| row["paygen_err"] = e.read().decode()[:200] | |
| results.append(row) | |
| print(json.dumps(row)); print("-" * 60) | |
| continue | |
| except Exception as exc: | |
| row["paygen_err"] = str(exc)[:200] | |
| results.append(row) | |
| print(json.dumps(row)); print("-" * 60) | |
| continue | |
| raw = base64.b64decode(pay.get("payment", "")) | |
| if not raw: | |
| row["paygen_err"] = "response missing 'payment' field" | |
| results.append(row) | |
| print(json.dumps(row)); print("-" * 60) | |
| continue |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 121-121: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(req, timeout=60)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
[info] 116-116: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 128-128: use jsonify instead of json.dumps for JSON output
Context: json.dumps(row)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 145-145: use jsonify instead of json.dumps for JSON output
Context: json.dumps(row)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.21)
[error] 115-120: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 122-122: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 129-129: Multiple statements on one line (semicolon)
(E702)
[warning] 142-142: Comment contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF003)
[error] 146-146: Multiple statements on one line (semicolon)
(E702)
🤖 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 `@scripts/run53-multicap-probe.py` around lines 109 - 146, Broaden the
exception handling around the payment-generation and parsing flow after the
generate-live-payment request so URLError, malformed JSON, missing payment data,
and protobuf decoding failures are captured per capability instead of aborting
the sweep. Record the failure in row, append and print the row, then continue to
the next capability, while preserving the existing successful payment-matching
logic and final results output.
Comment on lines
+1
to
+9
| # E2E Demo Results — 2026-07-03 — TRUE PRODUCTION (DB-assisted billed E2E attempt) | ||
|
|
||
| **Operator:** `qiang@livepeer.org` (`system:admin`), NaaP prod `https://operator.livepeer.org`. | ||
| **Method:** Automated the previously-manual prereqs via a Neon-API-obtained prod `DATABASE_URL` | ||
| (authorized) + admin flag-override API. Drove the full billed chain to the point of failure. | ||
| **Secrets:** Neon API key and the prod connection URI were handled as sensitive — never written to | ||
| this file, logs, or the report. Connection was via the Neon project `green-base-78237656` ("naap", | ||
| org `Vercel: Livepeer Foundation`), primary branch `br-patient-cake-aigip99y` (`main`), db `neondb`, | ||
| role `neondb_owner`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact production identity and credential-adjacent data from the committed report.
This file contains operator PII, user/application identifiers, deployment and wallet metadata, internal topology, and bearer/token fragments. Keep only non-sensitive evidence here; move full diagnostics to an access-controlled artifact store and reference an evidence ID instead.
Also applies to: 849-855, 4361-4367
🤖 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 `@USER-E2E-DEMO-RESULTS.md` around lines 1 - 9, Redact the committed
USER-E2E-DEMO-RESULTS.md report by removing operator identity,
credential-adjacent details, production URLs and infrastructure identifiers,
wallet/application metadata, and bearer or token fragments, including the
referenced sections. Retain only non-sensitive evidence and replace removed
diagnostics with an access-controlled evidence ID reference.
Comment on lines
+4530
to
+4532
| |---|---|---| | ||
| | `GET /healthz` | **200** | **200** | | ||
| | `POST /generate-live-payment` empty body (no bearer) | **400** `missing orchestrator` | **400** `missing orchestrator` | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not pass M2M secrets in curl arguments.
curl -u "$PMTH_M2M_ID:$PMTH_M2M_SECRET" exposes the secret in the process command line. Use a 0600 netrc/config file or feed curl configuration through stdin instead.
Also applies to: 4914-4921
🤖 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 `@USER-E2E-DEMO-RESULTS.md` around lines 4530 - 4532, Update the curl
reproduction command in the “Reproduce” section to avoid passing M2M credentials
through the command line; provide authentication via a temporary 0600-protected
netrc/config file or curl configuration supplied through stdin, while preserving
the existing request URL and options.
Comment on lines
+5044
to
+5046
| |---|---|---| | ||
| | `GET /healthz` | **200** | **200** | | ||
| | `POST /generate-live-payment` empty body (no bearer) | **400** `missing orchestrator` | **400** `missing orchestrator` | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Base the probe verdict on structured error fields, not body-text substrings.
The supplied run57-lr-auth-vs-pay.py maps any body containing "not a jwt" to auth failure and any body containing "priceinfo" to LR configuration. Parse the JSON error code/message together with the HTTP status, otherwise unrelated failures can be recorded as the wrong root cause.
🤖 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 `@USER-E2E-DEMO-RESULTS.md` around lines 5044 - 5046, Update the verdict logic
in run57-lr-auth-vs-pay.py to parse the structured JSON error code and message
together with the HTTP status before classifying failures. Remove substring-only
checks for “not a jwt” and “priceinfo”, and map verdicts only when the
structured fields match the expected auth or LR-configuration errors; preserve
unrelated failures as their actual status/error rather than misclassifying them.
Comment on lines
+5051
to
+5055
|
|
||
| | probe | prod DMZ | staging preview | | ||
| |---|---|---| | ||
| | `GET /healthz` | **200** | **200** | | ||
| | `POST /generate-live-payment` empty body (no bearer) | **400** `missing orchestrator` | **400** `missing orchestrator` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the report sections and any references to Run 56.
rg -n --context 3 'Run 5[5-7]|Run 56|generate-live-payment|sign-orchestrator-info|not a JWT|missing or zero priceInfo' USER-E2E-DEMO-RESULTS.mdRepository: livepeer/naap
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- exact lines around 5051-5055 ---'
sed -n '5044,5062p' USER-E2E-DEMO-RESULTS.md
echo
echo '--- all Run 56 mentions ---'
rg -n 'Run 56' USER-E2E-DEMO-RESULTS.md || trueRepository: livepeer/naap
Length of output: 2375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('USER-E2E-DEMO-RESULTS.md').read_text()
for needle in ['Run 55', 'Run 56', 'Run 57']:
print(f'[{needle}]', text.count(needle))
PYRepository: livepeer/naap
Length of output: 184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the local neighborhood where the comment points, with line numbers.
nl -ba USER-E2E-DEMO-RESULTS.md | sed -n '5046,5060p'Repository: livepeer/naap
Length of output: 191
Link or remove the Run 56 reference The report cites Run 56 here, but the document has no Run 56 section or link. Add the missing artifact or rephrase the claim to avoid the unsupported reference.
🤖 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 `@USER-E2E-DEMO-RESULTS.md` around lines 5051 - 5055, Update the LR-orch
results section around the “Run 56” mention: either add a valid link or artifact
for Run 56, or rephrase the statement to describe the observed opaque-session
rejection without referencing an unavailable run. Keep the surrounding
authentication findings unchanged.
…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>
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>
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 `@BILLED-E2E-BLOCKER-AUDIT.md`:
- Line 1342: Update the fenced code block at the referenced documentation
section to include a language identifier on its opening fence, using ```text or
the most appropriate language, while preserving the block contents.
In `@STORYBOARD-SIGNER-ROUTING.md`:
- Around line 5-6: Update the composite bearer examples in
STORYBOARD-SIGNER-ROUTING.md to use one underscore-based format consistently,
specifically the repository’s app_<24hex>_<secret> or app_…_pmth_… form; remove
or explicitly label any dotted app_<publicClientId>.pmth_<secret> example as
legacy.
🪄 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: 1f5fc147-faaf-4915-958b-eca5651dd454
📒 Files selected for processing (2)
BILLED-E2E-BLOCKER-AUDIT.mdSTORYBOARD-SIGNER-ROUTING.md
Comment on lines
+5
to
+6
| `type:lv2v`) or the **pymthouse per-key signer** (DMZ, `type:byoc`, composite | ||
| `app_…_pmth_…` bearer, NaaP validate)._ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant doc and locate the cited composite-bearer formats.
git ls-files | rg 'STORYBOARD-SIGNER-ROUTING\.md|pmth|pymthouse|bearer|signer|routing'
echo
echo '--- file outline ---'
ast-grep outline STORYBOARD-SIGNER-ROUTING.md --view expanded || true
echo
echo '--- search for composite bearer grammar ---'
rg -n --hidden -S 'app_<publicClientId>|app_…_pmth_…|pmth_|composite bearer|pymthouse|NaaP validate|type:byoc|type:lv2v' .
echo
echo '--- relevant slices from STORYBOARD-SIGNER-ROUTING.md ---'
nl -ba STORYBOARD-SIGNER-ROUTING.md | sed -n '1,220p'Repository: livepeer/naap
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- STORYBOARD-SIGNER-ROUTING.md (first 220 lines) ---'
nl -ba STORYBOARD-SIGNER-ROUTING.md | sed -n '1,220p'
echo
echo '--- exact composite-key grammar mentions ---'
rg -n -S 'app_<publicClientId>\.pmth_<secret>|app_<24hex>_<secret>|app_<clientId>_pmth_<secret>|app_…_pmth_…|app_.*pmth_' STORYBOARD-SIGNER-ROUTING.md docs packages plugins apps | head -n 200
echo
echo '--- builder-sdk / app key helpers ---'
rg -n -S 'isCompositeApiKey|composite api key|pmth_' apps/web-next/src plugins/developer-api/frontend/src packages docs | head -n 200Repository: livepeer/naap
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- STORYBOARD-SIGNER-ROUTING.md lines 1-180 ---'
sed -n '1,180p' STORYBOARD-SIGNER-ROUTING.md | cat -n
echo
echo '--- exact composite-format references in storyboard and docs ---'
rg -n -S 'app_<publicClientId>\.pmth_<secret>|app_<24hex>_<secret>|app_<clientId>_pmth_<secret>|app_…_pmth_…|pmth_.*composite|composite bearer|canonical composite' STORYBOARD-SIGNER-ROUTING.md docs apps plugins packages | head -n 200
echo
echo '--- builder-sdk helper references ---'
rg -n -S 'isCompositeApiKey|startsWith\(\'pmth_|includes\(\'\.pmth_\'\)|app_.*_pmth_' apps plugins packages docs | head -n 200Repository: livepeer/naap
Length of output: 17100
Use one underscore-based composite-bearer form everywhere.
STORYBOARD-SIGNER-ROUTING.md mixes app_…_pmth_… with the dotted app_<publicClientId>.pmth_<secret> citation; align it to the repo’s app_<24hex>_<secret> / app_…_pmth_… form or mark the dotted variant as legacy.
🤖 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 `@STORYBOARD-SIGNER-ROUTING.md` around lines 5 - 6, Update the composite bearer
examples in STORYBOARD-SIGNER-ROUTING.md to use one underscore-based format
consistently, specifically the repository’s app_<24hex>_<secret> or app_…_pmth_…
form; remove or explicitly label any dotted app_<publicClientId>.pmth_<secret>
example as legacy.
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.
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.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pymthouse-e2e.md (1)
17-19: 🩺 Stability & Availability | 🔵 TrivialAdd a prominent side-effect warning before the runnable probes.
These steps perform real payment generation, orchestrator generation, and metering. Explicitly warn that each run may consume signer reserve and create billed usage, and recommend disposable staging credentials plus bounded runs.
🤖 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 `@pymthouse-e2e.md` around lines 17 - 19, Update the introductory section before the runnable probes to add a prominent warning that each execution performs real payment and orchestrator generation, may consume signer reserve, and creates billed usage. Recommend using disposable staging credentials and bounded runs.
🤖 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 `@pymthouse-e2e.md`:
- Line 21: Specify the fenced code block language for the ASCII flow diagram in
pymthouse-e2e.md by changing its opening fence to use the text language
identifier, `text`, while preserving the diagram contents and closing fence.
---
Nitpick comments:
In `@pymthouse-e2e.md`:
- Around line 17-19: Update the introductory section before the runnable probes
to add a prominent warning that each execution performs real payment and
orchestrator generation, may consume signer reserve, and creates billed usage.
Recommend using disposable staging credentials and bounded runs.
🪄 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: f33f22c8-d1fa-4599-83e0-56a37b5f2cf1
📒 Files selected for processing (1)
pymthouse-e2e.md
| NaaP + pymthouse expose today, end to end, with real payment generation and real | ||
| orchestrator generation: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the fenced block language.
Markdownlint reports MD040 here. Use ```text for this ASCII flow diagram.
Proposed fix
-```
+```text🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@pymthouse-e2e.md` at line 21, Specify the fenced code block language for the
ASCII flow diagram in pymthouse-e2e.md by changing its opening fence to use the
text language identifier, `text`, while preserving the diagram contents and
closing fence.
Source: Linters/SAST tools
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>
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.
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>
…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>
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>
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>
…c 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>
…ce 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>
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>
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>
…ITECTURE 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>
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>
…1-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>
… 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>
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>
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>
3 tasks
seanhanca
added a commit
that referenced
this pull request
Jul 21, 2026
…ed state (#432) Section 1 now reflects live gh + Vercel/GitHub-deployment reality: - naap #430 (signer composite-bearer fix + perf-mode skill): MERGED 99b3d39, prod deploy dpl_HLWWTS3ivsGa5NAMp9qa8J1jj83u READY (operator.livepeer.org). - storyboard #685 (seedance reroute): prod deploy 5530586270 success (confirmed, previously "pending redeploy"). - storyboard #687 (unit_kind/quantity_source): MERGED fc78484, prod success. - storyboard #688 (registry parity): MERGED bda8eef (main HEAD), prod success. - Added simple-infra #106 billing-fix row: still OPEN issue, infra-owned, not deployed (no closing PR). Co-authored-by: Cursor <cursoragent@cursor.com>
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
Supersedes the closed #427 (
feat/opaque-session-signer-endpoint), which incorrectly forwarded the opaquepmth_session as the signer credential for the billed path.Two prior verification runs proved a real auth asymmetry in pymthouse's signer:
/sign-orchestrator-info(unbilled)/generate-live-payment(billed)pmth_sessionapp_…_pmth_…bearersign:jobJWTThe looser unbilled endpoint accepting the opaque session masked the asymmetry.
main'sresolveSignerEndpoint()already forwards the correct compositeapp_<24hex>_pmth_<secret>bearer (or exchanges a barepmth_key viaexchangeApiKeyForSignerSessionfor a signer JWT +signerUrl), so this PR bases offmainfor the smallest correct diff and does the remaining cleanup:mintUserSignerJwtForExternalUser()+ theUserSignerJwtinterface inpymthouse-client.ts. Verified zero non-test call sites (rgacrossapps/); it was merged via feat(billing): mint user-scoped signer JWT for per-key remote signer (P7) #406/fix(billing): forward Builder user-token JWT to the remote signer DMZ (P7) #410 but superseded by fix(pymthouse): emit composite app.pmth_ bearer for DMZ signer #421/feat(pymthouse): adopt new api-key signer-session exchange contract #412/feat(pymthouse): builder-sdk 0.6.0 and RFC 8693 API key exchange #424. Its unit tests are removed too. The live path (mintUserAccessToken,exchangeApiKeyForSignerSession,mintOpaqueSignerSessionForExternalUser) is untouched.resolveSignerEndpoint(): it still@linked the removed user-JWT mint and described forwarding a user-token JWT, contradictingmain's actual composite-bearer implementation. Rewritten to accurately describe (1) the JWT/composite-key-only billed webhook that rejects opaque sessions withInvalid JWT, and (2) the credential resolution order (composite bearer verbatim → bare-key exchange → legacy composite-key mint fallback).mintUserSignerJwtForExternalUsermock +.not.toHaveBeenCalled()assertions from the adapter test.What did NOT change
mainalready forwards the composite bearer correctly; this PR does not reintroduce the opaque-session divergence.resolveSignerEndpoint()remains gated by the front-doorPER_KEY_REMOTE_SIGNER_FLAG(fail-safe on error);PYMTHOUSE_BPP_VALIDATE_FLAG/per_key_remote_signerconventions untouched.Test / typecheck / lint
vitestonpymthouse-client.test.ts+pymthouse-adapter.test.ts→ 32 passed.src/lib/billing src/lib/pymthouse-client.test.ts src/lib/dev-api→ 167 passed.tsc --noEmit: zero errors in the 4 changed files (pre-existing repo-wide errors in unrelated files remain).node_modulespinned@pymthouse/builder-sdk@0.4.6whilepackage.jsonpins0.6.0; syncing to 0.6.0 (whereisCompositeApiKeylives) makes the suite green. This staleness pre-exists onmainand is not touched here.Live probe (test-production)
Auth-gate probe against
pymthouse-signer-test-productionwith the composite bearer (creds env-only, never logged):/generate-live-payment+ composite bearer →400 "missing orchestrator"— i.e. the credential cleared the auth gate (body-shape error, not401 "not a JWT"), confirming the composite-bearer path is accepted on the billed webhook.OrchestratorInfoprotobuf from a reachable orchestrator (the Run 50–54 machinery) and was not re-run here; the composite-bearer 200 on/generate-live-paymentis already recorded in those prior runs.Test plan
livepeer/naap(installs@pymthouse/builder-sdk@0.6.0).Made with Cursor
Summary by CodeRabbit