Repository navigation
fix(meeting): cancel queued model preparation promptly - #1099
kirolos-esmat wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d05719537
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| await fulfillment(of: [active], timeout: 2) | ||
| let second = Task { | ||
| waiting.fulfill() |
There was a problem hiding this comment.
Synchronize cancellation with actual queue admission
Under a loaded XCTest scheduler, waiting.fulfill() can wake the test before this task has entered queue.run and been added to the waiters array; the later 20 ms sleep is only a timing guess. If second.cancel() wins that race, the parent implementation also returns immediately from its leading cancellation check, so this regression test passes without exercising cancellation of an actually queued preparation. Signal actual admission from the queue (for example through a test hook) before cancelling.
Useful? React with 👍 / 👎.
Description
Cancelling a meeting-model preparation queued behind another preparation currently waits for the predecessor to finish. Replace the task chain with a cancellation-aware FIFO waiter queue so the cancelled caller returns promptly without running its preparation or allowing concurrent cache writes.
Type of Change
Related Issue or Discussion
Fixes #1098.
Testing
FluidVoice v1.6.10-beta.8 (
7daffb5), macOS 27.0.1, Apple Silicon arm64.Reproduction: hold the first queue operation on a gate, start and cancel a second preparation, and require cancellation before releasing the gate. The initial regression reproduced the failure on unchanged upstream and passed with this fix. The revised test now confirms actual queue admission before cancellation rather than relying on a delay. Existing cases also verify serialized execution and recovery after failure/cancellation. Admission is observed through a read-only Debug-only queue count; Release behavior is unchanged.
For the initial reproduction, the same upstream actor and three queue XCTest cases were isolated in a temporary Swift package for before/after comparison:
Follow-up validation: the revised three queue tests passed through Xcode. Disabling
cancelWaiterin a temporary copy caused the queued-cancellation expectation to time out, proving the revised test detects a broken cancellation path. Strict lint and code/test/documentation guard reviews passed.Focused Xcode check:
The full suite was not rerun on this isolated branch. The cancellation timing was reproduced at the queue boundary, not through the meeting UI.
Screenshots / Video
This image renders excerpts from the initial before/after test logs; it is not a live app UI screenshot. Copyable output and reproduction steps are included above.
Notes
Codex assisted with the fix, regression test, and submission preparation. I reviewed and approved the complete local diff before submission. This PR contains only the meeting preparation cancellation fix, its regression test, and diagnostic evidence.