Skip to content

fix(orch): apply auto-adjust overhead to advertised CapabilitiesPrices - #3993

Open
seanhanca wants to merge 31 commits into
masterfrom
fix/byoc-cap-price-overhead
Open

fix(orch): apply auto-adjust overhead to advertised CapabilitiesPrices#3993
seanhanca wants to merge 31 commits into
masterfrom
fix/byoc-cap-price-overhead

Conversation

@seanhanca

@seanhanca seanhanca commented Jul 18, 2026

Copy link
Copy Markdown

Problem

GetCapabilitiesPrices advertises the un-adjusted base per-capability price, while priceInfo/PriceInfoForCaps bind the overhead-inclusive price (1 + 1/txCostMultiplier) into TicketParams/RecipientRandHash.

On the BYOC billed path the remote signer copies the advertised CapabilitiesPrices into the payment's ExpectedPrice. Because the orchestrator binds a price that includes the auto-adjust overhead but advertises one that does not, the two differ by 1/txCostMultiplier (~1%). When the orchestrator validates the ticket it recomputes recipientRand from ExpectedPrice, which no longer matches RecipientRandHash, so the payment is rejected with invalid recipientRand for ticket recipientRandHash (surfaced to the gateway as Could not parse payment).

Observed on byoc-staging-1 (livepeer/go-livepeer:feat-byoc-payment-fleet-2026-05), which runs with AutoAdjustPrice enabled.

Fix

Extract the overhead computation into a shared applyAutoAdjustOverhead helper and apply it in both:

  • priceInfo (the bound price) — behavior unchanged (pure refactor), and
  • GetCapabilitiesPrices (the advertised price) — now overhead-inclusive,

using the identical PriceToFixed/FixedToPrice rounding so advertised price == bound price. When AutoAdjustPrice is off (or no Recipient is configured), the base price is returned unchanged, matching priceInfo.

Test plan

  • go test ./core/ -run 'TestGetCapabilitiesPrices|TestPriceInfo'
  • Advertise check: grpcurl ... GetOrchestrator | jq .capabilitiesPrices equals the per-cap PriceInfoForCaps value.
  • BYOC E2E: signer ExpectedPrice matches the orchestrator's bound ticket price; payment returns 200.

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Added asynchronous BYOC training with job submission, status tracking, cancellation, payment refresh, checkpoint recovery, and LoRA workflows.
    • Added billing event history with cost, balance, usage, status, and error details.
    • Added WHEP playback support for live BYOC streams.
    • Added SDK service, MCP tools, REST endpoints, workflow clients, and storyboard streaming support.
    • Added local and deployable end-to-end testing stacks.
  • Bug Fixes

    • Improved usage-based charging, payment validation, signature verification, and capability pricing consistency.
  • Documentation

    • Added architecture, SDK, deployment, streaming, testing, and provider-extension guides.

seanhanca and others added 30 commits March 11, 2026 18:29
- agent-client.py: workflow DAG engine with concurrent execution for BYOC tasks
- agent-client-sdk.py: SDK-based client using unified submit_job() interface
- MCP server exposing Livepeer BYOC tools (image/video/music generation)
- OpenAPI spec for LangChain/CrewAI/GPT agent integration
- Docker Compose configs for local and GCE deployment
- .mcp.json for Claude Code MCP integration

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implements async training job lifecycle across the full BYOC stack:
- Orchestrator: submit/status/cancel/list handlers with in-memory job store
- Incremental charging every 30s (same pattern as SSE streaming), with
  auto-cancellation on insufficient balance
- Gateway: proxies training requests to orchestrator with payment headers
- Adapter/Proxy: pass-through to fal.ai queue API (no changes needed)
- SDK: ByocTrainingRequest/Response/Status with cost/balance fields
- CLI: `train submit/status` subcommands with multi-trainer param mapping
- E2E test suite, docker-compose, SDK guide, and feature summary

Tested end-to-end: 10-step LoRA training via fal-ai/flux-lora-fast-training
producing .safetensors weights through the full BYOC pipeline.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Step-by-step guide with RunPod and Replicate examples showing how to
add new training providers with ~80-120 lines of Python (proxy only).
Covers LoRA, pre-training, RLHF, and multi-provider deployments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deploy separate proxy + adapter containers per provider to support
Replicate and RunPod alongside existing fal-ai. Registers 10 Replicate
models and 1 RunPod endpoint (wan-animate) as BYOC capabilities.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Containerize the livepeer-gateway SDK as a FastAPI REST service deployed
to Cloud Run. Thin clients only need stdlib urllib -- no SDK, protobuf,
grpcio, or av dependencies. Tested e2e: capabilities, image gen,
multi-step image->video pipeline all work through Cloud Run.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Compares agent-client-sdk.py (1070 LOC, SDK dependency),
agent-client-thin.py (348 LOC, stdlib only), and agent-client.py
(raw HTTP). Thin client is 3x less code with zero dependencies.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Agentic enrichment pipeline: 3-phase (decompose → plan → validate)
  with chain-of-thought reasoning, truncated JSON repair, and re-planning
- Storyboard UI (v3.0): DAG layout, wave-based concurrent execution,
  collision-free card positioning, save media, re-plan on failure
- SDK MCP server: routes through Cloud Run SDK Service
- Updated deploy script and MCP config for SDK Service backend

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r playback

Phase 1: nginx + Let's Encrypt TLS on VM (34-134-195-88.nip.io), CORS-enabled
reverse proxy to orchestrator, trickle URL rewriting to public HTTPS domain.

Phase 2: scope-trickle (fal H100) with in-process FrameProcessor — no HTTP
per-frame, direct GPU pipeline. Adapter in lifecycle-only mode calls
scope-trickle /start with public trickle URLs via query params. SDK Service
as MPEG-TS encode/decode proxy (MediaPublish/MediaOutput) with session affinity.

Phase 3: Browser playback via mux.js transmuxer (MPEG-TS → fMP4 → MSE).
WebCodecs H.264 + TSMuxer for direct trickle publish from browser (j0sh pattern).
Prompt Send button + /update-params endpoint on scope-trickle.

Phase 4: Cold start indicators, UX optimization (fast enrichment mode, capability
caching), architecture docs, Playwright E2E tests.

Key files:
- byoc/byoc.go: WHEP handler stub for BYOC gateway (TODO: needs Go build)
- sdk-service/app.py: MPEG-TS proxy, stream sessions, WHEP proxy endpoint
- storyboard/index.html: WebCodecs publish, mux.js subscribe, prompt updates
- storyboard/trickle_publisher.js, mpegts_muxer.js: from j0sh/trickle gist

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- SDK Service /watch: continuous MPEG-TS stream via aiohttp trickle subscribe
  (eliminates per-segment HTTP round-trip, simulates j0sh's /all endpoint)
- Storyboard: /watch streaming fetch → mux.js push (no flush during stream)
  → MSE playback. Falls back to segment-based fetch if /watch unavailable.
- scope-trickle: load longlive with correct 512x512 video mode params
  (noise_scale=0.7, denoising_steps=[1000,750]) — fixes tensor mismatch
- Adapter: use scope-trickle for both text-only and v2v modes
- Verified: /watch streams 59KB/s = ~20fps equivalent throughput

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Chrome supports video/mp2t directly in MSE SourceBuffer, eliminating
the mux.js transmuxer that was causing init segment duplication and
1fps playback. MPEG-TS bytes from /watch stream are fed directly to
MSE for hardware H.264 decode.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ys advertise capabilities prices

- Introduce FlattenBYOCJob (V1 binary format with LP_BYOC_JOB_V1 domain separator) in byoc/types.go
  to prevent cross-protocol signature replay.
- Update gateway sign() and orchestrator verifyJobCreds() to use V1 format; remove V0 legacy path.
- Improve payment error handling in job_orchestrator: reject when payment header is present but
  invalid, always return non-nil balance from processPayment.
- Add BYOC pricing branch in core/orchestrator.go priceInfo() using GetPriceForJob for
  Capability_BYOC capabilities, with sender -> "default" fallback.
- Guard fixed-price lookup with Balances != nil check.
- Always call GetCapabilitiesPrices in orchestratorInfoWithCaps so BYOC capability prices are
  advertised regardless of whether the request includes capabilities.
- Update rpc_test.go: add CapabilitiesPricesError test, fix mock to return error properly,
  assert empty (not nil) CapabilitiesPrices when no external capabilities exist.

Made-with: Cursor
…ling

- Add POST /sign-byoc-job endpoint that signs BYOC job credentials using the V1
  binary format (FlattenBYOCJob) and returns {sender, signature}.
- Add RemoteType_BYOC ("byoc") job type to GenerateLivePayment with time-based
  billing: 120-second preload on first request, elapsed-time billing thereafter.
- Add resolvePriceInfo() to find effective BYOC PriceInfo from OrchestratorInfo
  CapabilitiesPrices (Capability=BYOC, Constraint=<name>).
- Use capability constraint as manifest ID for shared balance tracking when
  manifest_id is omitted in BYOC requests.
- Add ManifestID json tag for proper serialization.
- Add tests for resolvePriceInfo and missing BYOC capability constraint validation.

Made-with: Cursor
Brings the LoRA training pipeline (ce397c7 + 3f72335) onto the
deployed signing/pricing baseline. No conflicts — v2's commits touch
signing/payment plumbing; byoc-e2e-testing's commits add training,
streaming, and test infra. Adds:

- byoc/training.go (528 lines) — orch training submit/status/cancel/list
  handlers, 30s incremental charging
- byoc/training_gateway.go (156 lines) — gateway training proxy with
  payment headers
- byoc/byoc.go — POST /process/train/, GET /process/job/{jobId} routes;
  WHEP playback handler

Once this image is built and rolled onto byoc-staging-1, the SDK's
fal-direct fallback (currently shipping in
sdk-service:nonblocking-fal-direct-2026-05-13) can be removed and
training will flow through the same payment-ticketed pipeline as
inference.
The merge from feat/byoc-e2e-testing added trainingStore: NewTrainingJobStore(24 * time.Hour) at line 265 but didn't bring 'time' into byoc.go's import list (the original file had no time references on the v2 side). Build error: byoc/byoc.go:265:45: undefined: time.
runTrainingJob runs in a goroutine launched from the /process/train/
HTTP handler. It was given the inbound request's ctx, which net/http
cancels the moment the 202 response is written. The orch's outbound
POST to the adapter /train endpoint inherited that cancellation,
manifesting as:

  Post "http://byoc-adapter:9090/train": context canceled

…almost always within the first second, before the adapter even
finished accepting the body.

Fix: use context.WithoutCancel(ctx) so the goroutine keeps the clog
values (training_job_id, capability) but is no longer tied to the
inbound HTTP request lifecycle.

Found via e2e test: /train POST returned 202, but the SDK status
poll immediately reported `status=failed, error=context canceled`.
PR-4 of byoc-payment-fleet-2026-05 plan, addressing two known sharp
edges in runTrainingJob's charge accounting (design doc §3.B):

1. **First-tick grace** (Invariant I7 — pre-billing-window grace).
   chargeTick no longer fires until the adapter has reported a non-
   submitted status at least once. Previously: orch billed for 30s
   of "training" that was really 30s of fal-side queue + zip download
   + GPU spin-up. Now: billing starts the moment we observe the
   adapter is actually running the job. ~5 LOC: seenInProgress flag,
   set on first non-empty/non-submitted status read.

2. **Stalled-adapter pause** (Invariant I1 — pay only for billable
   work). When status polls fail 3+ consecutive times, the orch
   stops accumulating billable time. consecutivePollFails resets on
   any successful poll. While stalled, lastChargeTime is pushed
   forward so accrued time is dropped on the floor.

These are the two §3.B simplifications from design §14.5 — no new
fields, no new persistence, ~30 LOC total.

Refund mechanism (the third §3.B item) deferred to a follow-up PR
once we identify the orch-side credit function (or build one as part
of the payment ledger work in §3.D Redis persistence).

Tests added in same PR — test_training_billing.go covers:
- T1: chargeTick doesn't fire before seenInProgress
- T2: chargeTick fires after seenInProgress observed
- T3: stall threshold reached → billing pauses
- T4: stall recovers → billing resumes

Branch: feat/byoc-payment-fleet-2026-05 (branched from
feat/remote-signer-byoc-v2 per design §19.6).
PR-5 of byoc-payment-fleet-2026-05 plan. Adds the orch-side endpoint
for refresh-on-watermark ticket top-up (design §3.A).

New route:
  POST /process/job/{jobId}/refresh-payment
  Headers: Livepeer-Payment (required), Livepeer-Segment (optional)
  Returns: {"job_id", "new_balance_wei"} on HTTP 200

Behavior:
- Job must exist + not be in terminal state (completed/failed/cancelled)
- Reads Livepeer-Payment header, parses to net.Payment protobuf
- Enforces invariant I6 (sender attribution): payment.Sender must match
  job.sender from the original submit. Mismatch → HTTP 403.
- Calls ProcessPayment (same as inference path) to credit the deposit
  ledger. Idempotency on duplicate nonces is enforced at the PM layer.
- Updates job.Balance + job.UpdatedAt in trainingStore.

Cross-repo dependencies:
- Required by: livepeer/livepeer-python-gateway PR-2 (calls this endpoint)
- Coordinates with: livepeer/simple-infra PR-8 (SDK refresh-watermark
  loop relies on this endpoint being live)
…Tick

Addresses review C1 on PR #3932. The synchronous completion path
(adapterJobID == "") would silently skip billing. Reviewer asked
whether this is intentional or a bug.

Answer: intentional dead-code path for fal-direct training
(fal-ai/flux-lora-fast-training is always async). The branch is
preserved for future sync training providers. Added comment so a
future reviewer doesn't restore billing here without re-reading the
charge accounting model in §3.B of the design doc.

When/if a sync training provider lands, that provider must include
cost info in adapterResp and we'll wire it through chargeTrainingTick
with the actual elapsed seconds — not by re-using the chargeInterval
ticker semantics.
PR-6 of byoc-payment-fleet-2026-05 plan. Adds opt-in persistence to
TrainingJobStore for recovery on orch restart (design §3.D).

**Deviation from design**: §3.D specified Redis. After review, filesystem
JSON checkpoint gives equivalent recovery semantics for the single-
instance BYOC orch today without the operational cost of a new Redis
container or `github.com/redis/go-redis` module dependency. Swap to
Redis if/when the orch becomes multi-instance — the TrainingJobStore
internal interface stays the same; only the persistence backend changes.

Implementation:
- New constructor `NewTrainingJobStoreWithCheckpoint(ttl, dir)` for
  filesystem-backed mode. `NewTrainingJobStore(ttl)` unchanged for
  backward-compat in-memory mode.
- Store/Update writes atomically to {jobID}.json via .tmp + rename.
- Startup sweep reads every {jobID}.json; in-flight jobs
  (submitted/running) are marked `failed_orchestrator_restart`,
  their checkpoint rewritten, and added to the in-memory map.
- TTL cleanup deletes terminal checkpoints alongside in-memory entries.
- Corrupt JSON files logged + skipped, not fatal.

Wiring:
- NewBYOCOrchestratorServer reads TRAINING_CHECKPOINT_DIR env var.
  Set on byoc-staging-1 to enable persistence; unset for local dev.

Invariant satisfied:
- I8 (restart-lost jobs are refunded): sweep marks them as
  failed_orchestrator_restart. Refund issuance is the orch's job
  (PR-10 BillingEvent emission), not the store's — TODO follow-up.

Tests (5, all isolated to byoc/ — verifiable with go test ./byoc/...):
- TestTrainingStoreInMemory: backward-compat
- TestTrainingStoreCheckpoint: disk write on Store + Update
- TestTrainingStoreSweepRecoversInflightAsFailed: I8 invariant
- TestTrainingStoreCorruptCheckpointSkipped: resilience
- TestTrainingStoreAtomicWriteIgnoresTmp: atomic-write safety
…tations

Addresses the load-bearing C1 finding from PR-6 review.

The original Store/Update path released store mu after taking a
snapshot, then re-acquired checkpointMu separately to do the fs write.
Under concurrent Update("job-X") calls, snapshots could write out of
order: T2's later mutation could complete + write S2 to disk before
T1's earlier mutation finished writing S1, leaving stale state on disk.

On orch crash + restart, sweep would see the stale "running" state on
disk for a job that had actually completed, mark it as
failed_orchestrator_restart, and emit a refund — INVERTING invariant I8.

Fix: hold checkpointMu across the WHOLE mutation+snapshot+write
window. This serializes checkpoint writes in mutation order. fs I/O
still happens outside store mu so reader requests aren't blocked.

Refactor: split writeCheckpoint into outer (acquires checkpointMu)
and writeCheckpointLocked (caller must already hold it).

New test: TestTrainingStoreConcurrentUpdatesPreserveOrder. 100
concurrent Updates on same job, final disk state must equal final
memory state. Catches the race definitively.

Note: review also flagged sweep ignoring fs errors past ReadDir
(R3) and missing refund wiring for failed_orchestrator_restart (R2).
Both are deferred to PR-10 (BillingEvent stream) per the design's
"refund issuance is orch's job, not store's" split.
…PR-10)

PR-10 of byoc-payment-fleet-2026-05 (design §16, §3.B). Adds a structured
JSONL audit record emitted on every terminal training-job state so the
off-chain reconciliation pipeline (Invariant I3) has a wei-accurate
record to cross-reference against orch's on-chain ticket-redemption
ledger.

Schema (billing_event.go::BillingEvent):
  event=billing_event, schema_version=1, timestamp, job_id, job_type,
  capability, model_id, user_hash (sha256(sender)[:16]), sender_address,
  status, cost_paid_wei, balance_wei, billable_seconds, wall_seconds,
  started_at, completed_at, billing_started_at?, error_message?

Emitted from every terminal-state path in training.go:
- Sync completion (rare — only if adapter returns inside the 30s submit timeout)
- Async completed / failed / cancelled / timed_out (in-poll)
- Mid-poll cancellation (Cancel API)
- Insufficient-balance cancellation
- Submit-time errors (request build, network, parse, non-2xx adapter)
- Sweep-on-restart (failed_orchestrator_restart) — emits with billable_seconds=0
  since pre-restart charge total is preserved in CostPaidWei but we have no
  record of seconds-charged accuracy

To carry sender identity through orch restarts (the sweep path needs it
to emit a useful BillingEvent), added SenderHex as a serialized field
on TrainingJob, kept in sync via setSender(). Older checkpoints
pre-PR-10 will have SenderHex="" and the sweep emits with zero-address —
operator reconciles those manually from signer logs.

Tests:
  TestHashSender_Deterministic — stable 16-char hex output
  TestTrainingBillingEvent_PopulatedFields — happy path
  TestTrainingBillingEvent_ErrorPath — failure status + empty billing start
  TestTrainingBillingEvent_EmptyCostDefaultsToZero — trims whitespace
  TestBillingEvent_JSONShape — round-trip contract check (rename-breaks-pipeline guard)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In-memory ring buffer (cap 500) of recent BillingEvents, exposed via
GET /admin/billing-events on the orch's HTTP mux. Feeds the storyboard
/payments dashboard so operators can see live billing activity during
the canary flip without grepping docker logs.

Ring buffer is intentionally lossy at the head — older events stay in
stdout (docker logs) for forensics; the buffer is just the live
dashboard cache. Events older than ~500 calls scroll out.

Endpoint shape: GET /admin/billing-events?limit=N. Returns
  { events: BillingEvent[], count, ring_size, schema_ver }
sorted newest-first.

Tests:
  TestBillingEventRing_AppendAndSnapshotOrdering — newest-first contract
  TestBillingEventRing_LimitClampsToBuffer — limit handling
  TestBillingEventRing_DropsOldestPastCap — eviction order

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ates (PR-10b)

PR-10 (training) was the audit-log scaffolding; PR-10b extends it to
the inference path so the /payments dashboard (PR-14) populates with
the canary-flipped caps (all 5 are inference).

byoc/billing_event.go: new helper inferenceBillingEvent(...) — same
struct as trainingBillingEvent but job_type="inference" and a simpler
arg list (no billingStartedAt — inference doesn't have a first-tick
grace).

byoc/job_orchestrator.go: capture balanceBefore at the top of
processJob, emit BillingEvent at every terminal state:
  1. connection error / timeout from worker
  2. 401 Unauthorized from worker
  3. response read error
  4. 4xx from worker (worker-reported error)
  5. successful completion (the main happy path)

cost_paid_wei is computed as balanceBefore - balanceAfter — exactly
what chargeForCompute debited, no re-derivation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(PR-10b fix)

Earlier PR-10b computed cost_paid_wei as balanceBefore - balanceAfter,
but the SDK sends a fresh payment ticket with each /process/request,
which CREDITS the orch's per-sender balance before chargeForCompute
DEBITS the compute cost. Net balance delta is typically ≤ 0 even on
priced traffic, so every event read as cost=0 in the dashboard.

Fix: new computeChargeWei(price, start) helper that re-derives the
same math chargeForCompute uses internally:
  wei = (price.PricePerUnit / price.PixelsPerUnit) × seconds
Used at every inference terminal-state emit site. Drops the
balanceBefore capture since it's no longer load-bearing.

The post-call balance shown in jobPaymentBalanceHdr is unchanged —
it's still the user-facing remaining-balance number.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR-A of pricing-metering-design.md / pricing-metering-plan.md.

When the adapter sets X-Livepeer-Units-Consumed on its response, the
orch debits that many units instead of wall-clock seconds. Header
absent → seconds fallback (bit-for-bit identical to pre-PR-A
behavior — zero regression risk).

Changes:
- New `resolveUnits(start, resp) (int64, string)` helper. Reads
  X-Livepeer-Units-Consumed + X-Livepeer-Units-Kind, falls back to
  ceil(seconds) on absent/malformed/negative.
- `chargeForCompute(start, price, sender, jobID, resp)` — added
  resp param; passes resolved units to DebitFees.
- `computeChargeWei(price, start, resp)` — same units source as
  DebitFees so BillingEvent.cost_paid_wei matches what was billed.
- All 5 inference call sites in job_orchestrator.processJob updated
  to pass resp; nil for the pre-response error path.
- stream_orchestrator.go: 2 call sites updated.
- BillingEvent gains BillableUnits + UnitsKind alongside the existing
  BillableSeconds — both stay in the audit record.
- 5 new unit tests in billing_event_test.go covering header
  absent/present/malformed/zero/kind-missing.

Behavior change for existing fleet: none. No adapter sets the
header yet — that's PR-B. Until then `billable_units == ceil(seconds)`
and `units_kind == "second"`, matching prior wei deduction exactly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI caught two compile errors that `go build ./byoc/...` locally
missed because they were in cmd/livepeer's import path:

- byoc/job_orchestrator.go: import strconv (needed for ParseFloat in
  resolveUnits)
- byoc/stream_orchestrator.go:183: third chargeForCompute call site
  in the stream-start happy path also needs the new resp arg

Full `go build ./...` clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GetCapabilitiesPrices advertised the un-adjusted base per-capability price
while priceInfo/PriceInfoForCaps bound the overhead-inclusive price
(1 + 1/txCostMultiplier) into TicketParams/RecipientRandHash. The remote
signer copies the advertised price into the payment's ExpectedPrice, so the
orchestrator's bound price and the signer's ExpectedPrice differed by
1/txCostMultiplier (~1%), causing "invalid recipientRand for ticket
recipientRandHash" rejections on BYOC payments.

Extract the overhead into a shared applyAutoAdjustOverhead helper and apply it
in both priceInfo (bound price) and GetCapabilitiesPrices (advertised price),
using the identical fixed-point rounding so advertised == bound.
@github-actions github-actions Bot added the go Pull requests that update Go code label Jul 18, 2026
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds BYOC training, billing, structured signing, capability pricing, WHEP/trickle streaming, SDK and MCP services, agent clients, deployment configurations, storyboard tooling, documentation, and E2E validation scripts.

Changes

BYOC orchestration and accounting

Layer / File(s) Summary
Billing and asynchronous training
byoc/*.go
Adds unit-based billing events, training-job persistence and lifecycle routes, checkpoint recovery, payment refresh, and gateway training proxies.
Structured signing and BYOC pricing
byoc/types.go, byoc/utils.go, server/*.go, core/orchestrator.go
Adds deterministic job signatures, remote signing, BYOC-specific price resolution, overhead-adjusted prices, and related RPC coverage.

BYOC SDK, streaming, and E2E tooling

Layer / File(s) Summary
SDK service and live streaming
e2e-byoc-test/sdk-service/*, e2e-byoc-test/storyboard/*, byoc/byoc.go
Adds REST inference/training/enrichment/streaming endpoints, MPEG-TS/trickle transport, WHEP support, and browser playback controls.
Agent clients and MCP interfaces
e2e-byoc-test/agent-client*.py, e2e-byoc-test/mcp-server/*, .mcp.json
Adds direct, SDK-backed, and HTTP-only clients, workflow planning, training commands, MCP tools, and an OpenAPI contract.
E2E deployment and validation
e2e-byoc-test/*.sh, e2e-byoc-test/docker-compose*, e2e-byoc-test/test-*
Adds local and VM deployment stacks, setup commands, media/training test clients, and storyboard streaming validation.

Estimated code review effort: 5 (Critical) | ~120 minutes

Suggested reviewers: rickstaa

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.62% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the stated fix: applying auto-adjust overhead to advertised CapabilitiesPrices.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/byoc-cap-price-overhead
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/byoc-cap-price-overhead

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
server/remote_signer.go (1)

382-420: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Bind continuation state to the BYOC manifest.

State validation checks only the orchestrator address. A caller can reuse state containing another capability’s InitialPrice and balance while supplying a new manifest, producing mismatched payment tickets. Store the request type and manifest in the signed state and reject changes.

🤖 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 `@server/remote_signer.go` around lines 382 - 420, Update the remote payment
state creation and validation flow around stateID, manifestID, and
byocCapability to persist the request type and resolved manifest identifier in
the signed state. When continuing an existing state, reject any change to either
the request type or manifest with a bad-request response, alongside the existing
orchestrator validation; ensure newly created state records these values before
signing.
core/orchestrator.go (1)

275-336: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Canonicalize wire prices even when auto-adjustment is disabled.

The helper’s early return bypasses fixed-point/int64 conversion, but GetCapabilitiesPrices immediately calls Int64(), which truncates oversized components. Meanwhile PriceInfoForCaps canonicalizes them. Always run the final wire conversion after optionally applying overhead so advertised and bound prices remain identical in both modes.

Also applies to: 467-491

🤖 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 `@core/orchestrator.go` around lines 275 - 336, Update applyAutoAdjustOverhead
and the price construction in GetCapabilitiesPrices so every price undergoes the
same final fixed-point/int64 wire canonicalization, regardless of whether
overhead is enabled. Apply overhead conditionally, then normalize the resulting
value before populating net.PriceInfo, including both built-in capability paths
and the BYOC external-capability path, matching PriceInfoForCaps behavior.
🟡 Minor comments (6)
e2e-byoc-test/extend-training-capabilities.md-433-433 (1)

433-433: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the charge-tick calculation.

Four hours is 14,400 seconds, but only 480 intervals of 30 seconds. Clarify whether billing accrues per second and is settled every 30 seconds, or whether each interval is one charge tick.

🤖 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 `@e2e-byoc-test/extend-training-capabilities.md` at line 433, Correct the
charge-tick calculation in the orchestrator billing description: distinguish the
14,400 elapsed seconds from the 480 30-second billing intervals, and explicitly
state whether charges accrue per second with 30-second settlement or each
interval represents one charge tick.
e2e-byoc-test/fat-thin-nosdk-client-comp.md-99-120 (1)

99-120: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not claim that the SDK service maps provider parameters.

The service passes req.params through unchanged, so callers still need the provider’s expected field names unless another documented layer performs the mapping. Update the comparison or implement the mapping centrally.

Also applies to: 138-146

🤖 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 `@e2e-byoc-test/fat-thin-nosdk-client-comp.md` around lines 99 - 120, The
comparison in the “No TRAINER_PARAM_MAP” section incorrectly claims the SDK
Service maps provider parameters. Update it to state that req.params passes
through unchanged and callers must provide provider-specific field names, unless
a documented mapping layer is identified; apply the same correction to the
referenced comparison section.
e2e-byoc-test/storyboard/trickle_publisher.js-71-77 (1)

71-77: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Await deletion before resolving close().

The async method returns before the DELETE completes, preventing callers from detecting failed cleanup and allowing navigation to cancel it.

🤖 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 `@e2e-byoc-test/storyboard/trickle_publisher.js` around lines 71 - 77, Update
the async close function to await the DELETE request before resolving, while
preserving the existing nextController shutdown and error reporting. Ensure
DELETE failures propagate to callers instead of being only logged through an
unawaited catch.
e2e-byoc-test/test-scope-trickle.py-87-88 (1)

87-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject non-positive FPS values.

--fps 0 crashes at 1.0 / args.fps, while negative values silently disable rate limiting.

Proposed fix
     args = parser.parse_args()
+    if args.fps <= 0:
+        parser.error("--fps must be greater than zero")

Also applies to: 123-123

🤖 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 `@e2e-byoc-test/test-scope-trickle.py` around lines 87 - 88, Validate the --fps
argument in the argument parser so only positive values are accepted, preventing
zero division and disabling rate limiting for negative values. Update the fps
configuration in the parser setup near --frames to use the existing
argument-validation mechanism or an equivalent positive-value validator, while
preserving the float type and default of 2.
e2e-byoc-test/DESIGN.md-119-131 (1)

119-131: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Update the obsolete SDK limitation.

This section says BYOC SDK support is deferred, while this stack includes livepeer-gateway BYOC submission/training APIs and SDK-backed clients. Update the recommended paths so users are not directed away from the implemented SDK.

🤖 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 `@e2e-byoc-test/DESIGN.md` around lines 119 - 131, Update the “Gap 3:
livepeer-python-gateway SDK Is LV2V-Focused” section in DESIGN.md to reflect the
implemented livepeer-gateway BYOC submission and training APIs and SDK-backed
clients. Remove the obsolete statement that BYOCClient support is deferred, and
revise the recommended BYOC testing paths to include the supported SDK while
retaining valid curl, Python, or direct HTTP options.
byoc/job_orchestrator.go-614-622 (1)

614-622: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Reject non-finite and out-of-range unit values before converting. ParseFloat accepts NaN, ±Inf, and values outside the int64 range; int64(math.Ceil(f)) collapses them to MinInt64, which then gets clamped to 1, so these requests are billed as 1 unit instead of falling back to seconds.

🤖 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 `@byoc/job_orchestrator.go` around lines 614 - 622, Update the unit parsing
logic around ParseFloat in resolveUnits to reject non-finite values and values
whose ceiled result cannot be represented as int64 before converting. Preserve
the existing warning and defaultSeconds/"second" fallback for all invalid or
out-of-range headers, while retaining the minimum-one-unit clamp for valid
positive values.
🧹 Nitpick comments (1)
byoc/billing_event_test.go (1)

242-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise billingEventRingBuffer.append() instead of duplicating it.

This test can pass even if the production eviction branch breaks. Append billingEventRingSize+1 events to a real buffer and assert that the oldest event was removed.

🤖 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 `@byoc/billing_event_test.go` around lines 242 - 264, Update
TestBillingEventRing_DropsOldestPastCap to call billingEventRingBuffer.append()
for each event instead of reproducing the eviction logic inline. Use a real
buffer with billingEventRingSize+1 events, then assert it retains
billingEventRingSize entries and the oldest event was removed while the newest
remains.
🤖 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 `@byoc/training_gateway.go`:
- Around line 123-144: The request handler currently builds statusURL from
caller-controlled orch_url values, enabling SSRF. Replace direct query/header
usage with an orchestrator origin resolved from the trusted discovery result, or
validate it against the registered-orchestrator allowlist before constructing
the request in the status proxy flow around sendReqWithTimeout.

In `@e2e-byoc-test/agent-client-thin.py`:
- Around line 224-231: Update the video-generation branching around
resolve_capability and tool_generate_video to remove the undefined cap_lower
reference. Check the resolved capability and step text independently so the i2v
path remains available regardless of whether the step contains “lucy”, while
preserving the existing v2v and default branches.

In `@e2e-byoc-test/ARCHITECTURE-live-streaming.md`:
- Around line 303-317: Replace the exposed concrete orchestrator credential in
the architecture documentation with a non-sensitive placeholder for both
orchSecret and ORCH_SECRET, and ensure any deployment configuration using the
published value is rotated to a new secret.

In `@e2e-byoc-test/PLAN-highfps-scope-trickle.md`:
- Around line 41-42: Revoke and rotate the exposed fal credentials, then remove
all committed values: in e2e-byoc-test/PLAN-highfps-scope-trickle.md lines
41-42, replace the administrative and model keys with placeholders; in
e2e-byoc-test/deploy-stream-adapter.sh line 19, remove the credential default
and require FAL_KEY from secret storage; in
e2e-byoc-test/test-storyboard-playback.py line 27, delete the unused embedded
credential. Purge the revoked values from history where practical.

In `@e2e-byoc-test/sdk-service/app.py`:
- Around line 61-72: The public SDK service must not expose anonymous paid
operations without safeguards. In e2e-byoc-test/sdk-service/app.py lines 61-72,
add client authentication and authorization, enforce quotas or rate limits, and
replace wildcard CORS with an explicit allowed-origin policy around the FastAPI
app and CORSMiddleware configuration. In
e2e-byoc-test/fat-thin-nosdk-client-comp.md lines 182-188, qualify the safety
claim to state that provider credentials are hidden but safe public use also
requires these controls.

In `@e2e-byoc-test/storyboard/index.html`:
- Around line 4247-4257: Remove the hardcoded Authorization credential from the
browser-side update flow in the try block, and rotate/revoke the exposed key.
Route the scope-trickle update through the SDK/server service using a
server-held credential, or use the returned scoped liveControlUrl, while
preserving the existing prompt and session_id parameters.
- Around line 4111-4129: Fix PSI generation in TSMuxer by updating _pk() so the
adaptation-field length and bytes are written before the PSI pointer byte, with
payload placement and packet continuity preserved. Update _crc32() to use the
non-reflected MPEG-2 PSI CRC-32 algorithm required by PAT and PMT sections, and
keep _pat() and _pmt() using that corrected checksum.
- Around line 1427-1462: Replace dynamic innerHTML construction in createCard
and the referenced sections with createElement-based DOM assembly, assigning
untrusted titles, refIds, inference data, capability metadata, and training
values via textContent, value, or src as appropriate. Keep only fully static
markup in any remaining innerHTML, and ensure all user/service-controlled
attributes are set through DOM properties rather than interpolated HTML.

In `@e2e-byoc-test/test-byoc.sh`:
- Around line 23-24: Update the counter increments in log_pass and log_fail to
avoid post-increment expressions whose initial zero value returns failure under
set -e. Use an increment form that returns success while preserving the pass and
fail counter behavior.

---

Outside diff comments:
In `@core/orchestrator.go`:
- Around line 275-336: Update applyAutoAdjustOverhead and the price construction
in GetCapabilitiesPrices so every price undergoes the same final
fixed-point/int64 wire canonicalization, regardless of whether overhead is
enabled. Apply overhead conditionally, then normalize the resulting value before
populating net.PriceInfo, including both built-in capability paths and the BYOC
external-capability path, matching PriceInfoForCaps behavior.

In `@server/remote_signer.go`:
- Around line 382-420: Update the remote payment state creation and validation
flow around stateID, manifestID, and byocCapability to persist the request type
and resolved manifest identifier in the signed state. When continuing an
existing state, reject any change to either the request type or manifest with a
bad-request response, alongside the existing orchestrator validation; ensure
newly created state records these values before signing.

---

Major comments:
In @.mcp.json:
- Around line 4-7: Replace the hardcoded user-specific path in the MCP server
configuration’s command arguments with a repository-relative path or supported
workspace variable that resolves correctly from any checkout.

In `@byoc/byoc.go`:
- Around line 266-271: Update the checkpoint initialization branch in the
training-store setup so an error from NewTrainingJobStoreWithCheckpoint is
propagated to startup instead of logging a warning and assigning
NewTrainingJobStore. Preserve checkpoint persistence whenever
TRAINING_CHECKPOINT_DIR is configured, and ensure the surrounding startup flow
returns or surfaces the initialization error.
- Around line 213-224: Update the OutWriter readiness wait in the WHEP handling
block around stream.OutCond so it uses a real ten-second deadline and observes
request cancellation instead of relying on sync.Cond.Wait(). Replace or adapt
the condition-wait mechanism with a deadline-aware channel or timer-based
select, preserving immediate use of stream.OutWriter when it becomes available
and exiting when the timeout or request context is canceled.
- Around line 306-319: Require application-level authentication for the
administrative routes registered in byoc/byoc.go, including job listing/status
and BillingEventsHandler, rather than relying on an optional edge proxy. In
byoc/billing_event.go, authorize requests before returning sender or payment
metadata; in byoc/training.go, authorize job enumeration and restrict payer
metadata to the requesting user or other permitted principals. Apply the
existing authentication mechanism consistently across all three sites.

In `@byoc/job_orchestrator.go`:
- Around line 583-650: Resolve billable units only once in chargeForCompute and
reuse that value for both DebitFees and the audit cost calculation. Update the
debit helper and computeChargeWei flow so the resolved units are returned or
passed through rather than calling resolveUnits independently, preserving header
parsing and seconds fallback behavior.
- Around line 324-334: Close resp.Body before returning from both early-exit
paths in byoc/job_orchestrator.go: the 401 billing branch around lines 324-334
and the http.Flusher-unavailable branch around lines 420-423. Update the
surrounding job orchestration flow so each branch releases the response body
before return, while preserving the existing later body-close ownership path.

In `@byoc/training_gateway.go`:
- Around line 43-48: Update the signing-failure branch in the gateway job
handler around gatewayJob.sign() to write an appropriate error response before
returning, rather than exiting with an implicit successful response. Preserve
the existing error logging and ensure the client receives a failure status and
message.

In `@byoc/training.go`:
- Around line 785-828: The polling loop around elapsed and pollTimeout must
measure the timeout with wall-clock time, not by incrementing elapsed by
pollInterval. Initialize a start time or deadline before the loop and use
time.Since(start) or deadline comparison in the loop condition, while preserving
the existing cancellation, billing, and polling behavior.
- Around line 623-636: Route all training job metering and lifecycle updates
through the training store API so each mutation writes a checkpoint: update the
cost and post-charge balance block at byoc/training.go lines 623-636, refreshed
payment balance at lines 588-595, adapter job ID and running transition at lines
720-729, and initial payment balance at lines 775-783. Reuse the store methods
that invoke writeCheckpointLocked() rather than mutating trainingStore.jobs
entries directly.
- Around line 278-292: The Update method in byoc/training.go (lines 278-292)
must validate the current-to-next status transition while holding s.mu and
reject every update once the job is completed or cancelled, preventing running
updates from overwriting terminal states. Add or update assertions in
byoc/training_store_test.go (lines 191-227) to verify concurrent running updates
cannot replace either terminal state, rather than only checking disk and memory
equality.
- Around line 868-879: Update the first-tick grace transition guarded by
seenInProgress so it only activates for explicit running or in-progress
statuses, excluding failed, cancelled, completed, submitted, and empty statuses.
Keep the billing timestamps and log behavior unchanged once a qualifying running
status is observed.
- Around line 129-181: Update sweepOnStartup and
NewTrainingJobStoreWithCheckpoint so each recovered in-flight job triggers the
documented I8 restart refund before startup recovery completes. Reuse the
existing refund mechanism and checkpointed CostPaidWei/sender data, and ensure
refund failures are surfaced rather than silently treating the sweep as
complete.
- Around line 481-482: Remove the direct
bso.orch.FreeExternalCapabilityCapacity(job.Capability) call from the
cancellation path in runTrainingJob(), relying on its existing deferred cleanup
so the reservation is released exactly once.

In `@byoc/types.go`:
- Line 322: Update FlattenBYOCJob and the corresponding verification path around
TimeoutSeconds to prevent values outside the uint32 payload range from being
accepted. Validate that the signed timeout is within the supported non-negative
uint32 bounds before encoding or verifying, returning an error for out-of-range
values; alternatively, consistently widen the encoded payload in both paths so
distinct timeout values cannot collide.

In `@e2e-byoc-test/agent-client-thin.py`:
- Around line 60-65: Update _sdk_get to catch HTTPError responses and normalize
them using the same behavior as _sdk_post, including the expected 404 response
from /train/{job_id}. Ensure status and capability commands receive the
normalized result instead of terminating with an uncaught traceback.

In `@e2e-byoc-test/agent-client.py`:
- Around line 39-41: Update the SSL context initialization around _ssl_ctx to
keep hostname and certificate verification enabled by default. Add an explicit
development configuration or custom CA bundle path for self-signed deployments,
and apply it only when that option is provided.
- Around line 512-519: The image capability fallback in agent-client.py lines
512-519 incorrectly filters capability names for “image”; update the fallback
used by resolve_cap to select the highest-ranked available entry from
IMAGE_QUALITY, while preserving the nano-banana preference. Apply the same
ranked IMAGE_QUALITY selection in agent-client-sdk.py lines 643-650, without
relying on capability-name text matching.
- Around line 635-700: Track failed steps separately from completed steps in the
execution loop around step_ready, execute_step, and the concurrent future
handling in e2e-byoc-test/agent-client.py:635-700; when a step fails or returns
no artifact, mark it failed and skip or fail all descendants instead of
satisfying their dependencies. Update the dependent execution logic in
e2e-byoc-test/agent-client-sdk.py:764-825 to require artifacts for every
prerequisite and prevent dependents from running when any required artifact is
absent.

In `@e2e-byoc-test/deploy-stream-adapter.sh`:
- Around line 33-48: Update the remote execution around the quoted REMOTE_EOF
heredoc to explicitly forward the local FAL_KEY and ORCH_SECRET values into the
remote shell. Ensure the export statements in the remote adapter startup use
those transferred secrets rather than relying on local expansion or the fallback
secret, while preserving secure handling of the credentials.
- Around line 36-37: Update the dependency installation command in the
deployment script to remove the unconditional success fallback, so pip3 failures
terminate the deployment while preserving the command’s diagnostic output for
troubleshooting.
- Around line 22-29: Make ADAPTER_SRC portable in the deployment script by using
an externally supplied environment value when present, or deriving the source
directory relative to deploy-stream-adapter.sh as the fallback. Keep the
existing rsync behavior and destination unchanged.

In `@e2e-byoc-test/deploy-vm.sh`:
- Around line 24-28: Update deploy-vm.sh to conditionally enable Replicate and
RunPod proxy/adapter services based on non-empty REPLICATE_KEY and RUNPOD_KEY
values, using the corresponding key-specific Docker Compose profiles. Ensure the
same profile gating is applied throughout the service startup logic covered by
the referenced range, while preserving unconditional startup for providers with
available required keys.
- Around line 9-28: Update deploy-vm.sh to stop accepting FAL_KEY, GEMINI_KEY,
REPLICATE_KEY, and RUNPOD_KEY as positional arguments; read them from
environment variables or hidden prompts instead. Preserve VM_IP as the
positional argument, and ensure the generated remote .env file is created with
permissions 600.

In `@e2e-byoc-test/docker-compose.fal.yaml`:
- Around line 17-20: Replace the floating latest orchestrator image in
e2e-byoc-test/docker-compose.fal.yaml:17-20 by building the local branch or
pinning the image to the reviewed commit. Update
e2e-byoc-test/deploy-vm.sh:47-50 to deploy the immutable digest produced from
that same reviewed commit, ensuring both paths run the intended code.

In `@e2e-byoc-test/docker-compose.yaml`:
- Line 153: Update the ORCH_URL value in the Docker Compose configuration to use
the orchestrator’s HTTPS scheme on port 8935, matching the other clients and
deployments. Also update the corresponding orchestrator URLs in the setup.sh
registration flows to HTTPS while preserving the existing host and port.

In `@e2e-byoc-test/fat-thin-nosdk-client-comp.md`:
- Around line 182-188: Update the “Credentials stay server-side” section to
remove the claim that distributing thin clients to untrusted environments is
safe while the service is anonymous. Document that authentication and usage
quotas must be implemented before making that claim, while preserving the
statement that provider credentials remain server-side.

In `@e2e-byoc-test/mcp-server/openapi.yaml`:
- Around line 9-14: Separate the OpenAPI server definitions for adapter/SDK and
orchestrator operations instead of assigning all paths to the orchestrator URL.
Update the /capabilities and related adapter/SDK endpoints to use the SDK
service, while keeping /process/request/{capability} on the orchestrator, using
operation-level servers or separate server declarations as appropriate.

In `@e2e-byoc-test/mcp-server/server-sdk.py`:
- Around line 52-58: Harden the shared media-download flow used by
_download_file in e2e-byoc-test/mcp-server/server-sdk.py (lines 52-58) by
enforcing an allowlisted URL scheme, rejecting resolved local/private/internal
addresses, limiting redirects, and enforcing a maximum response size before
writing the file. Apply the same centralized URL-validation policy to the
direct-orchestrator download path in e2e-byoc-test/mcp-server/server.py (lines
73-79), reusing the shared validator rather than implementing separate checks.

In `@e2e-byoc-test/sdk-service/app.py`:
- Around line 56-59: Replace the insecure SSL context in
e2e-byoc-test/sdk-service/app.py lines 56-59 with configuration that trusts the
deployment CA or pins the expected orchestrator certificate while retaining
hostname validation. Apply the same verified TLS configuration in
e2e-byoc-test/mcp-server/server.py lines 37-39, removing CERT_NONE and ensuring
both service and local-client paths validate orchestrator certificates.
- Around line 1134-1143: The publish flow around _init_stream_session must not
report success or send raw JPEG data when MPEG-TS initialization fails.
Propagate the initialization/encoding failure as an error response and ensure
the stream is not published; alternatively, replace the fallback with a
compatible MPEG-TS encoder. Apply the same behavior to the corresponding flow
near the second affected block.
- Around line 147-180: Update UploadRequest and upload_file to enforce explicit
encoded and decoded payload size limits before and during base64 decoding,
rejecting oversized uploads with HTTP 413 and avoiding unbounded memory use. Add
retention cleanup for files written by upload_file, including an expiry
mechanism that removes old uploads from UPLOAD_DIR, while preserving the
existing URL and response behavior for accepted files.
- Around line 1179-1216: The streaming implementation must not rely on
process-local _stream_sessions and its MediaPublish/MediaOutput objects when
multiple Cloud Run instances are allowed. Either configure this prototype to run
on a single stateful instance, or introduce explicit session ownership and
routing so publish, frame, and stop requests consistently reach the instance
holding each session; preserve session behavior across scaling and restarts.
- Around line 769-781: Update the plan-repair logic for gemini-image and
lucy-i2v dependencies to select replacements through _find_fallback() with the
appropriate incompatible-capability exclusions, rather than unconditionally
choosing flux-schnell or kling-i2v. Ensure each repaired capability is present
in available_caps, and reject the chain when no compatible fallback exists.
- Around line 1154-1165: Update the stream stop flow around the job_request
construction to reuse the capability recorded for the active stream session
instead of hardcoding "scope-live". Persist req.capability when starting the
stream, retrieve it by stream_id during teardown, and use that value in the
Livepeer stop header while preserving the existing request behavior.
- Around line 208-227: Update the async route handlers to offload blocking SDK
helpers— including list_capabilities in get_capabilities, submit_byoc_job,
submit_training_job, wait_for_training, get_training_status, and
_llm_call—through asyncio.to_thread or FastAPI’s threadpool; preserve their
existing arguments, results, and error handling while ensuring the event loop is
not blocked.

In `@e2e-byoc-test/sdk-service/Dockerfile`:
- Around line 18-22: Update the Dockerfile container setup around the app.py
copy and uvicorn CMD to create and select a dedicated non-root user before
starting the service. Ensure the application files are accessible to that user
and preserve the existing PORT-based uvicorn startup behavior.

In `@e2e-byoc-test/setup.sh`:
- Around line 89-93: Update the health-check flow around wait_for_health so
failed checks are not discarded before logging “Stack is running!”. Propagate
failures or explicitly report a degraded stack and exit nonzero, applying the
same behavior to the additional health-check blocks identified near the other
occurrences.

In `@e2e-byoc-test/storyboard/index.html`:
- Around line 3281-3313: Update entriesToZipBase64 to compute a CRC-32 for each
entry’s data and write the result to the local header at offset 14 and the
central directory header at offset 16. Keep the data-descriptor flag unset and
preserve the existing STORE-entry size and offset handling.
- Around line 3117-3131: The dependent-step scheduler around runSubAgent must
only consider a dependency ready when its result represents success, not merely
any non-null result; update the readiness check to exclude {error: ...}
outcomes. When dependencies fail, mark affected descendants as blocked and
remove them from execution rather than invoking them, including in the no-ready
fallback path, while preserving concurrent execution for independently ready
steps.
- Around line 4287-4335: Update the live start handler and transformLive flow to
read the live-video-url value and handle the “Video URL” input instead of only
checking inputSrc. Pass the URL through the /stream/start request in the
expected request field, or invoke URL-to-ingress publishing after stream
creation, so selecting Video URL sends frames to the GPU. Preserve the existing
webcam publisher behavior.
- Around line 2246-2265: Update sdkFetch so the abort timer remains active
through response-body parsing by moving timer cleanup to a finally path that
runs after resp.json() completes or throws. Preserve the existing HTTP error
handling and ensure the timer is cleared for both successful and failed
requests.

In `@e2e-byoc-test/storyboard/mpegts_muxer.js`:
- Around line 19-38: Update TSMuxer initialization so PAT/PMT packets are not
emitted during construction before onData is configured. Accept the output
callback through the constructor or add an explicit start method that calls
initTables after the consumer wires onData, while preserving table generation
and emission behavior.
- Around line 1-17: Update crc32 to use the non-reflected CRC-32/MPEG-2
algorithm required by PAT/PMT, including its polynomial and finalization
semantics. In the muxer initialization flow, ensure tsMuxer.onData is assigned
before initTables() emits the initial PAT/PMT packets, preserving those packets
instead of dropping them.

In `@e2e-byoc-test/storyboard/trickle_publisher.js`:
- Around line 5-15: Update the publisher initialization around create() and
next() so stream creation is guaranteed before any segment request. Cache a
shared create promise to prevent duplicate POSTs, and await that promise at the
start of next() before opening segment zero or subsequent segments. Preserve the
existing create error handling and request behavior.

In `@e2e-byoc-test/test-byoc-client.py`:
- Around line 188-191: Update the health-check calls in
e2e-byoc-test/test-byoc-client.py lines 188-191 so test_health uses the
orchestrator’s /status endpoint while retaining /health for the adapter. In
e2e-byoc-test/test-fal-e2e.py lines 349-367, replace the capability-token probe
with a GET request to ${ORCH}/status, preserving the existing health-check
result handling.

In `@e2e-byoc-test/test-fal-e2e.py`:
- Around line 335-340: Update the all-tests execution flow to skip or replace
test_protocol() when args.via is "adapter", since that test requires the
orchestrator directly. Preserve the existing protocol test for "orch" mode while
ensuring --test all succeeds using only adapter-compatible tests in adapter
mode.
- Around line 96-110: Update call_orch() to generate and include a valid
Livepeer-Payment header in each protocol request alongside Content-Type and
Livepeer. Extend the protocol test assertions to validate the ticket-related
behavior, including ExpectedPrice, RecipientRandHash, and either the resulting
Livepeer-Balance change or the expected payment-validation outcome.

In `@e2e-byoc-test/test-scope-trickle.py`:
- Around line 191-200: Update the script’s final summary flow to return a
nonzero process exit status when out_count is zero or pub_ok is less than
args.frames, while preserving the existing PASS output and successful zero exit
status when all input frames publish and output is received.

In `@e2e-byoc-test/test-storyboard-playback.py`:
- Around line 276-289: Update the playback verdict flow after computing ok so a
false result causes the test process to exit unsuccessfully, while preserving
browser cleanup and publisher completion before failing. Use the existing async
test flow around browser.close() and await pub_task, then raise an assertion or
call sys.exit(1) when playback validation fails.
- Around line 38-45: Update the sample-video setup before av.open in the
playback flow: either require and use a --video argument, or generate
deterministic frames and write /tmp/sample.mp4 before opening it. Ensure the
standalone command works on a fresh machine without assuming the file already
exists.
- Around line 69-76: Update the stream creation flow around the requests.post
call and response parsing to validate the HTTP response and require a valid
stream_id and subscribe_url before continuing. Raise or assert immediately on
request failure or malformed results, preventing publication through an invalid
trickle URL.

In `@e2e-byoc-test/test-training-e2e.py`:
- Around line 214-219: Update both training completion paths around the
final.status == "completed" checks to require final.lora_url to be present and
not "N/A" before returning success. Log or assert the missing artifact as a
failure and avoid marking the E2E test passed; preserve the existing success
logging for valid URLs.
- Around line 73-96: The endpoint tests in test_adapter_train_endpoint and the
related Test 3/Test 4 flows must enforce their contracts instead of treating
unexpected responses as success. Require Test 2 to receive exactly HTTP 400,
require Test 3 to receive its expected status and a non-empty job_id before
polling, and reject HTTP 404 in Test 4; return failure for all other statuses,
missing identifiers, or malformed responses.

In `@server/remote_signer.go`:
- Around line 515-529: Update the elapsed-unit calculation in calculateFee to
use the existing outer timing variables and ceil the elapsed seconds, enforcing
a minimum of one unit; do not multiply by priceInfo.PixelsPerUnit. Align this
behavior with resolveUnits in byoc/job_orchestrator.go so fee calculation and
monitoring use the same interval.
- Around line 287-303: Update the BYOC pricing selection logic around the
top-level check and CapabilitiesPrices loop to require an exact Constraint match
when manifestID is provided, including the top-level candidate. When manifestID
is omitted, only return a valid BYOC price if exactly one candidate exists;
otherwise return no match, preventing ambiguous prices from populating
ExpectedPrice.
- Around line 94-118: Limit the request body in the SignBYOCJob handler before
json.Decoder decodes it by wrapping r.Body with http.MaxBytesReader and an
appropriate maximum size. Ensure oversized payloads are rejected through the
existing decode-error path, preventing unbounded request, request, and
parameters memory usage before FlattenBYOCJob.

---

Minor comments:
In `@byoc/job_orchestrator.go`:
- Around line 614-622: Update the unit parsing logic around ParseFloat in
resolveUnits to reject non-finite values and values whose ceiled result cannot
be represented as int64 before converting. Preserve the existing warning and
defaultSeconds/"second" fallback for all invalid or out-of-range headers, while
retaining the minimum-one-unit clamp for valid positive values.

In `@e2e-byoc-test/DESIGN.md`:
- Around line 119-131: Update the “Gap 3: livepeer-python-gateway SDK Is
LV2V-Focused” section in DESIGN.md to reflect the implemented livepeer-gateway
BYOC submission and training APIs and SDK-backed clients. Remove the obsolete
statement that BYOCClient support is deferred, and revise the recommended BYOC
testing paths to include the supported SDK while retaining valid curl, Python,
or direct HTTP options.

In `@e2e-byoc-test/extend-training-capabilities.md`:
- Line 433: Correct the charge-tick calculation in the orchestrator billing
description: distinguish the 14,400 elapsed seconds from the 480 30-second
billing intervals, and explicitly state whether charges accrue per second with
30-second settlement or each interval represents one charge tick.

In `@e2e-byoc-test/fat-thin-nosdk-client-comp.md`:
- Around line 99-120: The comparison in the “No TRAINER_PARAM_MAP” section
incorrectly claims the SDK Service maps provider parameters. Update it to state
that req.params passes through unchanged and callers must provide
provider-specific field names, unless a documented mapping layer is identified;
apply the same correction to the referenced comparison section.

In `@e2e-byoc-test/storyboard/trickle_publisher.js`:
- Around line 71-77: Update the async close function to await the DELETE request
before resolving, while preserving the existing nextController shutdown and
error reporting. Ensure DELETE failures propagate to callers instead of being
only logged through an unawaited catch.

In `@e2e-byoc-test/test-scope-trickle.py`:
- Around line 87-88: Validate the --fps argument in the argument parser so only
positive values are accepted, preventing zero division and disabling rate
limiting for negative values. Update the fps configuration in the parser setup
near --frames to use the existing argument-validation mechanism or an equivalent
positive-value validator, while preserving the float type and default of 2.

---

Nitpick comments:
In `@byoc/billing_event_test.go`:
- Around line 242-264: Update TestBillingEventRing_DropsOldestPastCap to call
billingEventRingBuffer.append() for each event instead of reproducing the
eviction logic inline. Use a real buffer with billingEventRingSize+1 events,
then assert it retains billingEventRingSize entries and the oldest event was
removed while the newest remains.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 338faa2d-8058-4a99-8b41-612df0822361

📥 Commits

Reviewing files that changed from the base of the PR and between 5e8ab23 and ed8f454.

📒 Files selected for processing (47)
  • .mcp.json
  • byoc/billing_event.go
  • byoc/billing_event_test.go
  • byoc/byoc.go
  • byoc/job_orchestrator.go
  • byoc/stream_orchestrator.go
  • byoc/training.go
  • byoc/training_gateway.go
  • byoc/training_store_test.go
  • byoc/types.go
  • byoc/utils.go
  • core/orchestrator.go
  • e2e-byoc-test/ARCHITECTURE-live-streaming.md
  • e2e-byoc-test/ARCHITECTURE.md
  • e2e-byoc-test/DESIGN.md
  • e2e-byoc-test/PLAN-highfps-scope-trickle.md
  • e2e-byoc-test/SDK-GUIDE.md
  • e2e-byoc-test/agent-client-sdk.py
  • e2e-byoc-test/agent-client-thin.py
  • e2e-byoc-test/agent-client.py
  • e2e-byoc-test/deploy-stream-adapter.sh
  • e2e-byoc-test/deploy-vm.sh
  • e2e-byoc-test/docker-compose-training.yaml
  • e2e-byoc-test/docker-compose.fal.yaml
  • e2e-byoc-test/docker-compose.yaml
  • e2e-byoc-test/extend-training-capabilities.md
  • e2e-byoc-test/fat-thin-nosdk-client-comp.md
  • e2e-byoc-test/lora-feat.md
  • e2e-byoc-test/mcp-server/openapi.yaml
  • e2e-byoc-test/mcp-server/server-sdk.py
  • e2e-byoc-test/mcp-server/server.py
  • e2e-byoc-test/sdk-service/Dockerfile
  • e2e-byoc-test/sdk-service/app.py
  • e2e-byoc-test/setup.sh
  • e2e-byoc-test/storyboard/index.html
  • e2e-byoc-test/storyboard/mpegts_muxer.js
  • e2e-byoc-test/storyboard/trickle_publisher.js
  • e2e-byoc-test/test-byoc-client.py
  • e2e-byoc-test/test-byoc.sh
  • e2e-byoc-test/test-fal-e2e.py
  • e2e-byoc-test/test-scope-trickle.py
  • e2e-byoc-test/test-storyboard-playback.py
  • e2e-byoc-test/test-training-e2e.py
  • server/remote_signer.go
  • server/remote_signer_test.go
  • server/rpc.go
  • server/rpc_test.go

Comment thread byoc/training_gateway.go
Comment on lines +123 to +144
// The orchestrator URL should be passed as query param or header
orchURL := r.URL.Query().Get("orch_url")
if orchURL == "" {
orchURL = r.Header.Get("X-Orchestrator-Url")
}
if orchURL == "" {
http.Error(w, "orch_url query parameter or X-Orchestrator-Url header required", http.StatusBadRequest)
return
}

// Extract job ID from path
path := strings.TrimPrefix(r.URL.Path, "/process/job/")
jobID := strings.Split(path, "/")[0]

statusURL := orchURL + "/process/job/" + jobID
req, err := http.NewRequestWithContext(r.Context(), "GET", statusURL, nil)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}

resp, err := sendReqWithTimeout(req, 10*time.Second)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Do not proxy caller-controlled orch_url values directly.

This is a read-capable SSRF: a client can make the gateway request arbitrary internal or metadata endpoints and receive the response. Resolve the orchestrator from the discovery result or enforce an allowlist of registered orchestrator origins.

🤖 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 `@byoc/training_gateway.go` around lines 123 - 144, The request handler
currently builds statusURL from caller-controlled orch_url values, enabling
SSRF. Replace direct query/header usage with an orchestrator origin resolved
from the trusted discovery result, or validate it against the
registered-orchestrator allowlist before constructing the request in the status
proxy flow around sendReqWithTimeout.

Comment on lines +224 to +231
if any(w in step_lower for w in ("video", "animate", "i2v", "t2v", "v2v")):
cap = resolve_capability(cap_hint, "video")
if "i2v" in cap or "lucy" in cap_lower if "lucy" in step_lower else False:
result = tool_generate_video(prompt, cap, image_url=prev_image_url)
elif "v2v" in cap:
result = tool_generate_video(prompt, cap, video_url=prev_video_url)
else:
result = tool_generate_video(prompt, cap, image_url=prev_image_url if prev_image_url else None)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove the undefined cap_lower reference.

A task such as “animate using lucy” evaluates cap_lower and raises NameError. The conditional expression also suppresses the i2v check whenever the step does not contain “lucy”.

Proposed fix
-            if "i2v" in cap or "lucy" in cap_lower if "lucy" in step_lower else False:
+            if "i2v" in cap:
                 result = tool_generate_video(prompt, cap, image_url=prev_image_url)
📝 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
if any(w in step_lower for w in ("video", "animate", "i2v", "t2v", "v2v")):
cap = resolve_capability(cap_hint, "video")
if "i2v" in cap or "lucy" in cap_lower if "lucy" in step_lower else False:
result = tool_generate_video(prompt, cap, image_url=prev_image_url)
elif "v2v" in cap:
result = tool_generate_video(prompt, cap, video_url=prev_video_url)
else:
result = tool_generate_video(prompt, cap, image_url=prev_image_url if prev_image_url else None)
if any(w in step_lower for w in ("video", "animate", "i2v", "t2v", "v2v")):
cap = resolve_capability(cap_hint, "video")
if "i2v" in cap:
result = tool_generate_video(prompt, cap, image_url=prev_image_url)
elif "v2v" in cap:
result = tool_generate_video(prompt, cap, video_url=prev_video_url)
else:
result = tool_generate_video(prompt, cap, image_url=prev_image_url if prev_image_url else None)
🧰 Tools
🪛 Ruff (0.15.21)

[error] 226-226: Undefined name cap_lower

(F821)

🤖 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 `@e2e-byoc-test/agent-client-thin.py` around lines 224 - 231, Update the
video-generation branching around resolve_capability and tool_generate_video to
remove the undefined cap_lower reference. Check the resolved capability and step
text independently so the i2v path remains available regardless of whether the
step contains “lucy”, while preserving the existing v2v and default branches.

Source: Linters/SAST tools

Comment on lines +303 to +317
-orchestrator -network offchain
-serviceAddr 0.0.0.0:8935
-orchSecret offchain-test-secret-2026
-liveAITrickleHostForRunner 34-134-195-88.nip.io:443 # Public trickle URLs for workers
```

### Adapter (Docker)
```
ORCH_URL=https://byoc_orch:8935 # Internal Docker network
ORCH_SECRET=offchain-test-secret-2026
ADAPTER_CALLBACK_URL=http://byoc_stream_adapter:9093
SCOPE_TRICKLE_ENDPOINT=https://fal.run/Daydream/scope-trickle # Enables lifecycle-only mode
FAL_KEY=<fal-api-key>
CAPABILITIES=[{"name":"scope-live","model_id":"daydream/scope-app","capacity":2}]
REGISTER_INTERVAL=15 # Re-register every 15s

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Remove and rotate the published orchestrator secret.

The same concrete shared credential is documented for both orchSecret and ORCH_SECRET alongside the deployed infrastructure. Replace it with a placeholder and rotate any deployment that used this value.

🧰 Tools
🪛 markdownlint-cli2 (0.23.0)

[warning] 310-310: 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 `@e2e-byoc-test/ARCHITECTURE-live-streaming.md` around lines 303 - 317, Replace
the exposed concrete orchestrator credential in the architecture documentation
with a non-sensitive placeholder for both orchSecret and ORCH_SECRET, and ensure
any deployment configuration using the published value is rotated to a new
secret.

Comment on lines +41 to +42
- **fal admin key**: `513f7292-e6a3-40b4-836f-32a644919d6f:c055899e3c22b36a8309932d83f7163d`
- **fal model key** (used by adapter): `d26bca42-b78f-4933-aa85-7ca8196753e4:280a286603355c3a6a608085c652b754`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Revoke and remove the committed fal credentials.

Provider credentials are embedded across documentation, deployment, and test artifacts. Assume they are compromised, rotate them before merge, purge them from history where practical, and load replacements exclusively from secret storage.

  • e2e-byoc-test/PLAN-highfps-scope-trickle.md#L41-L42: replace both administrative and model keys with placeholders.
  • e2e-byoc-test/deploy-stream-adapter.sh#L19-L19: remove the credential default and require FAL_KEY.
  • e2e-byoc-test/test-storyboard-playback.py#L27-L27: delete the unused embedded credential.
📍 Affects 3 files
  • e2e-byoc-test/PLAN-highfps-scope-trickle.md#L41-L42 (this comment)
  • e2e-byoc-test/deploy-stream-adapter.sh#L19-L19
  • e2e-byoc-test/test-storyboard-playback.py#L27-L27
🤖 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 `@e2e-byoc-test/PLAN-highfps-scope-trickle.md` around lines 41 - 42, Revoke and
rotate the exposed fal credentials, then remove all committed values: in
e2e-byoc-test/PLAN-highfps-scope-trickle.md lines 41-42, replace the
administrative and model keys with placeholders; in
e2e-byoc-test/deploy-stream-adapter.sh line 19, remove the credential default
and require FAL_KEY from secret storage; in
e2e-byoc-test/test-storyboard-playback.py line 27, delete the unused embedded
credential. Purge the revoked values from history where practical.

Comment on lines +61 to +72
app = FastAPI(
title="Livepeer SDK Service",
description="REST API wrapping the Livepeer Python Gateway SDK for thin-client access.",
version="0.1.0",
)

app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_methods=["*"],
allow_headers=["*"],
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

The public SDK service has no client authentication or billing-abuse controls. Hiding provider credentials does not make anonymous paid operations safe.

  • e2e-byoc-test/sdk-service/app.py#L61-L72: require authentication, authorization, quotas/rate limits, and restricted CORS.
  • e2e-byoc-test/fat-thin-nosdk-client-comp.md#L182-L188: qualify the safety claim until those controls exist.
📍 Affects 2 files
  • e2e-byoc-test/sdk-service/app.py#L61-L72 (this comment)
  • e2e-byoc-test/fat-thin-nosdk-client-comp.md#L182-L188
🤖 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 `@e2e-byoc-test/sdk-service/app.py` around lines 61 - 72, The public SDK
service must not expose anonymous paid operations without safeguards. In
e2e-byoc-test/sdk-service/app.py lines 61-72, add client authentication and
authorization, enforce quotas or rate limits, and replace wildcard CORS with an
explicit allowed-origin policy around the FastAPI app and CORSMiddleware
configuration. In e2e-byoc-test/fat-thin-nosdk-client-comp.md lines 182-188,
qualify the safety claim to state that provider credentials are hidden but safe
public use also requires these controls.

Comment on lines +1427 to +1462
function createCard({ type, title, refId, width, height }) {
const id = nextCardId++;
const rid = refId || `out_${id}`;
const pos = nextPosition();
const w = width || CARD_W;
const h = height || CARD_H;

const el = document.createElement('div');
el.className = 'card';
el.id = `card-${id}`;
el.dataset.refId = rid;
el.style.cssText = `left:${pos.x}px; top:${pos.y}px; width:${w}px; height:${h}px;`;

const safeTitle = (title || type).replace(/"/g, '&quot;');

el.innerHTML = `
<div class="card-header">
<span class="card-type-badge ${type}">${type.toUpperCase()}</span>
<input class="card-title" value="${safeTitle}" spellcheck="false" title="ref: ${rid}">
<span class="card-ref">${rid}</span>
<div class="card-controls">
<button class="card-btn live-btn-card" title="Transform Live" style="display:none;">&#9654;</button>
<button class="card-btn save-btn" title="Save media" style="display:none;">&#8615;</button>
<button class="card-btn min-btn" title="Minimize">_</button>
<button class="card-btn close" title="Close">×</button>
</div>
</div>
<div class="card-body">
<div class="placeholder">
<div class="shimmer"></div>
<div class="spinner"></div>
<div class="label">Generating…</div>
</div>
</div>
<div class="resize-handle"></div>
`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Remove dynamic innerHTML construction to prevent DOM XSS.

Remote enrichment IDs/titles, inference URLs/errors, capability metadata, and training values are interpolated into HTML with incomplete escaping. A crafted service response can execute JavaScript in the storyboard origin. Build elements with createElement, textContent, .value, and .src; reserve innerHTML for static markup only.

Also applies to: 1499-1535, 1738-1817, 3485-3492

🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 1441-1461: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: el.innerHTML = <div class="card-header"> <span class="card-type-badge ${type}">${type.toUpperCase()}</span> <input class="card-title" value="${safeTitle}" spellcheck="false" title="ref: ${rid}"> <span class="card-ref">${rid}</span> <div class="card-controls"> <button class="card-btn live-btn-card" title="Transform Live" style="display:none;">&#9654;</button> <button class="card-btn save-btn" title="Save media" style="display:none;">&#8615;</button> <button class="card-btn min-btn" title="Minimize">_</button> <button class="card-btn close" title="Close">×</button> </div> </div> <div class="card-body"> <div class="placeholder"> <div class="shimmer"></div> <div class="spinner"></div> <div class="label">Generating…</div> </div> </div> <div class="resize-handle"></div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(inner-outer-html)


[warning] 1439-1439: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: (title || type).replace(/"/g, '"')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(manual-sanitization)

🤖 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 `@e2e-byoc-test/storyboard/index.html` around lines 1427 - 1462, Replace
dynamic innerHTML construction in createCard and the referenced sections with
createElement-based DOM assembly, assigning untrusted titles, refIds, inference
data, capability metadata, and training values via textContent, value, or src as
appropriate. Keep only fully static markup in any remaining innerHTML, and
ensure all user/service-controlled attributes are set through DOM properties
rather than interpolated HTML.

Source: Linters/SAST tools

Comment on lines +4111 to +4129
function _crc32(buf) {
const table = _crc32.t || (_crc32.t = (() => { const t = new Uint32Array(256); for (let n=0;n<256;n++){let c=n;for(let k=0;k<8;k++) c=(c&1)?(0xEDB88320^(c>>>1)):(c>>>1);t[n]=c;} return t; })());
let crc = 0xFFFFFFFF; for (const b of buf) crc = (crc>>>8)^table[(crc^b)&0xFF]; return (crc^0xFFFFFFFF)>>>0;
}
class TSMuxer {
constructor() { this.ps=188;this.pp=0;this.pm=0x100;this.vp=0x101;this.ap=0x102;this.cc=new Map();this.onData=()=>{};this._init(); }
_init() { this._pat().forEach(p=>this.onData(p)); this._pmt().forEach(p=>this.onData(p)); }
_pk(payload,pid,pus=false,ptr=false) {
const p=new Uint8Array(188).fill(0xFF);p[0]=0x47;p[1]=(pus?0x40:0)|((pid>>8)&0x1F);p[2]=pid&0xFF;
const cc=this.cc.get(pid)??0;let afc=1,off=4;this.cc.set(pid,(cc+1)&0xF);
if(pus&&ptr)p[off++]=0;const bl=188-off;
if(payload.length<bl){afc=3;const s=bl-payload.length-2;p[off++]=s+1;p[off++]=0;if(s>0)p.fill(0xFF,off,off+s);off+=s;}
p[3]=(afc<<4)|(cc&0xF);p.set(payload.subarray(0,188-off),off);return p;
}
_pat(){const s=[0,0xB0,0x0D,0,1,0xC1,0,0,0,1,0xE0|(this.pm>>8),this.pm&0xFF];const c=_crc32(Uint8Array.from(s));s.push((c>>24)&0xFF,(c>>16)&0xFF,(c>>8)&0xFF,c&0xFF);return[this._pk(Uint8Array.from(s),this.pp,true,true)];}
_pmt(){let s=[2,0xB0,0,0,1,0xC1,0,0,0xE0|(this.vp>>8),this.vp&0xFF,0xF0,0,0x1B,0xE0|(this.vp>>8),this.vp&0xFF,0xF0,0,0x0F,0xE0|(this.ap>>8),this.ap&0xFF,0xF0,0];const l=s.length+4-3;s[1]=0xB0|((l>>8)&0xF);s[2]=l&0xFF;const p=Uint8Array.from(s);const c=_crc32(p);const f=new Uint8Array(p.length+4);f.set(p);f.set([(c>>24)&0xFF,(c>>16)&0xFF,(c>>8)&0xFF,c&0xFF],p.length);return[this._pk(f,this.pm,true,true)];}
mux(isV,pts,dts,data){const pid=isV?this.vp:this.ap,sid=isV?0xE0:0xC0;const h=[0,0,1,sid];let pl=data.length+8;if(isV||pl>0xFFFF)pl=0;h.push((pl>>8)&0xFF,pl&0xFF,0x80,0x80,5);
const p=pts;h.push((2<<4)|(((p>>30)&7)<<1)|1,(p>>22)&0xFF,(((p>>15)&0x7F)<<1)|1,(p>>7)&0xFF,((p&0x7F)<<1)|1);
const pes=new Uint8Array(h.length+data.length);pes.set(h);pes.set(data,h.length);let o=0,f=true;while(o<pes.length){const sl=Math.min(pes.length-o,184);this.onData(this._pk(pes.subarray(o,o+sl),pid,f,false));f=false;o+=sl;}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift

Fix malformed PAT/PMT packet generation.

For PSI packets, _pk() writes the pointer byte before the adaptation field, although the header declares adaptation followed by payload. Demuxers therefore parse the pointer as the adaptation length. _crc32() also implements reflected ZIP CRC-32 rather than MPEG-2 PSI CRC-32. The WebCodecs publisher emits invalid program tables as a result.

🤖 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 `@e2e-byoc-test/storyboard/index.html` around lines 4111 - 4129, Fix PSI
generation in TSMuxer by updating _pk() so the adaptation-field length and bytes
are written before the PSI pointer byte, with payload placement and packet
continuity preserved. Update _crc32() to use the non-reflected MPEG-2 PSI CRC-32
algorithm required by PAT and PMT sections, and keep _pat() and _pmt() using
that corrected checksum.

Comment on lines +4247 to +4257
try {
// Call scope-trickle /update-params directly (via fal.run)
const FAL_SCOPE = 'https://fal.run/Daydream/scope-trickle';
const resp = await fetch(`${FAL_SCOPE}/update-params?prompt=${encodeURIComponent(prompt)}&session_id=${liveStreamId}`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Authorization': 'Key d26bca42-b78f-4933-aa85-7ca8196753e4:280a286603355c3a6a608085c652b754',
},
body: '{}',
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Revoke the exposed fal credential and remove it from browser code.

Any visitor can extract and reuse this authorization key. Route prompt updates through the SDK service using a server-held credential—or the returned scoped liveControlUrl—and rotate the committed key immediately.

🤖 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 `@e2e-byoc-test/storyboard/index.html` around lines 4247 - 4257, Remove the
hardcoded Authorization credential from the browser-side update flow in the try
block, and rotate/revoke the exposed key. Route the scope-trickle update through
the SDK/server service using a server-held credential, or use the returned
scoped liveControlUrl, while preserving the existing prompt and session_id
parameters.

Comment on lines +23 to +24
log_pass() { echo -e "${GREEN}[PASS]${NC} $1"; ((pass++)); }
log_fail() { echo -e "${RED}[FAIL]${NC} $1"; ((fail++)); }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Avoid post-increment under set -e.

The first ((pass++)) or ((fail++)) returns status 1 because its previous value is zero, terminating the script after its first result.

Proposed fix
-log_pass() { echo -e "${GREEN}[PASS]${NC} $1"; ((pass++)); }
-log_fail() { echo -e "${RED}[FAIL]${NC} $1"; ((fail++)); }
+log_pass() { echo -e "${GREEN}[PASS]${NC} $1"; ((++pass)); }
+log_fail() { echo -e "${RED}[FAIL]${NC} $1"; ((++fail)); }
📝 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
log_pass() { echo -e "${GREEN}[PASS]${NC} $1"; ((pass++)); }
log_fail() { echo -e "${RED}[FAIL]${NC} $1"; ((fail++)); }
log_pass() { echo -e "${GREEN}[PASS]${NC} $1"; ((++pass)); }
log_fail() { echo -e "${RED}[FAIL]${NC} $1"; ((++fail)); }
🤖 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 `@e2e-byoc-test/test-byoc.sh` around lines 23 - 24, Update the counter
increments in log_pass and log_fail to avoid post-increment expressions whose
initial zero value returns failure under set -e. Use an increment form that
returns success while preserving the pass and fail counter behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants