Skip to content

fix(nest): close media generation apply/cancel/budget races - #36

Merged
SergiyIva merged 3 commits into
developfrom
fix/media-generation-races
Aug 28, 2026
Merged

fix(nest): close media generation apply/cancel/budget races#36
SergiyIva merged 3 commits into
developfrom
fix/media-generation-races

Conversation

@SergiyIva

Copy link
Copy Markdown
Contributor
  • Serialize applyResult per task within the process so two overlapping apply requests can't both pass the output.applied guard and upload/edit twice.
  • Re-read the task inside applyProviderResult and bail when it is already terminal, so a cancel that lands while a provider status request is in flight (webhook path) no longer resurrects the task as succeeded/failed.
  • Reserve the monthly budget before checking it: the task is enqueued with its estimated cost first, so concurrent requests for the same user near the limit can't both read a stale total and submit paid provider calls; a rejected reservation is failed so its cost is excluded from future sums.

Addresses the three P1 review findings on media-generation.service.ts.

- Serialize applyResult per task within the process so two overlapping apply
  requests can't both pass the output.applied guard and upload/edit twice.
- Re-read the task inside applyProviderResult and bail when it is already
  terminal, so a cancel that lands while a provider status request is in flight
  (webhook path) no longer resurrects the task as succeeded/failed.
- Reserve the monthly budget before checking it: the task is enqueued with its
  estimated cost first, so concurrent requests for the same user near the limit
  can't both read a stale total and submit paid provider calls; a rejected
  reservation is failed so its cost is excluded from future sums.

Addresses the three P1 review findings on media-generation.service.ts.
@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds conditional task-status writes and process-local serialization to narrow media-generation apply, cancellation, and budget races.

  • Adds an atomic expectedStatus contract to memory, Prisma, and Drizzle task stores.
  • Serializes result application per task and budget reservations per user within one service process.
  • Adds focused tests for guarded status updates, concurrent reservations, and cancellation during webhook status retrieval.

Confidence Score: 3/5

The PR does not yet appear safe to merge because cross-instance reservations can reject all affordable contenders and cancellation can still be followed by stale processing side effects.

The process-local budget chain cannot order reservations made by separate service instances, allowing every contender to reject after observing the combined reservations. Separately, a processing result can successfully write running, then lose a race to cancellation before appending and publishing stale provider-status side effects.

Files Needing Attention: packages/nest/src/media-generation.service.ts

Important Files Changed

Filename Overview
packages/nest/src/media-generation.service.ts Adds apply serialization, guarded provider-result writes, and reserve-before-check budget handling.
packages/core/src/system/ports.ts Extends the task-store status-update contract with an optional atomic expected-status guard.
packages/core/src/system/memory.ts Implements the expected-status guard synchronously for the in-memory task store.
packages/system-drizzle/src/stores/ai-task-store.ts Implements guarded status updates with a conditional SQL predicate and current-row fallback.
packages/system-prisma/src/stores/ai-task-store.ts Implements guarded status updates through updateMany followed by a current-row read.
packages/nest/test/media-generation.service.test.ts Adds coverage for single-instance budget serialization and cancellation while provider status retrieval is in flight.

Reviews (3): Last reviewed commit: "docs(nest): document media generation bu..." | Re-trigger Greptile

Comment thread packages/nest/src/media-generation.service.ts Outdated
Comment thread packages/nest/src/media-generation.service.ts
Follow-up to the review on the previous fix:

- Add an optional `expectedStatus` guard to IAiTaskStore.updateStatus so a
  status write only lands while the task is still in an expected state — a
  `WHERE status IN (...)` predicate for the SQL stores and a synchronous
  check-and-set in memory. applyProviderResult uses ['pending','running'] so a
  cancel racing an in-flight provider status request can no longer resurrect the
  task, including across nodes on the webhook path.
- Serialize monthly-budget reservations per user so two concurrent requests near
  the limit admit exactly one instead of both rejecting (or both overspending).

Addresses the two P1 review findings on PR #36.
Comment thread packages/nest/src/media-generation.service.ts
The per-user budget reservation lock is process-local. Spell out that this is a
single-instance optimization and that cross-instance concurrency relies on the
reserve-then-check safety net, which bounds overspend to in-flight tasks and
never leaks budget permanently. Exact multi-instance enforcement would require a
distributed lock or serializable transaction and is intentionally out of scope
for an estimated soft cap.
@SergiyIva
SergiyIva merged commit e6d85ae into develop Aug 28, 2026
5 checks passed
@SergiyIva
SergiyIva deleted the fix/media-generation-races branch August 28, 2026 12:00
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