Conversation
…ld failures (#571) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* Implement native WebRTC cursor overlay matching GFN protocol Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Fix WebRTC cursor overlay lock synchronization Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Fix WebRTC cursor-channel position sync Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Add WebRTC native cursor overlay setting Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* Merge dev into main (#557) * Revert "Merge dev into main (#557)" This reverts commit a3c9b7a. * chore(release): prepare v0.5.0 * Add Android and iOS options to bug report template * Guard Linux AppImage updates on read-only installs Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Kiefer <67562560+Kief5555@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* Implement GFN-style clipboard paste protocol to match official client behavior Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Fix clipboard paste no-op when unavailable Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* docs(readme): fix mobile/platform references and add download badges Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: point Android builds to Discord Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
Contributor
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds native input protocol batching, cursor overlay support, clipboard paste support, release highlights UI and IPC, Escape/pointer-lock guarding, Linux AppImage updater checks, SDP and lag tuning, shortcut normalization, and docs/build updates. ChangesNative Input Protocol v3 Batching
Native Cursor Overlay
Clipboard Paste Support
Release Highlights
Escape Fullscreen / Pointer-Lock Guard
Linux AppImage Updater Write-Permission Check
Streaming Quality Tuning
Keyboard Shortcut Token Normalization
Docs and Build Housekeeping
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant MainProcess
participant ReleaseHighlights
participant GitHubAPI
participant Renderer
MainProcess->>ReleaseHighlights: shouldShowReleaseHighlights(current, lastSeen)
ReleaseHighlights->>GitHubAPI: fetch release body
GitHubAPI-->>ReleaseHighlights: body or null
ReleaseHighlights-->>MainProcess: ReleaseHighlightsPayload
MainProcess->>Renderer: RELEASE_HIGHLIGHTS_SHOW payload
Renderer->>MainProcess: RELEASE_HIGHLIGHTS_ACK
sequenceDiagram
participant Renderer
participant WebRTCClient
participant NativeInputBridge
participant GfnServer
Renderer->>WebRTCClient: user input (key/mouse)
WebRTCClient->>WebRTCClient: sendReliableSingleInput / mouse flush
WebRTCClient->>GfnServer: batched/restamped v3 packets
NativeInputBridge->>WebRTCClient: native-input-ready, native-clipboard-paste, native-input-capture-changed
WebRTCClient->>Renderer: update nativeInputBridgeReady/nativeInputCaptureActive
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: cc9d9b9c77
ℹ️ 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".
Comment on lines
+3793
to
+3795
| if (isCapsLockToggle) { | ||
| // Official GFN gg(): CapsLock keyup sends synthetic keydown then keyup (vk 160). | ||
| const capsVk = 0xa0; |
There was a problem hiding this comment.
When CapsLock is pressed while streaming, the keydown path has already added/sent the real CapsLock VK (0x14) to pressedKeys; this keyup branch sends a synthetic 0xa0 down/up and returns without deleting or sending keyup for the tracked CapsLock key. After that, later CapsLock presses are treated as duplicates and ignored, and the remote session can see CapsLock as held until focus/cleanup releases all keys.
Useful? React with 👍 / 👎.
Comment on lines
+500
to
+504
| for batch in reliable_single_batches { | ||
| for payload in finalize_reliable_single_input_packets(&batch, send_timestamp_us) { | ||
| let _ = input_channels.send_packet(&payload, false); | ||
| } | ||
| } |
There was a problem hiding this comment.
For the Windows native input bridge, a drained batch such as mouse-move then mouse-click is now split into reliable batches and pending_mouse_packets, but this loop sends all reliable clicks/keys before the pending mouse packets loop below. That reverses the order that was just observed locally, so clicks can land at the old remote cursor position after a move-and-click gesture; flush the pending mouse packets before the reliable batch that followed them or otherwise preserve event order.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
opennow-stable/src/renderer/src/App.tsx (1)
1536-1577: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPropagate
nativeCursorOverlaychanges to the active stream.updateSetting()never applies this toggle to the liveGfnWebRtcClient, and the client only reads it from constructor options, so changing it mid-stream has no effect until reconnect. Add a live setter or recreate the client when this setting changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/renderer/src/App.tsx` around lines 1536 - 1577, The updateSetting callback in App.tsx is missing live handling for nativeCursorOverlay, so toggling it only updates saved settings and not the active GfnWebRtcClient. Add this setting to the same live-propagation path used for mouseSensitivity and clipboardPaste by either calling a new client setter from clientRef.current or recreating the client when nativeCursorOverlay changes. Ensure the logic is placed in updateSetting alongside the other per-setting branches so the active stream reflects the change immediately.
🧹 Nitpick comments (8)
opennow-stable/src/renderer/src/gfn/clipboardProtocol.ts (1)
52-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider renaming
clampClipboardTextfor clarity.The function doesn't "clamp" (truncate to fit); it rejects (
null) when the text exceedsmaxBytesor is empty. A name likevalidateClipboardTextwould better reflect the reject-on-overflow semantics and avoid confusing future maintainers who might expect truncation behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/renderer/src/gfn/clipboardProtocol.ts` around lines 52 - 57, The helper named clampClipboardText does not actually clamp/truncate; it validates and rejects invalid input by returning null for empty or oversized text. Rename this function to something that reflects its reject-on-overflow behavior, such as validateClipboardText, and update any references to the function symbol in clipboardProtocol.ts so future readers do not expect truncation semantics.opennow-stable/src/renderer/src/shortcuts.test.ts (1)
33-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding boundary tests for function-key/numpad normalization.
No test exercises
F1/F24acceptance,F25rejection, or numpad digit tokens, which would have caught the canonicalization gap innormalizeFunctionKeyToken(shortcuts.ts Lines 56-64).✅ Example additional tests
+test("normalization accepts valid function keys and rejects out-of-range ones", () => { + assert.equal(normalizeShortcut("Ctrl+F24").valid, true); + assert.equal(normalizeShortcut("Ctrl+F25").valid, false); +}); + +test("normalization accepts numpad digit tokens", () => { + assert.equal(normalizeShortcut("Ctrl+Numpad5").key, "NUMPAD5"); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/renderer/src/shortcuts.test.ts` around lines 33 - 36, The shortcut normalization tests currently cover unknown tokens but miss boundary cases in `normalizeShortcut` and `normalizeFunctionKeyToken`. Add tests that verify function-key acceptance for `F1` and `F24`, rejection for `F25`, and canonicalization/acceptance of numpad digit tokens. Place these near the existing `shortcuts.test.ts` cases so the behavior of `normalizeShortcut` stays pinned across these edge inputs.opennow-stable/scripts/ensure-electron-installed.mjs (1)
18-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging on skip path.
The early exit is silent; adding a short log line would help debug CI/Nix builds when the download is unexpectedly skipped or not skipped.
♻️ Optional diagnostic log
if (process.env.ELECTRON_SKIP_BINARY_DOWNLOAD || process.env.npm_config_electron_skip_binary_download) { + console.log("[ensure-electron-installed] Skipping Electron binary download (env override set)."); process.exit(0); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/scripts/ensure-electron-installed.mjs` around lines 18 - 21, The skip path in ensure-electron-installed.mjs exits silently when ELECTRON_SKIP_BINARY_DOWNLOAD or npm_config_electron_skip_binary_download is set, so add a short diagnostic log before the process.exit(0) in that branch. Update the script’s top-level conditional to emit a clear message indicating Electron download/install was skipped and include which skip flag triggered it, so CI/Nix runs can confirm the control flow.opennow-stable/src/main/releaseHighlights.ts (2)
44-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHand-rolled prerelease comparator can misorder differently-tagged prereleases.
comparePrereleaseonly compares the trailing numeric suffix of each string (.replace(/^.*?(\d+)$/, "$1")). Per the SemVer spec, prerelease identifiers are compared component-by-component, and numeric identifiers always have lower precedence than alphanumeric ones. This implementation instead compares raw trailing digits, so e.g."beta.1"vs"alpha.3"yields1 - 3 < 0, incorrectly rankingbeta.1belowalpha.3, whereas spec-correct ordering ranks1.0.0-alpha < 1.0.0-beta.2because "alpha" < "beta" lexically. This could causeshouldShowReleaseHighlightsto skip showing "What's New" for a genuinely newer prerelease (or vice versa) if the project ever mixes prerelease tag names (alpha/beta/rc).Consider using the well-maintained
semverpackage (semver.gt/semver.compare) instead of a hand-rolled comparator to guarantee spec-correct precedence.Please confirm whether the project ever uses mixed prerelease tag names (e.g. alpha → beta → rc) for OpenNOW releases; if versions only ever use a single prerelease scheme (e.g. always
beta.N), the practical blast radius is smaller, though the correctness gap remains.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/main/releaseHighlights.ts` around lines 44 - 57, The prerelease ordering logic in comparePrerelease is too simplistic and can misorder mixed prerelease tags. Replace the hand-rolled trailing-number comparison in comparePrerelease with semver-based comparison (for example via semver.compare/semver.gt) so prerelease identifiers are ordered component-by-component per SemVer. Keep shouldShowReleaseHighlights using this comparator, and ensure the new logic correctly handles alpha/beta/rc-style version strings.
96-119: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSynchronous file I/O in the Electron main process.
readCache/writeCacheEntryusereadFileSync/writeFileSync, which block the single main-process thread (and therefore all pending IPC dispatch) while the file is read/written. The cache file is small today, but considerfs/promises(readFile/writeFile) to avoid blocking the event loop, especially sincewriteCacheEntryis also invoked from theupdate-downloadedhandler inupdater.ts.♻️ Suggested async rewrite
-import { readFileSync, writeFileSync, existsSync } from "node:fs"; +import { existsSync } from "node:fs"; +import { readFile, writeFile } from "node:fs/promises"; ... -function readCache(): ReleaseNotesCache { +async function readCache(): Promise<ReleaseNotesCache> { try { const path = getCachePath(); if (!existsSync(path)) return {}; - return JSON.parse(readFileSync(path, "utf-8")) as ReleaseNotesCache; + return JSON.parse(await readFile(path, "utf-8")) as ReleaseNotesCache; } catch { return {}; } } -export function writeCacheEntry(version: string, body: string): void { +export async function writeCacheEntry(version: string, body: string): Promise<void> { try { - const cache = readCache(); + const cache = await readCache(); cache[version] = body; - writeFileSync(getCachePath(), JSON.stringify(cache, null, 2), "utf-8"); + await writeFile(getCachePath(), JSON.stringify(cache, null, 2), "utf-8"); } catch (error) { console.warn("[ReleaseHighlights] Failed to write cache:", error); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/main/releaseHighlights.ts` around lines 96 - 119, The cache helpers in releaseHighlights.ts are blocking the Electron main process by using synchronous file I/O in readCache, writeCacheEntry, and readCacheEntry. Refactor these helpers to use fs/promises readFile and writeFile with async/await, and update callers so the cache read/write path no longer blocks IPC or the update-downloaded flow. Keep the same behavior and symbols (readCache, writeCacheEntry, readCacheEntry, getCachePath), but make the cache access asynchronous end-to-end.opennow-stable/src/main/index.ts (1)
1153-1165: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAck handler is fine; GET handler input validation should be tightened at this boundary too.
RELEASE_HIGHLIGHTS_GETforwardsversionstraight togetReleaseHighlightsPayloadwithout validating its shape. See the corresponding comment inreleaseHighlights.tsonfetchFromGitHubfor the downstream URL-construction risk this enables; consider validating/sanitizingversionhere as the first line of defense since this is the actual IPC trust boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/main/index.ts` around lines 1153 - 1165, Tighten the RELEASE_HIGHLIGHTS_GET IPC boundary in ipcMain.handle so version is validated/sanitized before calling getReleaseHighlightsPayload. The current handler in the RELEASE_HIGHLIGHTS_GET branch only strips a leading v and forwards the value, so add a shape check there to accept only expected release-version strings and fall back to app.getVersion() or reject invalid input. Keep RELEASE_HIGHLIGHTS_ACK unchanged.opennow-stable/src/main/linuxUpdaterSupport.ts (1)
104-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDiagnostics logging bypasses shared logger.
The
console.debugcall here for a diagnostics feature (why AppImage updates were disabled) bypasses@shared/loggerand won't be captured by thelogs:exportIPC channel, making it harder for users/support to retrieve this reason from exported logs.♻️ Suggested fix
+import { logger } from "`@shared/logger`"; ... } catch (error) { const reason = error instanceof Error ? error.message : String(error); - console.debug("[AppUpdater] AppImage install directory is not writable:", appImageDirectory, reason); + logger.debug("[AppUpdater] AppImage install directory is not writable:", appImageDirectory, reason); return false; }As per coding guidelines, "Use the shared logger (
@shared/logger) and export logs vialogs:exportIPC channel; avoid bypassing this path for diagnostics features."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/main/linuxUpdaterSupport.ts` around lines 104 - 116, The diagnostics message in canReplaceAppImage is using console.debug, which bypasses the shared logging path and won’t appear in logs:export. Update the AppImage-writability failure branch to log through `@shared/logger` instead of console.debug, keeping the same context (appImageDirectory and reason) so the updater diagnostics are captured by the shared log export flow.Source: Coding guidelines
opennow-stable/src/renderer/src/App.tsx (1)
4439-4453: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate release-highlights dismiss handler.
This
onDismissblock (ack-if-auto → clear payload → clear auto flag) is identical to the one at Lines 4178-4192 in the unauthenticated render path. Extract into a singlehandleDismissReleaseHighlightscallback and reuse it in both places.♻️ Proposed extraction
+ const handleDismissReleaseHighlights = useCallback((): void => { + if (releaseHighlightsIsAuto) { + void window.openNow.ackReleaseHighlights().catch((err) => { + console.warn("[App] Failed to ack release highlights:", err); + }); + } + setReleaseHighlightsPayload(null); + setReleaseHighlightsIsAuto(false); + }, [releaseHighlightsIsAuto]);Then use
onDismiss={handleDismissReleaseHighlights}at both render sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/renderer/src/App.tsx` around lines 4439 - 4453, The release-highlights dismissal logic is duplicated in App’s two render paths, so extract the shared ack-if-auto then clear-payload and clear-auto-flag sequence into a single handleDismissReleaseHighlights callback in App and reuse it for both ReleaseHighlightsModal onDismiss props. Keep the existing behavior intact by moving the current window.openNow.ackReleaseHighlights call, setReleaseHighlightsPayload(null), and setReleaseHighlightsIsAuto(false) into that shared handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@native/opennow-streamer/src/gstreamer_input.rs`:
- Around line 457-497: The batching in
gstreamer_input::collect_reliable_single_input_packets is being flushed too
eagerly before every non-MouseMove event, which prevents final coalescing in
finalize_reliable_single_input_packets. Update the loop over other_events so
flush_current_reliable_singles and collect_pending_mouse_move_packets only run
when pending_mouse_move is set or a MouseMove actually needs to break the batch,
while still preserving the current handling of
encode_native_window_input_payload for the non-mouse events.
In `@opennow-stable/src/main/releaseHighlights.ts`:
- Around line 78-84: The token lookup logic is duplicated in pickRuntimeToken
and updater.ts, so extract the OPENNOW_GH_TOKEN/GH_TOKEN environment-variable
resolution into a shared main-process helper module and have both call sites use
it. Keep the existing behavior of trimming values and returning the first
non-empty token, and update releaseHighlights.ts and the updater path to import
the shared helper so any future env var list changes stay in sync.
In `@opennow-stable/src/renderer/src/App.tsx`:
- Around line 343-366: The nativeCursorOverlay setting is only applied when
creating GfnWebRtcClient, so changes made during an active session are not
reflected immediately. Update App.tsx so the runtime toggle is propagated to the
current active client instance as well, using the existing GfnWebRtcClient setup
path and any state/effect wiring around sendStreamClipboardPaste or the client
creation logic to apply the new value without waiting for a stream restart.
In `@opennow-stable/src/renderer/src/gfn/cursorChannel.ts`:
- Around line 347-393: applyCursor() in cursorChannel.ts should avoid calling
refresh() when the incoming cursor is the same object as this.currentCursor and
no new image load completed, since that triggers unnecessary
rasterizeCurrentCursor()/canvas.toDataURL work on every message. Update the
logic so identical cursor shapes only reuse the existing raster and just
re-apply position/visibility state, while still refreshing after a genuinely new
cursor or a successful image decode/load. Use this.currentCursor,
imageLoadGeneration, applyCursorVisibility(), and refresh() as the key points to
gate the redraw.
In `@opennow-stable/src/renderer/src/gfn/webrtcClient.ts`:
- Around line 375-398: The subsampling in subsampleCoalescedPointerEvents is
dropping real movement by keeping only every stride-th sample, which makes
queueMouseMovement’s accumulated motion smaller than the physical input. Change
the logic in subsampleCoalescedPointerEvents so it preserves the full coalesced
delta across the batch by aggregating movementX/movementY into the output events
instead of sampling a subset, while still limiting packet count. Keep the
existing function signature and return shape, and make sure the MouseDeltaFilter
and later flush paths continue to receive the complete summed movement from the
coalesced samples.
In `@opennow-stable/src/renderer/src/shortcuts.ts`:
- Around line 56-64: normalizeFunctionKeyToken currently validates the
function-key range but returns the original token, so inputs like F01 stay
non-canonical and won’t match real key events. Update normalizeFunctionKeyToken
in shortcuts.ts to return the canonical function-key form derived from the
parsed index (for example, using the numeric value rather than the raw upper
string), and keep normalizeShortcut relying on that helper so persisted shortcut
strings and event.code comparisons use the same F1–F24 representation.
---
Outside diff comments:
In `@opennow-stable/src/renderer/src/App.tsx`:
- Around line 1536-1577: The updateSetting callback in App.tsx is missing live
handling for nativeCursorOverlay, so toggling it only updates saved settings and
not the active GfnWebRtcClient. Add this setting to the same live-propagation
path used for mouseSensitivity and clipboardPaste by either calling a new client
setter from clientRef.current or recreating the client when nativeCursorOverlay
changes. Ensure the logic is placed in updateSetting alongside the other
per-setting branches so the active stream reflects the change immediately.
---
Nitpick comments:
In `@opennow-stable/scripts/ensure-electron-installed.mjs`:
- Around line 18-21: The skip path in ensure-electron-installed.mjs exits
silently when ELECTRON_SKIP_BINARY_DOWNLOAD or
npm_config_electron_skip_binary_download is set, so add a short diagnostic log
before the process.exit(0) in that branch. Update the script’s top-level
conditional to emit a clear message indicating Electron download/install was
skipped and include which skip flag triggered it, so CI/Nix runs can confirm the
control flow.
In `@opennow-stable/src/main/index.ts`:
- Around line 1153-1165: Tighten the RELEASE_HIGHLIGHTS_GET IPC boundary in
ipcMain.handle so version is validated/sanitized before calling
getReleaseHighlightsPayload. The current handler in the RELEASE_HIGHLIGHTS_GET
branch only strips a leading v and forwards the value, so add a shape check
there to accept only expected release-version strings and fall back to
app.getVersion() or reject invalid input. Keep RELEASE_HIGHLIGHTS_ACK unchanged.
In `@opennow-stable/src/main/linuxUpdaterSupport.ts`:
- Around line 104-116: The diagnostics message in canReplaceAppImage is using
console.debug, which bypasses the shared logging path and won’t appear in
logs:export. Update the AppImage-writability failure branch to log through
`@shared/logger` instead of console.debug, keeping the same context
(appImageDirectory and reason) so the updater diagnostics are captured by the
shared log export flow.
In `@opennow-stable/src/main/releaseHighlights.ts`:
- Around line 44-57: The prerelease ordering logic in comparePrerelease is too
simplistic and can misorder mixed prerelease tags. Replace the hand-rolled
trailing-number comparison in comparePrerelease with semver-based comparison
(for example via semver.compare/semver.gt) so prerelease identifiers are ordered
component-by-component per SemVer. Keep shouldShowReleaseHighlights using this
comparator, and ensure the new logic correctly handles alpha/beta/rc-style
version strings.
- Around line 96-119: The cache helpers in releaseHighlights.ts are blocking the
Electron main process by using synchronous file I/O in readCache,
writeCacheEntry, and readCacheEntry. Refactor these helpers to use fs/promises
readFile and writeFile with async/await, and update callers so the cache
read/write path no longer blocks IPC or the update-downloaded flow. Keep the
same behavior and symbols (readCache, writeCacheEntry, readCacheEntry,
getCachePath), but make the cache access asynchronous end-to-end.
In `@opennow-stable/src/renderer/src/App.tsx`:
- Around line 4439-4453: The release-highlights dismissal logic is duplicated in
App’s two render paths, so extract the shared ack-if-auto then clear-payload and
clear-auto-flag sequence into a single handleDismissReleaseHighlights callback
in App and reuse it for both ReleaseHighlightsModal onDismiss props. Keep the
existing behavior intact by moving the current
window.openNow.ackReleaseHighlights call, setReleaseHighlightsPayload(null), and
setReleaseHighlightsIsAuto(false) into that shared handler.
In `@opennow-stable/src/renderer/src/gfn/clipboardProtocol.ts`:
- Around line 52-57: The helper named clampClipboardText does not actually
clamp/truncate; it validates and rejects invalid input by returning null for
empty or oversized text. Rename this function to something that reflects its
reject-on-overflow behavior, such as validateClipboardText, and update any
references to the function symbol in clipboardProtocol.ts so future readers do
not expect truncation semantics.
In `@opennow-stable/src/renderer/src/shortcuts.test.ts`:
- Around line 33-36: The shortcut normalization tests currently cover unknown
tokens but miss boundary cases in `normalizeShortcut` and
`normalizeFunctionKeyToken`. Add tests that verify function-key acceptance for
`F1` and `F24`, rejection for `F25`, and canonicalization/acceptance of numpad
digit tokens. Place these near the existing `shortcuts.test.ts` cases so the
behavior of `normalizeShortcut` stays pinned across these edge inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 63f82929-17f9-43ba-aa11-84002c12c6c5
⛔ Files ignored due to path filters (1)
opennow-stable/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
README.mdflake.nixlocales/en.jsonnative/opennow-streamer/src/gstreamer_input.rsnative/opennow-streamer/src/gstreamer_platform.rsnative/opennow-streamer/src/input.rsnative/opennow-streamer/src/protocol.rsopennow-stable/scripts/ensure-electron-installed.mjsopennow-stable/src/main/escapeFullscreenGuard.test.tsopennow-stable/src/main/escapeFullscreenGuard.tsopennow-stable/src/main/index.tsopennow-stable/src/main/linuxUpdaterSupport.test.tsopennow-stable/src/main/linuxUpdaterSupport.tsopennow-stable/src/main/nativeStreamer/manager.tsopennow-stable/src/main/releaseHighlights.tsopennow-stable/src/main/settings.tsopennow-stable/src/main/updater.tsopennow-stable/src/preload/index.tsopennow-stable/src/renderer/src/App.tsxopennow-stable/src/renderer/src/components/ReleaseHighlightsModal.tsxopennow-stable/src/renderer/src/components/SettingsPage.tsxopennow-stable/src/renderer/src/components/StreamView.tsxopennow-stable/src/renderer/src/gfn/clipboardProtocol.test.tsopennow-stable/src/renderer/src/gfn/clipboardProtocol.tsopennow-stable/src/renderer/src/gfn/cursorChannel.test.tsopennow-stable/src/renderer/src/gfn/cursorChannel.tsopennow-stable/src/renderer/src/gfn/inputProtocol.test.tsopennow-stable/src/renderer/src/gfn/inputProtocol.tsopennow-stable/src/renderer/src/gfn/sdp.test.tsopennow-stable/src/renderer/src/gfn/sdp.tsopennow-stable/src/renderer/src/gfn/webrtcClient.test.tsopennow-stable/src/renderer/src/gfn/webrtcClient.tsopennow-stable/src/renderer/src/lib/streamDiagnostics.tsopennow-stable/src/renderer/src/shortcuts.test.tsopennow-stable/src/renderer/src/shortcuts.tsopennow-stable/src/renderer/src/styles.cssopennow-stable/src/shared/gfn.tsopennow-stable/src/shared/ipc.tsopennow-stable/src/shared/nativeStreamer.ts
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
opennow-stable/src/main/updater.ts (1)
280-290: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnnecessary type erasure via
unknowncast.
UpdateDownloadedEvent extends UpdateInfo, andUpdateInfo.releaseNotesis already typed asnull | string | ReleaseNoteInfo[]in electron-updater 6.8.3, so casting to{ releaseNotes?: unknown }throws away existing type information for no benefit —info.releaseNotescan be used directly with the sametypeof/Array.isArraynarrowing.♻️ Proposed simplification
- const releaseNotesRaw = (info as UpdateDownloadedEvent & { releaseNotes?: unknown }).releaseNotes; + const releaseNotesRaw = info.releaseNotes;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opennow-stable/src/main/updater.ts` around lines 280 - 290, The `releaseNotesRaw` handling in `updater.ts` is erasing useful types by casting `info` to `{ releaseNotes?: unknown }` even though `UpdateDownloadedEvent` already carries `UpdateInfo.releaseNotes` with the correct union type. Remove the `unknown` cast and read `releaseNotes` directly from `info`, then keep the existing `typeof` and `Array.isArray` narrowing in the `downloadedVersion`/`releaseNotesRaw` block so the code stays type-safe without discarding the built-in `UpdateInfo` typing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@opennow-stable/src/main/updater.ts`:
- Around line 280-290: The `releaseNotesRaw` handling in `updater.ts` is erasing
useful types by casting `info` to `{ releaseNotes?: unknown }` even though
`UpdateDownloadedEvent` already carries `UpdateInfo.releaseNotes` with the
correct union type. Remove the `unknown` cast and read `releaseNotes` directly
from `info`, then keep the existing `typeof` and `Array.isArray` narrowing in
the `downloadedVersion`/`releaseNotesRaw` block so the code stays type-safe
without discarding the built-in `UpdateInfo` typing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 226cfe59-c9de-4b07-a3f1-900f94103373
📒 Files selected for processing (15)
native/opennow-streamer/src/gstreamer_input.rsopennow-stable/scripts/ensure-electron-installed.mjsopennow-stable/src/main/githubRuntimeToken.tsopennow-stable/src/main/index.tsopennow-stable/src/main/linuxUpdaterSupport.tsopennow-stable/src/main/releaseHighlights.tsopennow-stable/src/main/updater.tsopennow-stable/src/renderer/src/App.tsxopennow-stable/src/renderer/src/gfn/clipboardProtocol.test.tsopennow-stable/src/renderer/src/gfn/clipboardProtocol.tsopennow-stable/src/renderer/src/gfn/cursorChannel.tsopennow-stable/src/renderer/src/gfn/webrtcClient.test.tsopennow-stable/src/renderer/src/gfn/webrtcClient.tsopennow-stable/src/renderer/src/shortcuts.test.tsopennow-stable/src/renderer/src/shortcuts.ts
✅ Files skipped from review due to trivial changes (1)
- opennow-stable/src/main/githubRuntimeToken.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- opennow-stable/scripts/ensure-electron-installed.mjs
- opennow-stable/src/renderer/src/gfn/clipboardProtocol.test.ts
- opennow-stable/src/renderer/src/shortcuts.ts
- opennow-stable/src/main/releaseHighlights.ts
- opennow-stable/src/renderer/src/gfn/webrtcClient.test.ts
- opennow-stable/src/main/index.ts
- opennow-stable/src/main/linuxUpdaterSupport.ts
- opennow-stable/src/renderer/src/App.tsx
- opennow-stable/src/renderer/src/gfn/cursorChannel.ts
- opennow-stable/src/renderer/src/gfn/webrtcClient.ts
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
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
2 participants
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.
Note
Medium Risk
Changes touch low-level WebRTC input encoding and Win32 capture paths used during live streaming; regressions could affect keyboard, paste, or pointer-lock behavior on Windows native sessions.
Overview
Native Windows streamer input is aligned closer with official GeForce NOW behavior: Ctrl+V triggers a host clipboard-paste event, Caps/Num/Scroll lock state is synced via a new lock-keys packet on capture and key handling, and pointer-lock capture changes are reported to the Electron side. Reliable keyboard/button traffic can be batched under protocol v3 with send-time timestamp restamping; mouse and gamepad packets get the same outer-timestamp treatment.
User-facing copy and packaging: English strings add a native cursor overlay toggle (WebRTC
cursor_channel), a What's new entry in settings, and areleaseHighlightsblock for in-app release notes. README and the bug template now call out iOS, Android, and Switch download paths and de-emphasize in-repo iOS. Nix bumpsnpmDepsHashand symlinkslocalesinto the build so packaged desktop builds pick up Crowdin files.Reviewed by Cursor Bugbot for commit cc9d9b9. Configure here.
Summary by CodeRabbit