feat(linux): AppImage, deb, rpm packaging with XDG-aware paths#20
feat(linux): AppImage, deb, rpm packaging with XDG-aware paths#20rle-mino wants to merge 22 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughStandardizes per-user storage paths (macOS legacy vs Linux XDG), adds Electron/Rust helpers for path resolution, detects Linux install type to disable in-app updates for package-managed installs, refactors CLI discovery into a new module, and implements Linux packaging and release workflow changes with tests/docs. ChangesPlatform-aware storage paths and detection
Linux build and release infrastructure
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/desktop-release.yml:
- Around line 47-49: The build job is unguarded and can run publish-capable
steps even on non-tag workflow_dispatch runs; add a tag-only guard to the build
job (job name "build") by adding a job-level condition such as if:
startsWith(github.ref, 'refs/tags/') so the entire job only runs for tag refs,
and/or add the same conditional to the publish steps that run "electron-builder
... --publish always" to ensure those steps never execute on non-tag refs; make
sure this complements the existing "prepare-release" and "publish-release"
gating so only tag-triggered workflows reach publishing.
- Around line 21-22: Add hardening to the checkout steps and prevent unintended
publishing: add persist-credentials: false to each actions/checkout invocation
(the checkout steps referenced in the workflow), ensure the build job does not
run publisher logic on non-tagged runs by gating the electron-builder publish
behavior (either change the build job's condition to start only on refs/tags or
set electron-builder to --publish never when github.ref is not a tag), and keep
the existing publish-release job gated with if: startsWith(github.ref,
'refs/tags/') so only tagged releases can create/publish GitHub release assets.
In `@packages/claude-agent-sdk-rs/src/discovery.rs`:
- Around line 135-145: The test find_cli_returns_searched_dirs_on_not_found
currently allows Ok(_) and can false-positive; update the match on the result of
find_cli(Some(bogus_override)).await to explicitly fail when Ok(_) is returned
(e.g., panic or assert!(false, ...)) so the test only passes for
Err(SdkError::CliNotFound { searched }) and still asserts !searched.is_empty();
ensure you reference the find_cli call and the SdkError::CliNotFound variant
when making the change so overrides returning Ok are treated as test failures.
In `@packages/desktop/electron/main/sidecar.ts`:
- Around line 136-144: The current ensureLinuxSidecarExecutable function must
not abort startup on chmod failure; change it so that if fs.chmodSync(binary,
0o755) throws, catch the error, then check the file's current mode
(fs.statSync(binary).mode & 0o111) to see if it is already executable and simply
return if so, otherwise log a warning (e.g. console.warn with the binary path
and error message) and continue without throwing; keep the platform check and
references to ensureLinuxSidecarExecutable and binary unchanged.
🪄 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: 4c5659e5-41be-4265-8b61-7576714278c3
⛔ Files ignored due to path filters (8)
packages/desktop/icons/128x128.pngis excluded by!**/*.pngpackages/desktop/icons/16x16.pngis excluded by!**/*.pngpackages/desktop/icons/256x256.pngis excluded by!**/*.pngpackages/desktop/icons/32x32.pngis excluded by!**/*.pngpackages/desktop/icons/48x48.pngis excluded by!**/*.pngpackages/desktop/icons/512x512.pngis excluded by!**/*.pngpackages/desktop/icons/64x64.pngis excluded by!**/*.pngpackages/desktop/icons/icon.pngis excluded by!**/*.png
📒 Files selected for processing (35)
.agents/skills/db/SKILL.md.agents/skills/migration-safety/SKILL.md.claude/skills/migration-safety/SKILL.md.github/workflows/desktop-release.ymlCLAUDE.mdpackages/claude-agent-sdk-rs/src/discovery.rspackages/claude-agent-sdk-rs/src/lib.rspackages/claude-agent-sdk-rs/src/query/metadata.rspackages/claude-agent-sdk-rs/src/query/spawn.rspackages/claude-agent-sdk-rs/src/transport.rspackages/codex-app-server-sdk-rs/src/discovery.rspackages/desktop/build/after-pack.cjspackages/desktop/electron-builder.local.ymlpackages/desktop/electron-builder.ymlpackages/desktop/electron/main/app-paths.test.tspackages/desktop/electron/main/app-paths.tspackages/desktop/electron/main/index.tspackages/desktop/electron/main/linux-install-type.test.tspackages/desktop/electron/main/linux-install-type.tspackages/desktop/electron/main/sidecar.test.tspackages/desktop/electron/main/sidecar.tspackages/desktop/electron/main/updater.tspackages/desktop/electron/preload/index.tspackages/desktop/src/lib/desktop-bridge.tspackages/desktop/src/routes/settings.tsxpackages/desktop/src/stores/update-store.tspackages/opencode-sdk-rs/src/process.rspackages/service/src/domain/git/commands/worktree_ops.rspackages/service/src/domain/workflow/worktree/reuse.rspackages/service/src/shared/app_paths.rspackages/service/src/shared/login_env.rspackages/service/src/shared/mod.rspackages/service/src/shared/worktree_paths.rspackages/service/tests/common/worktree.rspackages/service/tests/feature_worktree_test.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/claude-agent-sdk-rs/src/discovery.rs (1)
134-155: 💤 Low valueConsider renaming this test to reflect its actual contract.
The test name
find_cli_returns_searched_dirs_on_not_foundimplies it always exercises theCliNotFoundpath, but as the new comment explains, it can returnOk(_)when a realclaudebinary exists. A name likefind_cli_reports_searched_dirs_when_cli_not_foundor splitting into two tests (one with a mock that guarantees no binary) would make the intent clearer.The detailed comment is helpful, but the test effectively becomes a no-op on systems with
claudeinstalled.🤖 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 `@packages/claude-agent-sdk-rs/src/discovery.rs` around lines 134 - 155, The test name find_cli_returns_searched_dirs_on_not_found is misleading because the test can succeed with Ok(_) on systems that have a real claude binary; rename the test function to something that reflects the conditional assertion (e.g., find_cli_reports_searched_dirs_when_cli_not_found) or split into two tests: one deterministic unit test that mocks/resolves find_cli to produce Err(SdkError::CliNotFound { searched }) and asserts searched is non-empty, and another integration-style test that allows Ok(_) to pass; update the test function name and/or add a mock-based test so the contract around SdkError::CliNotFound and the searched list is explicit.
🤖 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 `@packages/claude-agent-sdk-rs/src/discovery.rs`:
- Around line 134-155: The test name find_cli_returns_searched_dirs_on_not_found
is misleading because the test can succeed with Ok(_) on systems that have a
real claude binary; rename the test function to something that reflects the
conditional assertion (e.g., find_cli_reports_searched_dirs_when_cli_not_found)
or split into two tests: one deterministic unit test that mocks/resolves
find_cli to produce Err(SdkError::CliNotFound { searched }) and asserts searched
is non-empty, and another integration-style test that allows Ok(_) to pass;
update the test function name and/or add a mock-based test so the contract
around SdkError::CliNotFound and the searched list is explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 406980ab-aac7-4c88-b6a5-9984b240bc66
📒 Files selected for processing (4)
.github/workflows/desktop-release.ymlpackages/claude-agent-sdk-rs/src/discovery.rspackages/desktop/electron/main/sidecar.test.tspackages/desktop/electron/main/sidecar.ts
b471ef5 to
d99a9eb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/desktop-release.yml:
- Around line 171-203: The workflow step named "Smoke-test .deb on debian:12 and
.rpm on fedora:40" validates DEB and RPM but omits the built AppImage; add an
AppImage smoke test by locating the AppImage (e.g., APPIMAGE="$(ls *.AppImage |
head -n1)" under packages/desktop/dist-electron), ensure it is executable, and
run a non-invasive check like APPIMAGE --help or --version inside a container
(similar to the docker run blocks used for DEB/RPM) to verify the
updater-enabled Linux artifact actually runs and retains +x before publishing.
🪄 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: 2db5c2f8-69ac-4771-abb4-13d40614b57d
⛔ Files ignored due to path filters (8)
packages/desktop/icons/128x128.pngis excluded by!**/*.pngpackages/desktop/icons/16x16.pngis excluded by!**/*.pngpackages/desktop/icons/256x256.pngis excluded by!**/*.pngpackages/desktop/icons/32x32.pngis excluded by!**/*.pngpackages/desktop/icons/48x48.pngis excluded by!**/*.pngpackages/desktop/icons/512x512.pngis excluded by!**/*.pngpackages/desktop/icons/64x64.pngis excluded by!**/*.pngpackages/desktop/icons/icon.pngis excluded by!**/*.png
📒 Files selected for processing (23)
.agents/skills/db/SKILL.md.agents/skills/migration-safety/SKILL.md.claude/skills/migration-safety/SKILL.md.github/workflows/desktop-release.ymlCLAUDE.mdpackages/claude-agent-sdk-rs/src/discovery.rspackages/claude-agent-sdk-rs/src/lib.rspackages/claude-agent-sdk-rs/src/query/metadata.rspackages/claude-agent-sdk-rs/src/query/spawn.rspackages/claude-agent-sdk-rs/src/transport.rspackages/codex-app-server-sdk-rs/src/discovery.rspackages/desktop/build/after-pack.cjspackages/desktop/electron-builder.local.ymlpackages/desktop/electron-builder.ymlpackages/desktop/electron/main/app-paths.test.tspackages/desktop/electron/main/app-paths.tspackages/desktop/electron/main/index.tspackages/desktop/electron/main/linux-install-type.test.tspackages/desktop/electron/main/linux-install-type.tspackages/desktop/electron/main/sidecar.test.tspackages/desktop/electron/main/sidecar.tspackages/desktop/electron/main/updater.tspackages/desktop/electron/preload/index.ts
✅ Files skipped from review due to trivial changes (6)
- .agents/skills/migration-safety/SKILL.md
- packages/claude-agent-sdk-rs/src/query/metadata.rs
- .claude/skills/migration-safety/SKILL.md
- packages/desktop/electron-builder.local.yml
- packages/claude-agent-sdk-rs/src/query/spawn.rs
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (12)
- .agents/skills/db/SKILL.md
- packages/desktop/electron-builder.yml
- packages/codex-app-server-sdk-rs/src/discovery.rs
- packages/desktop/electron/main/index.ts
- packages/desktop/build/after-pack.cjs
- packages/desktop/electron/main/sidecar.test.ts
- packages/desktop/electron/main/app-paths.test.ts
- packages/desktop/electron/main/linux-install-type.test.ts
- packages/desktop/electron/main/app-paths.ts
- packages/claude-agent-sdk-rs/src/transport.rs
- packages/desktop/electron/main/updater.ts
- packages/claude-agent-sdk-rs/src/discovery.rs
ba59636 to
534be4b
Compare
Build and publish AppImage artifacts from the desktop release workflow so Linux users can launch Cadencr from a single packaged binary. The release workflow now prepares a draft once, uploads macOS and Linux artifacts through a matrix build, and publishes only after both build legs complete.
Hydrate Linux GUI launch environments from the user's login shell so local agent CLIs remain discoverable outside terminal launches. Add common Linux install locations to provider discovery specs while preserving provider-neutral boundaries.
Phase 2 of Linux support. On Linux, Cadencr now stores data under $XDG_DATA_HOME/cadencr (default ~/.local/share/cadencr), config under $XDG_CONFIG_HOME/cadencr, and cache under $XDG_CACHE_HOME/cadencr — the locations packagers, distros, and sandbox tooling expect. macOS keeps the legacy ~/.cadencr/... layout bit-for-bit, so existing installs and the db/migration skills keep working with no migration. Introduces a single seam in each runtime so future callers don't reach for dirs::home_dir() directly: - packages/service/src/shared/app_paths.rs (Rust) - packages/desktop/electron/main/app-paths.ts (Electron main) Both helpers route the production DB path, the worktrees root, and the logs directory through one macOS-or-XDG resolver. The existing worktree_paths and sidecar callsites now delegate to them; the integration tests' HomeGuard also clears the XDG vars so Linux runs get a predictable tempdir layout.
…type Phase 3 of Linux compatibility. CI now produces .AppImage, .deb, and .rpm artifacts per release; the in-app auto-updater stays active only on AppImage installs and surfaces a "managed by your package manager" hint on deb/rpm/unknown installs instead of silently no-op'ing. - electron-builder.yml / .local.yml: add deb + rpm targets with pinned depends lists (default electron-builder deps can drift between releases; pinning keeps Ubuntu/Fedora installability predictable). - desktop-release.yml: install `rpm` on ubuntu-latest so rpmbuild is available; rename the Linux build step. - after-pack.cjs: comment clarifying the sidecar chmod propagates to deb/rpm because afterPack runs once before any target packager. - linux-install-type.ts (+ test): detect install type from $APPIMAGE and /etc/os-release ID/ID_LIKE; returns the apt/dnf upgrade command. - updater.ts: new `update:unsupported` IPC channel; check / install handlers and the startup announce skip electron-updater entirely on deb/rpm/unknown installs. Tracks the announce timeout so shutdownAutoUpdater can cancel it on fast quit. - preload + desktop-bridge + update-store: thread the new event through, add `"unsupported"` status with the package-manager message. - settings.tsx: About button reads "Managed by package manager", disabled, with the distro-specific upgrade command as tooltip + a11y status line — never silently no-ops, per error-handling.md.
- H1: namespace macOS config/cache under ~/.cadencr/{config,cache} so
future callers can't collide on a shared filename. Data stays at the
legacy root — moving it would orphan existing installs.
- H2: surface productionDbPath mkdir failures with the resolved path,
rather than leaking a raw EACCES from Node.
- H3: smoke-install the produced .deb (debian:12) and .rpm (fedora:40)
in CI and run `cadencr-service --help` to validate both the depends
lists and that the sidecar keeps its +x bit through dpkg/rpm.
- M1: move .cargo/bin to the end of the well_known relative-to-home
lists in claude-agent-sdk-rs and codex-app-server-sdk-rs to match
the original ordering convention.
- M3: switch the Rust Linux app_paths tests to tempfile::tempdir()
instead of hardcoded /tmp paths to remove cross-binary races.
- sidecar chmod (critical): skip the chmod when the binary is already executable, and re-check the X bit on EPERM/EROFS/EACCES before throwing. dpkg/rpm install the sidecar into a root-owned, read-only /opt/Cadencr/resources/, so the previous unconditional chmod would hard-fail startup for every .deb/.rpm user even though the bit was already set. AppImage and dev paths still get the chmod they need. - Workflow hardening: add persist-credentials: false to all three actions/checkout steps so the GITHUB_TOKEN doesn't linger in .git/config — we only ever use gh with an explicit GH_TOKEN env. - Gate the build job to refs/tags/ so a branch workflow_dispatch can't fire electron-builder --publish always (which uploads to the configured GitHub draft release). - Document why find_cli_returns_searched_dirs_on_not_found can't assert panic-on-Ok — find_cli intentionally falls back from a bogus override to PATH/login-shell/well-known discovery, so any host with a resolvable `claude` binary hits the Ok arm legitimately.
Build local Linux packages from the production Electron Builder config so local testing uses the same package metadata and artifact names as release builds. Install the generated Debian package after successful local builds to make repeated Ubuntu testing one command.
Capture the Linux package strategy, AppImage caveats, and installer checklist so the future one-line installer can build on the compatibility work already validated locally.
Bind the Cadencr service listener before expensive startup work so dev startup stops immediately when the configured port is already in use.
Replace the native Linux window frame with app-owned controls so the header can reserve space and avoid overlap with feature actions.
Use Ctrl+Shift+C/V for terminal copy and paste on Linux without stealing Ctrl+C from the PTY. Add native clipboard IPC so terminal paste works reliably from the Electron renderer.
Replace macOS-only Cmd glyphs in shortcut labels with platform-aware formatting and tune Ctrl/Shift key badge alignment for Linux.
6286123 to
0b94b68
Compare
Confidence Score: 5/5Safe to merge. The XDG path helpers are mirrored and tested in both Rust and TypeScript, macOS paths are unchanged, and all three previously flagged concerns have been addressed in this revision. The XDG path layer, install-type detector, and CI restructuring are all well-tested and internally consistent. The remaining observations are minor: the upload-count guard threshold is one below the actual artifact count, and one Linux distro-detection token is misleading in isolation but harmless in practice. .github/workflows/desktop-release.yml — the upload-count guard (< 4) and unguarded macOS sidecar-check steps are worth a second look before the first Linux release tag fires. Important Files Changed
Reviews (3): Last reviewed commit: "Use zypper for openSUSE update guidance" | Re-trigger Greptile |
|
Too many files changed for review. ( Bypass the limit by tagging |
Use Electron's X11 compatibility backend by default on Linux to prevent Wayland WebContentsView scroll drift while preserving explicit overrides.
…linux-compatibility-plan-491e
Handle continuous debounced writes so live Git data stays fresh, while coalescing queued recomputes to keep sustained churn bounded.
Summary
Ships first-class Linux support alongside macOS:
~/.cadencr/layout.electron-updaterand select the artifact matching the packaged install identity. Custom/unrecognized Linux builds receive a clear unsupported-install message.resources/package-type, and requireslatest-linux.ymlto advertise all three updater targets.CLAUDE.md, generatedAGENTS.md, and a shared rule so macOS and Linux are both evaluated and any real-platform verification still required is explicitly disclosed.Validation
pnpm testsuite passesresources/package-type=deb