Make clipboard paste the default - #811
Conversation
Greptile SummaryThe PR makes clipboard-based paste the default and adds coordinated focus restoration, clipboard preservation, delivery status, retry handling, and integration coverage. Failure recovery still needs to preserve clipboard writes made concurrently by the user or another application.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 719f3de47b
ℹ️ 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".
|
@greptileai review again based on my replies |
FluidVoice PR build readyDownload FluidVoice-PR-811-f3c84014ee63 The artifact contains the ad-hoc-signed app ZIP, Xcode archive, build manifest, and installation instructions. It expires 5 days after the build. Install the app
This build has its own app identity, so its permissions are separate from the release version of FluidVoice. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6542d82fc
ℹ️ 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".
|
|
||
| @MainActor | ||
| final class SystemPasteCommandPoster: PasteCommandPosting { | ||
| private static let vKeyCode: CGKeyCode = 0x09 |
There was a problem hiding this comment.
On keyboard layouts where virtual key code 0x09 does not produce “v”, this posts Command plus a different character instead of Command+V. The previous implementation resolved the key code through the current TIS layout, but this hard-coded replacement still returns success after posting, so the coordinator reports the transcript as delivered even though no paste occurred. Restore layout-aware lookup, including reevaluation after runtime layout switches.
Useful? React with 👍 / 👎.
| @MainActor | ||
| final class PasteDeliveryCoordinator { | ||
| static let shared = PasteDeliveryCoordinator() | ||
| nonisolated static let defaultSettlementDelayNanoseconds: UInt64 = 500_000_000 |
There was a problem hiding this comment.
When a busy or asynchronous target handles the posted Command+V more than 500 ms later, settlement has already restored the original clipboard, so the target pastes the old contents instead of the transcript while delivery was reported successful. The newly added testDefaultSettlementDelayIsLongEnoughForSlowPasteTargets at Tests/FluidDictationIntegrationTests/PasteDeliveryCoordinatorTests.swift:228-230 explicitly requires 1.5 seconds, so the current constant also makes that test fail.
Useful? React with 👍 / 👎.
c6542d8 to
6169400
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6169400fd6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad2a01b3a0
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dd3ea1ff0
ℹ️ 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".
| let isStillFocused = context.pid == TypingService.currentFocusedPID() && | ||
| (context.element == nil || TypingService.isCapturedFocusStillActive(context)) | ||
| return (context.pid, !isStillFocused) |
There was a problem hiding this comment.
When focus moves to another editable element in the same application during recording, this can incorrectly decide that the original field is still focused: isCapturedFocusStillActive ultimately accepts any text-like AX element owned by the same PID, even when it is not CFEqual to the captured element. The restore is then skipped and the global Cmd+V lands in the newly focused field rather than the recorded destination; require exact element identity here or always prepare the captured target when the focused element changed.
Useful? React with 👍 / 👎.
| private static func restoreExactTarget(_ context: RecordingTargetContext) async -> Bool { | ||
| guard AXIsProcessTrusted(), context.window != nil, context.element != nil else { return false } |
There was a problem hiding this comment.
For applications that expose the focused AX element but not kAXFocusedWindowAttribute or kAXMainWindowAttribute, the captured context legitimately has window == nil; this guard rejects it before attempting to set kAXFocusedAttribute on the usable element, and the recovery guard rejects it again. After any focus displacement, delivery therefore reports targetRestoreFailed even though the exact field could still be restored; require the element and treat the window raise as optional, as the previous restoration path did.
Useful? React with 👍 / 👎.
|
This pull request has been marked stale because it has had no activity for 5 days. It will be closed in 2 days if there is no further activity. Add the |
5dd3ea1 to
f3c8401
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3c84014ee
ℹ️ 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".
| if case let .recoverableFailure(failure) = result { | ||
| self.showTextDeliveryFailure(failure, transcript: text) |
There was a problem hiding this comment.
When Paste Last is invoked through its global shortcut while the recording overlay is hidden, this failure path only stores the error and calls finishProcessingKeepingOverlayVisible(). That method does not present an idle overlay—NotchOverlayManager.setProcessing(false) only updates state—so the retry/copy controls remain invisible and the failed paste appears to do nothing. Explicitly show the overlay before reporting the recoverable failure, or provide feedback through a surface that is already visible.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77261b6ef9
ℹ️ 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".
| let didWrite = self.pasteboard.writeIntentionalText(lease.text) | ||
| self.log("intentional_copy_settled generation=\(generation) success=\(didWrite)") |
There was a problem hiding this comment.
When “Copy transcription to clipboard” is enabled and the settlement write fails, writeIntentionalText may already have cleared the general pasteboard before returning false, but this result is only logged after self.lease discarded the original snapshot. A transient pasteboard/server write failure can therefore leave the clipboard empty—losing both the user's previous contents and the requested transcript—even though insertion was reported successful; retain the lease and restore or retry when ownership has not moved to an external copy.
Useful? React with 👍 / 👎.
| guard self.pasteboard.writeTemporaryText(text, sessionID: sessionID) else { | ||
| self.restoreAfterFailure(originalSnapshot, generation: generation, reason: "clipboard_write_failed") |
There was a problem hiding this comment.
If the user or another application copies content after FluidVoice writes its temporary text but before ownership verification or while posting Cmd+V fails, restoreAfterFailure restores the old snapshot without checking ownership, causing the newer clipboard entry to be silently lost.
Knowledge Base Used: Dictation Capture Flow
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/PasteDeliveryCoordinator.swift
Line: 592-593
Comment:
**Failure restore overwrites newer clipboard**
If the user or another application copies content after FluidVoice writes its temporary text but before ownership verification or while posting Cmd+V fails, `restoreAfterFailure` restores the old snapshot without checking ownership, causing the newer clipboard entry to be silently lost.
**Knowledge Base Used:** [Dictation Capture Flow](https://app.greptile.com/altic/-/custom-context/knowledge-base/altic-dev/fluidvoice/-/docs/dictation-capture-flow.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
1 participant
Description
Type of Change
Related Issue or Discussion
#802
Testing
swiftlint --strict --config .swiftlint.yml SourcesScreenshots / Video
Attach screenshots or a video for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes.
Notes
Add reviewer context, rollout notes, or known tradeoffs here.