build(release): remove AppImage distribution#503
Merged
Conversation
yandy-r
added
area:build
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughTransition the repository from AppImage-centric packaging to Flatpak-first: add a release-binary build script, delegate/remove native/AppImage build artifacts and container flows, update Flatpak build and CI release jobs, disable Tauri bundling, remove AppImage startup paths, and update docs and helper scripts to Flatpak semantics. ChangesFlatpak-only distribution transition
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
platform:linux
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/internal-docs/profile-collections-browser-mocks.md (1)
41-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync this CI snippet with the current workflow check.
The snippet around Line 46 documents an older sentinel list and omits the JS-asset existence guard now present in
.github/workflows/release.yml. Please update it so docs match the actual release gate.Suggested doc update
- name: Verify no mock code in production bundle run: | - if grep -rl '\[dev-mock\]\|getMockRegistry\|registerMocks\|MOCK MODE' \ + if ! compgen -G "src/crosshook-native/dist/assets/*.js" > /dev/null; then + echo "::error::No JS assets found under src/crosshook-native/dist/assets/ — build output missing or glob did not match" >&2 + exit 1 + fi + if grep -rl '\[dev-mock\]\|getMockRegistry\|registerMocks\|MOCK MODE\|__MOCK_VALIDATION_ERROR__\|__MOCK_VALIDATION_WARNING__' \ src/crosshook-native/dist/assets/*.js 2>/dev/null; then echo "::error::Mock code found in production bundle — refusing to ship" exit 1 fi🤖 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 `@docs/internal-docs/profile-collections-browser-mocks.md` around lines 41 - 51, Update the docs' verify:no-mocks snippet so it exactly matches the workflow: include the JS-asset existence guard around the grep and use the same sentinel pattern the workflow currently checks for (i.e., the grep pattern used in the verify:no-mocks job), and keep the same asset glob (src/crosshook-native/dist/assets/*.js) and error behaviour; specifically change the code block to first test for JS assets' existence (the guard used in verify:no-mocks) and then run the grep with the exact sentinel regex currently used by the workflow (replacing the older '\[dev-mock\]\|getMockRegistry\|registerMocks\|MOCK MODE' list) so the docs and the verify:no-mocks job are identical.src/crosshook-native/src/lib/mocks/README.md (1)
116-117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the
/home/...examples from the mock README.The fixture-lint job already rejects developer-home-path strings in mock fixture docs, so this file will keep failing CI until those examples are rewritten to synthetic prefixes like
/mock/.... This matches the CI failure reported for mock fixture files.Suggested fix
-- File paths: use `/home/devuser/…` or `/mock/…` prefixes. No real system -- paths. +- File paths: use `/mock/…` prefixes or other synthetic placeholders. No real + system paths.🤖 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 `@src/crosshook-native/src/lib/mocks/README.md` around lines 116 - 117, Remove all `/home/...` developer-home-path examples from the mocks README and replace them with synthetic prefixes (e.g. `/mock/...`); specifically update the line that currently reads "**File paths**: use `/home/devuser/…` or `/mock/…` prefixes." to eliminate `/home/devuser` examples (use `/mock/...` consistently), ensure no strings matching developer home paths remain, and re-run fixture-lint to confirm the mock README no longer triggers the CI rejection.Source: Pipeline failures
CONTRIBUTING.md (1)
116-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPoint the style section at the repo’s actual formatter/linter config.
The section now says
cargo fmtandcargo clippyuse standard defaults, but this repo usessrc/crosshook-native/rustfmt.tomland workspace lints insrc/crosshook-native/Cargo.toml. Please update the wording so contributors don’t run the wrong config surface.Suggested wording
- - **Formatting**: `cargo fmt` (standard defaults -- no `rustfmt.toml`) - - **Linting**: `cargo clippy` (standard defaults -- no `clippy.toml`) + - **Formatting**: `cargo fmt` (uses `src/crosshook-native/rustfmt.toml`) + - **Linting**: `cargo clippy` (workspace lints in `src/crosshook-native/Cargo.toml`)As per coding guidelines, Rust formatting and linting are driven by the repo-specific configs, not the defaults.
🤖 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 `@CONTRIBUTING.md` around lines 116 - 117, Update the Formatting/Linting lines that currently state "cargo fmt (standard defaults -- no rustfmt.toml)" and "cargo clippy (standard defaults -- no clippy.toml)" in CONTRIBUTING.md to point contributors at the repo-specific configs: mention that Rust formatting uses src/crosshook-native/rustfmt.toml and linting is driven by the workspace settings in src/crosshook-native/Cargo.toml (and instruct running cargo fmt / cargo clippy which will pick up those configs), replacing the current "standard defaults" wording so contributors use the repo configs.Source: Coding guidelines
docs/getting-started/quickstart.md (1)
131-131:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the Flatpak sandbox path separate from the legacy host import path.
Both docs still present
~/.config/crosshook/as the normal storage location, but that path is only the legacy/AppImage import source. Fresh Flatpak installs store live config in the sandboxed app directory, so the quickstart and release notes should name the legacy path explicitly or switch to the Flatpak path.🤖 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 `@docs/getting-started/quickstart.md` at line 131, Update the quickstart and release notes to distinguish the legacy AppImage/config import path from the Flatpak sandbox path: leave the legacy path as `~/.config/crosshook/...` but explicitly mark it as "legacy/AppImage import source", and add the Flatpak live-config examples using the sandboxed paths (e.g. `~/.var/app/<flatpak-id>/config/crosshook/profiles/` and `~/.var/app/<flatpak-id>/.local/share/crosshook/prefixes/<slug>/`), and adjust the prose around the Install Game flow (the paragraph that currently references `~/.config/crosshook/profiles/` and `~/.local/share/crosshook/prefixes/<slug>`) so it shows both paths and which installer type uses each.Source: Linked repositories
docs/architecture/adr-0004-flatpak-per-app-isolation.md (1)
104-111:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReword the reset-path caveat.
A
--delete-datareset does not duplicate tap state on its own; it re-imports the current host tree. The real risk is that sandbox-only changes are lost and host-side edits become the new source of truth after the reset.🤖 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 `@docs/architecture/adr-0004-flatpak-per-app-isolation.md` around lines 104 - 111, Update the "Sandbox reset triggers re-import" caveat so it accurately explains that `flatpak uninstall --delete-data` simply re-imports the current host tree and does not by itself duplicate tap state; specifically replace the sentence starting with "A `flatpak uninstall --delete-data` removes the sandbox tree; the next Flatpak launch re-imports from the host (idempotent, but the operation runs again and may duplicate community tap state if the host tree changed in the interim)." with wording that emphasizes the real risk: sandbox-only changes are lost and host-side edits become the new source of truth after the reset, and clarify that duplication of tap state only occurs if the host tree itself changed in the interim, not as a direct consequence of the reset operation.
🤖 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 `@docs/architecture/adr-0001-platform-host-gateway.md`:
- Around line 9-13: The ADR's example list conflates game-process wrappers with
general host utilities; update the paragraph describing CrossHook so tools that
must "interact with a game process" are listed separately (e.g. gamescope,
MangoHud, winetricks, umu-run) and move general host utilities (e.g. git,
unshare) into a distinct clause or example list to keep the contract precise and
scoped to process-interaction vs general host tooling. Ensure the sentence
describing CrossHook's role (mentions of CrossHook, Proton/Wine, and "host
process") is preserved and only the example grouping is split for clarity.
In `@README.md`:
- Around line 174-182: Update the README's Flatpak build example to ensure the
release binary is refreshed; change the command or instruct to run the release
build first so build-flatpak.sh won't reuse a cached binary. Specifically either
call ./scripts/build-release-binary.sh before running ./scripts/build-flatpak.sh
--strict, or update the example to ./scripts/build-flatpak.sh --rebuild --strict
so build-flatpak.sh will regenerate the release binary rather than packaging a
potentially stale one.
In `@scripts/build-flatpak.sh`:
- Around line 212-226: The cached release binary at BINARY_PATH
("$DIST_DIR/crosshook-native") is not keyed by TARGET_TRIPLE, so
build-flatpak.sh may reuse a wrong-architecture binary; update build-flatpak.sh
to verify the cached binary matches the current TARGET_TRIPLE by reading a
marker file (e.g. "$DIST_DIR/crosshook-native.target-triple") and only reuse the
cache when the marker content equals $TARGET_TRIPLE (otherwise treat as missing
and rebuild), and update scripts/build-release-binary.sh to write that marker
immediately after copying the built binary (write "$TARGET_TRIPLE" into
"$DIST_DIR/crosshook-native.target-triple") so the check can succeed on
subsequent runs.
---
Outside diff comments:
In `@CONTRIBUTING.md`:
- Around line 116-117: Update the Formatting/Linting lines that currently state
"cargo fmt (standard defaults -- no rustfmt.toml)" and "cargo clippy (standard
defaults -- no clippy.toml)" in CONTRIBUTING.md to point contributors at the
repo-specific configs: mention that Rust formatting uses
src/crosshook-native/rustfmt.toml and linting is driven by the workspace
settings in src/crosshook-native/Cargo.toml (and instruct running cargo fmt /
cargo clippy which will pick up those configs), replacing the current "standard
defaults" wording so contributors use the repo configs.
In `@docs/architecture/adr-0004-flatpak-per-app-isolation.md`:
- Around line 104-111: Update the "Sandbox reset triggers re-import" caveat so
it accurately explains that `flatpak uninstall --delete-data` simply re-imports
the current host tree and does not by itself duplicate tap state; specifically
replace the sentence starting with "A `flatpak uninstall --delete-data` removes
the sandbox tree; the next Flatpak launch re-imports from the host (idempotent,
but the operation runs again and may duplicate community tap state if the host
tree changed in the interim)." with wording that emphasizes the real risk:
sandbox-only changes are lost and host-side edits become the new source of truth
after the reset, and clarify that duplication of tap state only occurs if the
host tree itself changed in the interim, not as a direct consequence of the
reset operation.
In `@docs/getting-started/quickstart.md`:
- Line 131: Update the quickstart and release notes to distinguish the legacy
AppImage/config import path from the Flatpak sandbox path: leave the legacy path
as `~/.config/crosshook/...` but explicitly mark it as "legacy/AppImage import
source", and add the Flatpak live-config examples using the sandboxed paths
(e.g. `~/.var/app/<flatpak-id>/config/crosshook/profiles/` and
`~/.var/app/<flatpak-id>/.local/share/crosshook/prefixes/<slug>/`), and adjust
the prose around the Install Game flow (the paragraph that currently references
`~/.config/crosshook/profiles/` and `~/.local/share/crosshook/prefixes/<slug>`)
so it shows both paths and which installer type uses each.
In `@docs/internal-docs/profile-collections-browser-mocks.md`:
- Around line 41-51: Update the docs' verify:no-mocks snippet so it exactly
matches the workflow: include the JS-asset existence guard around the grep and
use the same sentinel pattern the workflow currently checks for (i.e., the grep
pattern used in the verify:no-mocks job), and keep the same asset glob
(src/crosshook-native/dist/assets/*.js) and error behaviour; specifically change
the code block to first test for JS assets' existence (the guard used in
verify:no-mocks) and then run the grep with the exact sentinel regex currently
used by the workflow (replacing the older
'\[dev-mock\]\|getMockRegistry\|registerMocks\|MOCK MODE' list) so the docs and
the verify:no-mocks job are identical.
In `@src/crosshook-native/src/lib/mocks/README.md`:
- Around line 116-117: Remove all `/home/...` developer-home-path examples from
the mocks README and replace them with synthetic prefixes (e.g. `/mock/...`);
specifically update the line that currently reads "**File paths**: use
`/home/devuser/…` or `/mock/…` prefixes." to eliminate `/home/devuser` examples
(use `/mock/...` consistently), ensure no strings matching developer home paths
remain, and re-run fixture-lint to confirm the mock README no longer triggers
the CI rejection.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68f58363-18c2-4236-8a03-4d54cd9c90b0
📒 Files selected for processing (41)
.ai/rules/project.md.cursor/rules/project.mdc.cursorrules.github/pull_request_template.md.github/workflows/release.ymlAGENTS.mdCLAUDE.mdCONTRIBUTING.mdREADME.mddocs/architecture/adr-0001-platform-host-gateway.mddocs/architecture/adr-0002-flatpak-portal-contracts.mddocs/architecture/adr-0003-proton-download-manager.mddocs/architecture/adr-0004-flatpak-per-app-isolation.mddocs/getting-started/quickstart.mddocs/internal-docs/local-build-publish.mddocs/internal-docs/profile-collections-browser-mocks.mddocs/internal-docs/steam-deck-validation-checklist.mddocs/prps/prds/flatpak-distribution.prd.mdpackage.jsonpackaging/PKGBUILDpackaging/flatpak/README.mdscripts/build-flatpak.shscripts/build-native-container.Dockerfilescripts/build-native-container.shscripts/build-native.shscripts/build-release-binary.shscripts/generate-crosshook-desktop.shscripts/install-native-build-deps.shscripts/lib/build-paths.shscripts/lib/sync-tauri-icons.shscripts/lint.shsrc/crosshook-native/crates/crosshook-cli/src/args.rssrc/crosshook-native/crates/crosshook-core/Cargo.tomlsrc/crosshook-native/crates/crosshook-core/src/flatpak_migration/mod.rssrc/crosshook-native/crates/crosshook-core/src/platform/detect.rssrc/crosshook-native/crates/crosshook-core/src/platform/gateway.rssrc/crosshook-native/crates/crosshook-core/src/platform/mod.rssrc/crosshook-native/crates/crosshook-core/src/platform/xdg.rssrc/crosshook-native/src-tauri/src/lib.rssrc/crosshook-native/src-tauri/tauri.conf.jsonsrc/crosshook-native/src/lib/mocks/README.md
💤 Files with no reviewable changes (5)
- scripts/generate-crosshook-desktop.sh
- scripts/lib/sync-tauri-icons.sh
- scripts/build-native-container.Dockerfile
- scripts/build-native-container.sh
- packaging/PKGBUILD
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
1 participant
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.
Summary
Changes
Testing