Skip to content

fix: finish video persistence and billing after disconnects - #146

Merged
CMGS merged 2 commits into
mainfrom
fix/video-cancellation-settlement
Oct 5, 2026
Merged

CMGS merged 2 commits into
mainfrom
fix/video-cancellation-settlement

Conversation

@CMGS

@CMGS CMGS commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

Async video requests could leave permanent partial state when a client disconnected: a submitted vendor job lost its local handle, or a completed job stayed marked billed without consuming its cost budget. Normal retries could not reconstruct the missing job or re-enter the already claimed settlement.

  • Keep generation, handle persistence, and response finalization in the same server-owned task. This replaces the existing pipeline task instead of nesting another task.
  • After a billable done poll, let one task own the atomic first-settlement claim through ledger writing and budget consumption. Both video read routes use this path; the existing claim still prevents duplicate charges.
  • Both tasks carry the request span, so the access line and the trace fields of a video submit stay on the request span; the OTLP test asserts it.
  • Add SQLite-backed tests that close real TCP connections while the upstream reply or budget lookup is pending. A middleware drop notification confirms the HTTP handler was cancelled before the dependency is released. A repeated successful poll must preserve one ledger row and one budget charge.

The ownership gaps date to async video support in #34; #99 added completion-budget accounting inside the same cancellable path. #145's asynchronous user-budget lookup makes the latter trigger straightforward, but did not introduce the underlying cancellation gap.

Validation:

  • The new tests fail on unchanged db2e82e189b06af6186848c831f9ddb1c15a15d5 and pass with this fix.
  • macOS / Rust 1.98.1: format and Clippy pass; cargo test --workspace --locked --offline reports 704 passed, 0 failed, 4 ignored.
  • PostgreSQL/Redis-gated tests are not counted as locally exercised by that run. Linux CI passed for the first commit with real PostgreSQL/Redis services, both TCP regressions, all workspace tests, control-plane Go/integration and Web/Playwright gates, image build, and cargo-deny. The final head passed the same checks.
  • A live run against real vendors on the final head passed 127 of 129 matrix cases; the two failures reproduce on main (one vendor 404, one account without balance). A real submit whose client disconnected kept its job row and was billed once on the first done poll; the same submit on main left no job row.

Performance: submit still creates one task. Pending, failed, and nonbillable polls create none. Each billable-done poll, including a repeated poll whose claim returns false, now creates one task and awaits its result. Job/account/key values move into that task; storage operations, upstream calls, locks, and retry policy are unchanged. The Rust production diff is net +9 lines. Existing throughput benchmarks do not isolate this video-completion path, so no latency improvement is claimed.

CMGS added 2 commits October 5, 2026 01:42
log_access runs inside the submit task; without the span the gw.* fields are not recorded when the OTLP layer is on.
@CMGS
CMGS marked this pull request as ready for review October 5, 2026 07:31
@CMGS
CMGS merged commit 57b49b0 into main Oct 5, 2026
2 checks passed
@CMGS
CMGS deleted the fix/video-cancellation-settlement branch October 5, 2026 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant