Skip to content

fix(operation-queue): guard queue ownership by operation id#5687

Open
piyushkr0509 wants to merge 2 commits into
simstudioai:stagingfrom
piyushkr0509:codex/fix-operation-queue-ownership
Open

fix(operation-queue): guard queue ownership by operation id#5687
piyushkr0509 wants to merge 2 commits into
simstudioai:stagingfrom
piyushkr0509:codex/fix-operation-queue-ownership

Conversation

@piyushkr0509

@piyushkr0509 piyushkr0509 commented Jul 15, 2026

Copy link
Copy Markdown

Summary

  • Track active queue ownership by operation ID to prevent stale events from advancing the queue
  • Add regression coverage for acknowledgements, failures, cancellations, retries, skipped emits, missing targets, and offline resets

Type of Change

  • Bug fix

Testing

  • 24 focused Vitest tests passed
  • TypeScript type-check passed
  • Full lint and lint check passed
  • Strict API validation audit passed

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 15, 2026 1:00pm

Request Review

@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes core client sync queue ordering and offline/retry behavior; regressions could cause stuck ops or duplicate emits, though behavior is heavily regression-tested.

Overview
Replaces the queue’s boolean isProcessing flag with processingOperationId, so only the operation that currently owns the queue can release it and trigger processNextOperation.

releaseQueueOwnership / advanceQueueAfterRelease centralize that rule. confirmOperation and failOperation ignore unknown or non-active IDs (stale acks/failures no longer skip ahead). Cancels, missing-target drops, skipped emits, and retries release ownership only for the active ID; unrelated pending cancellations no longer force a re-emit. The public useOperationQueue().isProcessing getter stays boolean via Boolean(processingOperationId).

Vitest coverage expands for sequential advance, backoff acks, stale events, cancel behavior, retries, and offline mode.

Reviewed by Cursor Bugbot for commit 17b2295. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes queue processing to track ownership by operation ID. The main changes are:

  • Guard queue release against stale acknowledgements and failures.
  • Preserve ownership when unrelated operations are cancelled.
  • Release and reacquire ownership across retries and skipped emits.
  • Add focused tests for queue advancement and reset paths.

Confidence Score: 4/5

The retry acknowledgement path needs a fix before merging.

  • A late acknowledgement during retry backoff removes the pending retry and cancels its timer.
  • The guarded release then skips queue advancement because no operation currently owns the queue.
  • Later pending operations can remain blocked indefinitely.

apps/sim/stores/operation-queue/store.ts

Important Files Changed

Filename Overview
apps/sim/stores/operation-queue/store.ts Adds operation-ID ownership throughout queue completion, failure, retry, cancellation, and reset paths; late confirmations during retry backoff can stall the queue.
apps/sim/stores/operation-queue/store.test.ts Adds focused tests for acknowledgements, failures, cancellations, retries, skipped emits, missing targets, and offline resets.
apps/sim/stores/operation-queue/types.ts Replaces the boolean processing state with a nullable operation owner ID.

Reviews (1): Last reviewed commit: "fix(operation-queue): guard queue owners..." | Re-trigger Greptile

Comment thread apps/sim/stores/operation-queue/store.ts
@piyushkr0509

Copy link
Copy Markdown
Author

@icecrasher321

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.

2 participants