fix(macos): stop cursor reconcile and mailbox waits on cancel - #114
Open
SebTardif wants to merge 1 commit into
Open
fix(macos): stop cursor reconcile and mailbox waits on cancel#114SebTardif wants to merge 1 commit into
SebTardif wants to merge 1 commit into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 availability
|
Codex review: needs real behavior proof before merge. Reviewed August 25, 2026, 9:16 AM ET / 13:16 UTC. ClawSweeper reviewWhat this changesThe PR makes Share This Mac stop cursor-reconciliation retries and video-mailbox timeout waits promptly when sharing is cancelled. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain Current main still has both cancellation gaps, so this PR remains necessary. The patch appears correct and focused, but its terminal transcript proves only a Swift test; a real signed Share This Mac stop trace is still needed before merge. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — step 1 Assertions:
How this fits togetherShare This Mac captures the macOS display and forwards video and cursor updates to connected viewers. Cancellation flows through capture reconciliation and mailbox waits, then determines whether share teardown can finish promptly. flowchart LR
A[User stops sharing] --> B[Capture task cancellation]
B --> C[Cursor reconcile retry]
B --> D[Video mailbox wait]
C --> E[Stop backoff]
D --> F[Cancel timeout]
E --> G[Share teardown]
F --> G
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land this narrow cancellation repair after a redacted stable-path Share This Mac run demonstrates prompt teardown during the affected waits. Do we have a high-confidence way to reproduce the issue? Yes, from source: current main retains neither the mailbox timeout task nor cancellation-aware retry backoff, and the branch adds a focused cancellation regression test; no live share reproduction is attached. Is this the best way to solve the issue? Yes: retaining and cancelling the timeout task plus exiting cancelled backoff is the narrowest repair for the observed ownership gaps, provided the live teardown path is demonstrated. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 7763656acf79. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (21 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
PLAN: cursor reconcile caught every error (including CancellationError) then slept with try?, so cancel became another backoff retry. Mailbox timeout Tasks used try? sleep and were not cancelled when a frame arrived. DO: break on CancellationError, cancel the mailbox timeout Task, and honor Task.sleep cancellation. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
SebTardif
force-pushed
the
fix/macos-cancel-hangs
branch
from
August 25, 2026 09:14
0b6a6cc to
b3e2bd4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
Fixes an issue where stopping Share This Mac (or cancelling a share task) could leave cursor-reconcile retries and mailbox timeout waits running. Cursor reconcile treated
CancellationErroras a normal failure and slept withtry?, so cancel became another backoff retry. The video mailbox spawned an unstructured timeout task that usedtry?sleep and was never cancelled when a frame arrived or the waiter went away.Why This Change Was Made
Honor cancellation in both loops: break when reconcile or its sleep is cancelled, and cancel the mailbox timeout task on offer, finish, replace, and expire.
Task.sleepnow usestry awaitso cancel stops the wait.User Impact
Share teardown no longer waits out a 5s backoff or a leftover mailbox timeout after the user stops sharing.
Evidence
Terminal output from the patched tree. Before the patch, a cancelled mailbox waiter sat for the full 5s timeout:
After the patch the same waiter returns immediately:
Real behavior proof
Behavior or issue addressed: Share cancel no longer retries cursor reconcile or leaves mailbox timeout tasks sleeping.
Real environment tested: macOS, Xcode at /Applications/Xcode.app, swift-testing on arm64, branch fix/macos-cancel-hangs at 0b6a6cc.
Exact steps or command run after this patch:
Evidence after fix: terminal output from the patched tree:
Observed result after fix: A cancelled mailbox
next(timeout: 5s)returns in 0.001s instead of waiting the full 5s. Cursor reconcile treats CancellationError as non-retryable.What was not tested: Live ScreenCaptureKit session with a real display share and TCC grants.
Related: introduced in #90 (cursor reconcile) and #74 (mailbox timeout Task).