Repository navigation
fix(hints): keep macOS hint-search callbacks in order - #1869
Merged
Merged
Conversation
Each exported bridge of the macOS hint-search field started its own goroutine for the Go callback, so nothing ordered them. A Return typed right after an edit could reach the handler before that edit, and the search confirmed the previous query. The bridges now post to the same callback queue the Windows field uses. The queue runs the callbacks in order on one goroutine, so the main thread still never reaches the handler lock. Start replaces and stops any previous queue, Stop drops the queue before closing the field, and a field that fails to open stops its queue, since the handler never stops a session that did not start.
Contributor
 [Low impact] The PR appears safe to merge; both previous findings are addressed. SummarymacOS hint-search callbacks now share an ordered queue instead of starting separate goroutines.
The changes since the previous review only revise the regression test. No new actionable issues were found. DiagramsequenceDiagram
participant Field as macOS text field
participant Bridge as Go bridges
participant Queue as Callback queue
participant Handler as Mode handler
Field->>Bridge: Query change
Bridge->>Queue: Post query
Field->>Bridge: Return or Escape
Bridge->>Queue: Post control action
Queue->>Handler: Run query callback on queue goroutine
Queue->>Handler: Run control callback afterward
Reviews (2) · Last reviewed commit: "test(hints): pin hint-search callback or..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The bridge test stopped as soon as one query and the confirm had run, so under the old per-callback goroutines it could pass with the second query running after the confirm. It also covered only the Return bridge. The test is now a table over the Return and Escape bridges. The first query blocks until the second query and the control are posted, so the second query is queued on its own, and the test waits for all three callbacks and asserts the exact order.
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
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.
Description
This PR keeps macOS hint-search callbacks in the order they were typed. Each keystroke, Return and Escape in the hint-search field started its own goroutine to reach the mode handler, and nothing ordered those goroutines, so pressing Return right after an edit could confirm the query from before the edit, and Escape could likewise overtake it. The macOS field now hands its callbacks to the same ordered queue the Windows field uses, so they run in the order they arrived.
The callbacks still never run on the main thread, which the mode handler's locking contract requires.
Related Issues
None.
Target Platform
Type of Change
feat: New featurefix: Bug fixrefactor: Code restructuring (no behavior change)perf: Performance improvementdocs: Documentation onlytest: Adding or updating testschore: Build, CI, dependencies, toolingCross-Platform Checklist
//go:build darwin). No build tag changedCodeNotSupported). N/A: no new functionalityGeneral Checklist
just fmt)just cipasses. It runs the same checks as CI, on your host only(format check, lint, vet, build, a CGO-off type-check of the Linux and
Windows builds, tests including a unit
-racepass, vulnerability scan),and CI runs them on macOS, Linux and Windows
subject written for users. This PR squash-merges, so the title is what
Release Please ships in the changelog, not the commits on the branch
Additional Context
-racebefore the fix and passes 50 of 50 after.