Add Linux CLI support#1908
Conversation
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| } else if (userAgent.includes("Mac")) { | ||
| if (userAgent.includes("Intel")) { | ||
| } else if (clientPlatform.includes("macos") || userAgent.includes("mac")) { | ||
| if (userAgent.includes("intel")) { |
There was a problem hiding this comment.
Intel Macs don’t always include intel in the UA (I’ve seen x86_64/amd64 instead), so this can mis-route to Apple Silicon.
| if (userAgent.includes("intel")) { | |
| if (userAgent.includes("intel") || userAgent.includes("x86_64") || userAgent.includes("amd64")) { | |
| platform = "apple-intel"; | |
| } else { | |
| platform = "apple-silicon"; | |
| } |
| if ! find_cap_app; then | ||
| install_cap_desktop | ||
| echo "Downloading Cap Desktop AppImage..." | ||
| curl -fL "https://cap.so/download/linux-appimage" -o "$APPIMAGE_PATH" |
There was a problem hiding this comment.
Since this is an installer script, it’s worth making the downloads a bit more robust against transient network failures (and harder to accidentally fetch over a downgraded transport).
| curl -fL "https://cap.so/download/linux-appimage" -o "$APPIMAGE_PATH" | |
| curl --proto '=https' --tlsv1.2 --retry 3 --retry-delay 1 -fL "https://cap.so/download/linux-appimage" -o "$APPIMAGE_PATH" |
| let y_stride = format.stride.max(width); | ||
| let chroma_width = width / 2; | ||
| let chroma_height = height / 2; | ||
| let chroma_stride = y_stride / 2; |
There was a problem hiding this comment.
chroma_stride = y_stride / 2 assumes U/V stride is exactly half the luma stride. If the driver pads format.stride oddly (or it’s not describing the luma plane stride the way we expect), this can produce incorrect plane offsets and out-of-bounds slices. Might be worth either validating y_stride % 2 == 0 (and erroring early) or plumbing per-plane strides from the capture format so we don’t have to guess here.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <a | ||
| href="/download/linux-deb" | ||
| onClick={() => | ||
| trackDownloadClick( | ||
| "other_option_linux_deb", | ||
| "/download/linux-deb", | ||
| ) | ||
| } | ||
| className="text-sm transition-all text-gray-10 hover:underline" | ||
| > | ||
| Debian/Ubuntu | ||
| </a> | ||
| <a | ||
| href="/download/linux-rpm" | ||
| onClick={() => | ||
| trackDownloadClick( | ||
| "other_option_linux_rpm", | ||
| "/download/linux-rpm", | ||
| ) | ||
| } | ||
| className="text-sm transition-all text-gray-10 hover:underline" | ||
| > | ||
| Fedora/RHEL | ||
| </a> |
There was a problem hiding this comment.
The "Debian/Ubuntu" and "Fedora/RHEL" links have no platform guard, so they are rendered for every visitor — macOS users see three Linux format links alongside the Apple Silicon/Intel alternatives, and Windows users also see all three. The "Linux AppImage" link correctly uses platform !== "linux" to hide itself from Linux users who already have it as the primary download, but the DEB and RPM links lack any guard. Consider wrapping them the same way the AppImage link and other alternatives are gated so each platform sees a relevant short list.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/components/pages/DownloadPage.tsx
Line: 157-180
Comment:
**Linux DEB/RPM links shown unconditionally**
The "Debian/Ubuntu" and "Fedora/RHEL" links have no platform guard, so they are rendered for every visitor — macOS users see three Linux format links alongside the Apple Silicon/Intel alternatives, and Windows users also see all three. The "Linux AppImage" link correctly uses `platform !== "linux"` to hide itself from Linux users who already have it as the primary download, but the DEB and RPM links lack any guard. Consider wrapping them the same way the AppImage link and other alternatives are gated so each platform sees a relevant short list.
How can I resolve this? If you propose a fix, please make it concise.| let mut last_cursor_id: Option<String> = None; | ||
|
|
||
| loop { | ||
| let sleep = tokio::time::sleep(Duration::from_millis(16)); | ||
| let Either::Right(_) = | ||
| futures::future::select(pin!(stop_token_child.cancelled()), pin!(sleep)).await | ||
| else { | ||
| if stop_token_child.is_cancelled() { | ||
| break; | ||
| }; | ||
| } | ||
|
|
||
| std::thread::sleep(Duration::from_millis(16)); |
There was a problem hiding this comment.
The cancellation token is checked at the top of the loop, before std::thread::sleep. Once stop() is called, the thread does not wake early — it finishes the current 16 ms sleep, then checks is_cancelled() on the next iteration. The previous async implementation using futures::future::select would wake immediately on cancellation. For a cursor-recording thread this delay at teardown is harmless, but since CursorActor::stop joins the thread before returning, callers will observe up to a 16 ms block. If prompt teardown becomes important, consider parking_lot::Condvar or an mpsc channel to interrupt the sleep.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/cursor.rs
Line: 259-266
Comment:
**Stop latency up to 16 ms after cancellation**
The cancellation token is checked at the top of the loop, before `std::thread::sleep`. Once `stop()` is called, the thread does not wake early — it finishes the current 16 ms sleep, then checks `is_cancelled()` on the next iteration. The previous async implementation using `futures::future::select` would wake immediately on cancellation. For a cursor-recording thread this delay at teardown is harmless, but since `CursorActor::stop` joins the thread before returning, callers will observe up to a 16 ms block. If prompt teardown becomes important, consider `parking_lot::Condvar` or an `mpsc` channel to interrupt the sleep.
How can I resolve this? If you propose a fix, please make it concise.| for name in cap-cli cap-cli-x86_64-unknown-linux-gnu cap-cli-aarch64-unknown-linux-gnu; do | ||
| candidate="$dir/$name" | ||
| if [ -x "$candidate" ]; then | ||
| CLI_TARGET="$candidate" | ||
| return 0 | ||
| fi | ||
| done | ||
| done | ||
|
|
||
| candidate="$(find "$root" -type f \( -name "cap-cli" -o -name "cap-cli-*linux*" \) -perm -111 2>/dev/null | head -n 1 || true)" | ||
| if [ -n "$candidate" ]; then | ||
| CLI_TARGET="$candidate" | ||
| return 0 | ||
| fi | ||
|
|
||
| return 1 | ||
| } | ||
|
|
||
| prepare_appimage_cli() { | ||
| if [ -z "$APPIMAGE_PATH" ] || [ ! -f "$APPIMAGE_PATH" ]; then | ||
| return 1 | ||
| fi | ||
|
|
||
| if [ -z "$APPDIR_PATH" ]; then | ||
| APPDIR_PATH="$(linux_app_data_dir)/Cap.AppDir" |
There was a problem hiding this comment.
prepare_appimage_cli leaves the EXIT trap active on success
When prepare_appimage_cli succeeds it zeroes out TMP_ROOT and APP_TMP, but it never resets the trap (trap - EXIT HUP INT TERM). The behavior is idempotent — cleanup_desktop_install is a no-op when both variables are empty — but the asymmetry with install_cap_desktop_macos (which always pairs trap … EXIT … with a matching trap -) makes future maintenance harder. Adding trap - EXIT HUP INT TERM at the successful-return point would align with the pattern used everywhere else in the script.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/install-cli.sh/route.ts
Line: 96-120
Comment:
**`prepare_appimage_cli` leaves the EXIT trap active on success**
When `prepare_appimage_cli` succeeds it zeroes out `TMP_ROOT` and `APP_TMP`, but it never resets the trap (`trap - EXIT HUP INT TERM`). The behavior is idempotent — `cleanup_desktop_install` is a no-op when both variables are empty — but the asymmetry with `install_cap_desktop_macos` (which always pairs `trap … EXIT …` with a matching `trap -`) makes future maintenance harder. Adding `trap - EXIT HUP INT TERM` at the successful-return point would align with the pattern used everywhere else in the script.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
superagent-security
Bot
added
the
pr:flagged
|
hey @greptileai please re-review the pr |
| trap cleanup_desktop_install EXIT HUP INT TERM | ||
|
|
||
| echo "Downloading Cap Desktop Debian package..." | ||
| curl -fL "https://cap.so/download/linux-deb" -o "$DEB_PATH" |
There was a problem hiding this comment.
Nice touch hardening the AppImage download curl invocation; worth using the same flags for the DEB download so it’s equally robust.
| curl -fL "https://cap.so/download/linux-deb" -o "$DEB_PATH" | |
| curl --proto '=https' --tlsv1.2 --retry 3 --retry-delay 1 -fL "https://cap.so/download/linux-deb" -o "$DEB_PATH" |
| trap cleanup_desktop_install EXIT HUP INT TERM | ||
|
|
||
| echo "Downloading Cap Desktop RPM package..." | ||
| curl -fL "https://cap.so/download/linux-rpm" -o "$RPM_PATH" |
There was a problem hiding this comment.
Same for RPM download — matching the AppImage curl flags makes this more resilient to transient failures and avoids protocol downgrade surprises.
| curl -fL "https://cap.so/download/linux-rpm" -o "$RPM_PATH" | |
| curl --proto '=https' --tlsv1.2 --retry 3 --retry-delay 1 -fL "https://cap.so/download/linux-rpm" -o "$RPM_PATH" |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
superagent-security
Bot
removed
the
pr:flagged
| return CursorActor { | ||
| stop: None, | ||
| thread: None, | ||
| rx: rx.shared(), | ||
| }; |
There was a problem hiding this comment.
stop_wakeup field — Linux compile error
The CursorActor struct now has four fields (stop, stop_wakeup, thread, rx), but this Wayland early-return initializer only sets stop, thread, and rx — it omits stop_wakeup: None. Rust requires all fields to be listed in a struct literal, so the Linux build will fail with error[E0063]: missing field 'stop_wakeup' in initializer of 'CursorActor'.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/cursor.rs
Line: 228-232
Comment:
**Missing `stop_wakeup` field — Linux compile error**
The `CursorActor` struct now has four fields (`stop`, `stop_wakeup`, `thread`, `rx`), but this Wayland early-return initializer only sets `stop`, `thread`, and `rx` — it omits `stop_wakeup: None`. Rust requires all fields to be listed in a struct literal, so the Linux build will fail with `error[E0063]: missing field 'stop_wakeup' in initializer of 'CursorActor'`.
How can I resolve this? If you propose a fix, please make it concise.
|
hey @greptileai please rereview |
1 participant
Adds Linux recording support, Linux release publishing, and Linux download/install paths for the Cap CLI.
Greptile Summary
This PR adds end-to-end Linux support: a new Rust screen-capture backend (X11/x11grab and Wayland via XDG portal + PipeWire), Linux camera capture, a substantially expanded shell install script with AppImage/DEB/RPM paths, updated web download routing and release metadata for Linux, and a new Linux CI publish job.
linux.rsintroduces X11 capture (FFmpeg x11grab) and a Wayland capture path (XDG Desktop Portal + PipeWire) with runtime fallback from Wayland to X11;cursor.rsrefactors the cursor thread from async tostd::threadwith anmpscwakeup channel and adds X11 XFixes cursor capture.install-cli.shgains Linux-specific AppImage extraction, DEB/RPM installation, arch detection, andXDG_DATA_HOMEawareness; download routes and the versions page are updated forlinux,linux-appimage,linux-deb, andlinux-rpmplatform slugs.publish.ymladds anubuntu-24.04build target with Apple signing steps correctly skipped for Linux runners.Confidence Score: 5/5
Safe to merge — Linux recording backend, install script, and web download routing are well-structured with correct platform guards and no regressions on macOS/Windows paths.
Both findings are cosmetic/quality concerns that do not affect recording correctness. The premultiplied alpha issue only affects cursor themes with anti-aliased edges, and the FrameScaler mismatch causes a single extra allocation on the first x11grab frame.
crates/recording/src/cursor.rs and crates/recording/src/sources/screen_capture/linux.rs are new Linux-only code paths that have not yet been battle-tested in production.
Important Files Changed
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "fix: resolve linux cli follow-ups" | Re-trigger Greptile