Skip to content

feat(desktop, analytics): add packaging and usage export#172

Open
awsl233777 wants to merge 27 commits into
mainfrom
pr-168
Open

feat(desktop, analytics): add packaging and usage export#172
awsl233777 wants to merge 27 commits into
mainfrom
pr-168

Conversation

@awsl233777
Copy link
Copy Markdown
Collaborator

@awsl233777 awsl233777 commented May 25, 2026

Summary

  • add a Tauri desktop shell for Codex Mate and macOS/Windows packaging
  • publish desktop installer assets directly from the release workflow (.dmg, .msi, .exe) alongside existing npm/standalone archives
  • add analytics usage export for BI/archive workflows via codexmate analytics export and the export-sessions-usage API action
  • support CSV/JSON output, date range filtering, model filtering, and empty-data exports without crashes

Validation

  • npm run test:unitAll 535 tests passed.
  • node --check cli.js
  • node cli.js analytics export --format csv --from 2100-01-01 --to 2100-01-02 --output -
  • node cli.js analytics export --format json --from 2100-01-01 --to 2100-01-02 --output -
  • git diff --check

Notes

  • Local Linux desktop packaging was not run because this machine is missing Rust/Cargo and Linux Tauri system prerequisites (webkit2gtk-4.1, rsvg2). Windows/macOS bundle validation runs through GitHub Actions.
  • The first desktop implementation still expects a system Node runtime. A later hardening pass can bundle a Node sidecar/runtime for fully self-contained desktop installers.
  • The release workflow is workflow_dispatch, so the PR validates its release asset handoff by workflow contract tests and review; a real Release asset upload requires running the release workflow for a tag.

Closes #169
Closes #139

Summary by CodeRabbit

  • New Features

    • macOS and Windows desktop apps with CI-built installers (Windows installer verifies UAC manifest); bundled Node runtime so users need no system Node
    • New CLI/Web API analytics export (CSV/JSON) for session usage
    • Desktop startup diagnostics with optional console output and configurable persistent logs
  • Documentation

    • Desktop staging, build, packaging, debugging, and Windows startup/log guidance added
  • Chores

    • Added desktop npm scripts and CI workflows; raised Node engine to >=16.14.0
  • Tests

    • Added unit tests for desktop staging, analytics export/args, icons, packaging, and release workflow

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds Tauri desktop packaging and staging tooling, Rust Tauri lifecycle and diagnostics to spawn the Node CLI with TCP health-checks, CI desktop build/release integration (macOS/Windows) with Windows manifest verification, NSIS installer hook, staging/asset tests and docs, plus analytics export CLI/API and unit tests.

Changes

Desktop Application with Tauri

Layer / File(s) Summary
Tauri config, manifests, icons, and package updates
src-tauri/.gitignore, src-tauri/Cargo.toml, src-tauri/build.rs, src-tauri/app.manifest, src-tauri/capabilities/default.json, src-tauri/icons/*, src-tauri/tauri.conf.json, package.json
Adds Cargo version bump and build.rs embedding app.manifest, capabilities and icon resources, .gitignore updates, Tauri config with product/window/bundle/resource mapping and CSP, and npm script/engine/devDependency updates for desktop commands.
Staging script and tauri/Cargo updates
tools/desktop/prepare-tauri-resources.js, src-tauri/tauri.conf.json, src-tauri/Cargo.toml
Node staging script builds dist/desktop/codexmate, copies production node_modules derived from package-lock.json, writes codexmate-desktop.json, validates layout, injects dev/build URLs and commands into Tauri config, and optionally updates Cargo package version.
Rust backend lifecycle and entrypoint
src-tauri/src/lib.rs, src-tauri/src/main.rs
Adds diagnostics initialization, file/console logging and panic hook, stale-port cleanup, Node backend spawn/configuration, raw TCP health-check to 127.0.0.1:3737, readiness timeout handling, and shutdown on Tauri window destroy.
CI & Release integration
.github/workflows/desktop-build.yml, .github/workflows/release.yml
Adds desktop-build workflow to stage and build Tauri bundles on macOS/Windows (including Windows manifest extraction/verification) and updates release.yml to include a desktop matrix job, artifact download, and include desktop installers in release assets.
Tests, docs, and installer hooks
tests/unit/*, tests/unit/run.mjs, doc/desktop.md, src-tauri/windows/installer-hooks.nsh
Adds unit tests for staging layout, icon binaries, diagnostics contract, release workflow contract, npm package files, and test-runner imports; documents desktop architecture, staging layout, npm scripts, prerequisites, Windows diagnostics, and CI steps/artifact naming; adds NSIS preinstall hook to taskkill existing exe.

Analytics export feature

Layer / File(s) Summary
Analytics CLI argument parser
cli/analytics-export-args.js, tests/unit/analytics-export-args.test.mjs
Adds parseAnalyticsExportArgs(args=[]) with flag parsing, choice validation, CommonJS export, and unit tests for valid/invalid inputs.
Session-usage export core
cli/session-usage.js, tests/unit/session-usage-backend.test.mjs
Adds date/model normalization, aggregation into per-day/per-model token/session rows, CSV/JSON serialization, and exportSessionUsageCore producing payloads; tests validate CSV/JSON outputs and empty inputs.
CLI + API wiring
cli.js, tests/unit/run.mjs
Wires new analytics export CLI command and help, file/stdout output handling, export wrapper for /api action export-sessions-usage, and registers new tests in the test runner.

Sequence Diagram

sequenceDiagram
  participant AppMain as src-tauri/src/main.rs
  participant AppLib as src-tauri/src/lib.rs
  participant NodeBackend as dist/desktop/codexmate/cli.js
  participant HealthCheck as 127.0.0.1:3737
  participant CI as GitHub Actions

  AppMain->>AppLib: init_desktop_diagnostics() and run()
  AppLib->>NodeBackend: spawn_backend (node cli.js run --host 127.0.0.1 --no-browser)
  NodeBackend->>HealthCheck: serve /api
  AppLib->>HealthCheck: health_check_ready polling until 200 + {"ok":true}
  CI->>AppLib: build (tauri/cargo) -> bundle outputs and verify Windows manifest
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • ymkiux
  • SurviveM

Poem

🐰 I hopped to stage the desktop bright,
Tauri cradles web and Rust at night,
Node hums the pages, health-checks shine light,
CI packs installers for Windows and macOS flight,
Little rabbit cheers — builds landed right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding Tauri desktop packaging for macOS/Windows and analytics usage export functionality.
Linked Issues check ✅ Passed All requirements from #169 (macOS/Windows native packaging) and #139 (CSV/JSON analytics export with filtering) are implemented with CI workflows, desktop builds, analytics export CLI/API, and comprehensive tests.
Out of Scope Changes check ✅ Passed All changes align with stated objectives: desktop packaging infrastructure, analytics export implementation, release workflow updates, CI validations, and supporting documentation—no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pr-168

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@awsl233777 awsl233777 changed the title feat(desktop): add Tauri shell feat(desktop): add macOS and Windows app packaging May 25, 2026
@awsl233777
Copy link
Copy Markdown
Collaborator Author

Updated this PR after rebasing onto latest main and replacing the direct Tauri resource mapping with a staged desktop runtime layout.

Validation run locally:

  • npm run desktop:stage
  • node tests/unit/run.mjs ✅ All 532 tests passed
  • npm run lint ✅ 303 files passed
  • npm pack --dry-run --json ✅ verified no src-tauri/, tools/desktop/, or dist/ in the npm CLI package
  • npm run test:e2e
  • npm run desktop:build attempted locally, but this host has no Rust/Cargo (cargo metadata ... No such file or directory), so native bundle validation is left to the GitHub Actions macOS/Windows desktop-build jobs.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

@awsl233777 Thanks for the detailed validation summary! Triggering a full review of the updated PR now. 🐇

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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-build.yml:
- Around line 34-35: Replace floating action refs with pinned commit SHAs for
every `uses:` entry (e.g., change `actions/checkout@v4` to the corresponding
full commit SHA ref) and for the checkout step(s) add `persist-credentials:
false` under the step config to disable token persistence; update all
occurrences noted (the Checkout step and the other actions referenced) so each
`uses:` is a full commit SHA and each checkout step includes
`persist-credentials: false`.

In `@src-tauri/src/lib.rs`:
- Around line 116-124: The spawned child process from command.spawn() is not
terminated if wait_for_backend(Duration::from_secs(15)) returns false; modify
the error path so that when wait_for_backend(...) is false you call child.kill()
(and optionally child.wait() to reap it), handle/ignore any kill/wait errors,
and then return the Err string instead of leaving the process running; update
the block where you create let child = command.spawn() and the subsequent
readiness check to ensure child is killed and awaited before returning Err.
- Around line 39-44: The readiness probe currently only checks for "HTTP/1.1
200" or "HTTP/1.0 200" in the raw response (using response and
stream.read_to_string), which can false-positive for other services; update the
check so after reading the response it also validates a backend-specific
identity token (for example a unique header like "X-Backend: MyAppBackend" or a
distinct body string such as "MyAppBackend ready") in the response before
returning true; modify the conditional that currently returns
response.starts_with(...) to require both the 200 status line and the presence
of that unique identifier, and keep the existing error handling for
stream.read_to_string failures.

In `@tests/unit/desktop-stage.test.mjs`:
- Around line 54-56: The test currently uses a brittle full-object equality on
tauriConfig.bundle.resources via assert.deepStrictEqual; instead assert only
that the required mapping exists by checking that tauriConfig.bundle.resources
has the key '../dist/desktop/codexmate' and its value equals 'codexmate' (e.g.,
use hasOwnProperty or in-check plus assert.strictEqual on
tauriConfig.bundle.resources['../dist/desktop/codexmate']). Update the assertion
in tests/unit/desktop-stage.test.mjs to verify the single mapping rather than
the entire resources object.

In `@tools/desktop/prepare-tauri-resources.js`:
- Around line 48-53: The use of fs.cpSync in prepare-tauri-resources.js is
incompatible with engines.node >=14; either update package.json.engines to a
Node version that includes fs.cpSync or replace the fs.cpSync call with a
Node-14-compatible recursive copy routine: implement a small utility in
prepare-tauri-resources.js (e.g., copyDirRecursive) that creates destination
dirs with fs.mkdirSync(..., { recursive: true }), iterates entries with
fs.readdirSync, skips any path containing '.git' (match the existing filter
logic), copies files with fs.copyFileSync and handles directories recursively,
and preserves the intended dereference/force semantics as best-effort; modify
the code to call this utility instead of fs.cpSync (or bump engines.node if you
prefer the simpler change).
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 95d40a9b-5e65-40b6-bcd5-49df9fc4fc2a

📥 Commits

Reviewing files that changed from the base of the PR and between 0e38488 and d69d704.

⛔ Files ignored due to path filters (16)
  • package-lock.json is excluded by !**/package-lock.json
  • src-tauri/icons/128x128.png is excluded by !**/*.png
  • src-tauri/icons/128x128@2x.png is excluded by !**/*.png
  • src-tauri/icons/32x32.png is excluded by !**/*.png
  • src-tauri/icons/Square107x107Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square142x142Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square150x150Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square284x284Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square30x30Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square310x310Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square44x44Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square71x71Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square89x89Logo.png is excluded by !**/*.png
  • src-tauri/icons/StoreLogo.png is excluded by !**/*.png
  • src-tauri/icons/icon.ico is excluded by !**/*.ico
  • src-tauri/icons/icon.png is excluded by !**/*.png
📒 Files selected for processing (15)
  • .github/workflows/desktop-build.yml
  • doc/desktop.md
  • package.json
  • src-tauri/.gitignore
  • src-tauri/Cargo.toml
  • src-tauri/build.rs
  • src-tauri/capabilities/default.json
  • src-tauri/icons/icon.icns
  • src-tauri/src/lib.rs
  • src-tauri/src/main.rs
  • src-tauri/tauri.conf.json
  • tests/unit/desktop-stage.test.mjs
  • tests/unit/npm-package-files.test.mjs
  • tests/unit/run.mjs
  • tools/desktop/prepare-tauri-resources.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Windows
  • GitHub Check: macOS
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/desktop-build.yml

[warning] 34-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 44-44: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 59-59: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🔇 Additional comments (10)
doc/desktop.md (1)

1-62: LGTM!

src-tauri/.gitignore (1)

1-4: LGTM!

src-tauri/Cargo.toml (1)

1-25: LGTM!

src-tauri/build.rs (1)

1-3: LGTM!

src-tauri/capabilities/default.json (1)

1-11: LGTM!

src-tauri/tauri.conf.json (1)

1-47: LGTM!

src-tauri/src/main.rs (1)

1-6: LGTM!

package.json (1)

45-49: LGTM!

Also applies to: 79-80

tests/unit/npm-package-files.test.mjs (1)

20-25: LGTM!

tests/unit/run.mjs (1)

57-57: LGTM!

Comment thread .github/workflows/desktop-build.yml Outdated
Comment thread src-tauri/src/lib.rs
Comment thread src-tauri/src/lib.rs Outdated
Comment thread tests/unit/desktop-stage.test.mjs Outdated
Comment thread tools/desktop/prepare-tauri-resources.js
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
tests/unit/desktop-stage.test.mjs (1)

54-56: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid brittle full-object equality for bundle.resources.

Assert the required mapping instead of the whole object so valid extra resources don’t break this test.

Proposed fix
-    assert.deepStrictEqual(tauriConfig.bundle.resources, {
-        '../dist/desktop/codexmate': 'codexmate'
-    });
+    assert.strictEqual(
+        tauriConfig.bundle?.resources?.['../dist/desktop/codexmate'],
+        'codexmate',
+        'tauri bundle.resources must map ../dist/desktop/codexmate to codexmate'
+    );
🤖 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 `@tests/unit/desktop-stage.test.mjs` around lines 54 - 56, The test currently
asserts full-object equality on tauriConfig.bundle.resources which is brittle;
update the assertion in tests/unit/desktop-stage.test.mjs to check only that the
required mapping exists (e.g., assert that
tauriConfig.bundle.resources['../dist/desktop/codexmate'] equals 'codexmate' or
that Object.prototype.hasOwnProperty.call(tauriConfig.bundle.resources,
'../dist/desktop/codexmate') and its value is 'codexmate') so extra resources
won’t break the test while still verifying the necessary mapping.
tools/desktop/prepare-tauri-resources.js (1)

48-53: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align declared Node support with fs.cpSync usage.

fs.cpSync here breaks on Node 14, but the project still declares engines.node >=14. Please either raise the engine floor (e.g. Node >=16.7) or replace this copy path with a Node-14-compatible recursive implementation.

#!/bin/bash
set -euo pipefail

echo "Declared Node engine in package.json:"
rg -n '"engines"\s*:\s*\{|"node"\s*:' package.json -A2 -B1

echo
echo "Node API usage in desktop staging script:"
rg -n '\bfs\.cpSync\b' tools/desktop/prepare-tauri-resources.js -C2
🤖 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 `@tools/desktop/prepare-tauri-resources.js` around lines 48 - 53, The code uses
fs.cpSync in prepare-tauri-resources.js which requires a newer Node than the
project's declared engines.node (>=14); either update package.json engines.node
to a minimum Node version that supports fs.cpSync (e.g., ">=16.7.0") or replace
the fs.cpSync call with a Node-14-compatible recursive copier: implement a
copyDirSync function (using fs.readdirSync, fs.statSync/lstatSync to honor
dereference, fs.mkdirSync, fs.copyFileSync and handling overwrite/force) and
apply the same filter logic (skip paths containing '.git') before calling it in
place of fs.cpSync; ensure the replacement preserves recursive, force/overwrite,
dereference, and filter behavior used in the original fs.cpSync invocation.
.github/workflows/desktop-build.yml (1)

34-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin all workflow actions to commit SHAs and harden checkout credentials.

This workflow still uses floating action refs and keeps default checkout credential persistence. Pin every uses: to full commit SHAs and set persist-credentials: false on checkout.

#!/bin/bash
set -euo pipefail

echo "Unpinned action refs:"
rg -n '^\s*uses:\s*[^@]+@(v[0-9]+|stable|main|master)\s*$' .github/workflows/desktop-build.yml

echo
echo "Checkout credential persistence setting:"
rg -n '^\s*- name:\s*Checkout|persist-credentials' .github/workflows/desktop-build.yml -C2

Also applies to: 38-38, 44-44, 59-59

🤖 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 @.github/workflows/desktop-build.yml around lines 34 - 35, Pin every "uses:"
entry to a full commit SHA instead of floating refs (e.g., replace uses:
actions/checkout@v4 with the corresponding commit SHA for actions/checkout and
do the same for any other uses: entries currently pinned to tags/branches), and
harden the Checkout step by adding persist-credentials: false under the step
that contains uses: actions/checkout@... (ensure the key is present and set to
false). Locate these changes by searching for the literal "uses:
actions/checkout@" and other "uses:" entries in the workflow and update each to
its full commit SHA; add or update the "persist-credentials" field in the
checkout step to false so credentials are not persisted.
src-tauri/src/lib.rs (2)

116-124: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Kill the spawned backend if readiness times out.

If wait_for_backend() times out, the function returns an error without terminating the already-spawned child process, leaving an orphaned backend running.

🧹 Proposed fix to clean up on timeout
-  let child = command
+  let mut child = command
     .spawn()
     .map_err(|err| format!("unable to start codexmate backend with Node.js: {err}"))?;
 
   if !wait_for_backend(Duration::from_secs(15)) {
+    let _ = child.kill();
+    let _ = child.wait();
     return Err("codexmate backend did not become ready on 127.0.0.1:3737".into());
   }
 
   Ok(Some(child))
🤖 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-tauri/src/lib.rs` around lines 116 - 124, After spawning the child with
command.spawn() and before returning an error when
wait_for_backend(Duration::from_secs(15)) returns false, ensure you terminate
the spawned process: call child.kill() (handle/ignore errors if the process
already exited) and then call child.wait() to reap it; only after
killing/waiting return the error string. This guarantees the spawned child
(variable child from command.spawn()) is cleaned up on wait_for_backend timeout.

39-44: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the readiness probe validate backend identity, not just status code.

The current health check only verifies HTTP 200 status, which could false-positive if an unrelated service is running on 127.0.0.1:3737, causing the app to skip backend startup or connect to the wrong service.

🔍 Proposed fix to validate backend identity
   let mut response = String::new();
   if stream.read_to_string(&mut response).is_err() {
     return false;
   }
-  response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200")
+  (response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200"))
+    && response.contains("\"ok\":true")

This checks for a backend-specific token in the response body to confirm identity.

🤖 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-tauri/src/lib.rs` around lines 39 - 44, The readiness probe currently
only checks the HTTP status by calling stream.read_to_string(&mut response) and
response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200");
update this logic to also validate a backend-specific identity token in the
response body (e.g., a known marker like "X-BACKEND-ID: my-backend" or
"MY_BACKEND_READY") before returning true, so after successful read_to_string
verify both the 200 status and response.contains("<IDENTITY_TOKEN>") and return
false if the token is missing.
🤖 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 `@src-tauri/tauri.conf.json`:
- Around line 26-28: Replace the null Content Security Policy in tauri.conf.json
by setting the security.csp key to a restrictive CSP string that permits only
'self' and the local backend origin (http://127.0.0.1:3737) and only the minimal
additional directives your frontend needs (e.g., avoid unsafe-eval, avoid
unsafe-inline unless absolutely required); update the security.csp value (the
"security": {"csp": ...} entry) accordingly so CSP is enforced rather than
disabled.

---

Duplicate comments:
In @.github/workflows/desktop-build.yml:
- Around line 34-35: Pin every "uses:" entry to a full commit SHA instead of
floating refs (e.g., replace uses: actions/checkout@v4 with the corresponding
commit SHA for actions/checkout and do the same for any other uses: entries
currently pinned to tags/branches), and harden the Checkout step by adding
persist-credentials: false under the step that contains uses:
actions/checkout@... (ensure the key is present and set to false). Locate these
changes by searching for the literal "uses: actions/checkout@" and other "uses:"
entries in the workflow and update each to its full commit SHA; add or update
the "persist-credentials" field in the checkout step to false so credentials are
not persisted.

In `@src-tauri/src/lib.rs`:
- Around line 116-124: After spawning the child with command.spawn() and before
returning an error when wait_for_backend(Duration::from_secs(15)) returns false,
ensure you terminate the spawned process: call child.kill() (handle/ignore
errors if the process already exited) and then call child.wait() to reap it;
only after killing/waiting return the error string. This guarantees the spawned
child (variable child from command.spawn()) is cleaned up on wait_for_backend
timeout.
- Around line 39-44: The readiness probe currently only checks the HTTP status
by calling stream.read_to_string(&mut response) and
response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200");
update this logic to also validate a backend-specific identity token in the
response body (e.g., a known marker like "X-BACKEND-ID: my-backend" or
"MY_BACKEND_READY") before returning true, so after successful read_to_string
verify both the 200 status and response.contains("<IDENTITY_TOKEN>") and return
false if the token is missing.

In `@tests/unit/desktop-stage.test.mjs`:
- Around line 54-56: The test currently asserts full-object equality on
tauriConfig.bundle.resources which is brittle; update the assertion in
tests/unit/desktop-stage.test.mjs to check only that the required mapping exists
(e.g., assert that tauriConfig.bundle.resources['../dist/desktop/codexmate']
equals 'codexmate' or that
Object.prototype.hasOwnProperty.call(tauriConfig.bundle.resources,
'../dist/desktop/codexmate') and its value is 'codexmate') so extra resources
won’t break the test while still verifying the necessary mapping.

In `@tools/desktop/prepare-tauri-resources.js`:
- Around line 48-53: The code uses fs.cpSync in prepare-tauri-resources.js which
requires a newer Node than the project's declared engines.node (>=14); either
update package.json engines.node to a minimum Node version that supports
fs.cpSync (e.g., ">=16.7.0") or replace the fs.cpSync call with a
Node-14-compatible recursive copier: implement a copyDirSync function (using
fs.readdirSync, fs.statSync/lstatSync to honor dereference, fs.mkdirSync,
fs.copyFileSync and handling overwrite/force) and apply the same filter logic
(skip paths containing '.git') before calling it in place of fs.cpSync; ensure
the replacement preserves recursive, force/overwrite, dereference, and filter
behavior used in the original fs.cpSync invocation.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e089cd15-bd97-4c65-8b49-bc57bab0cb3b

📥 Commits

Reviewing files that changed from the base of the PR and between 0e38488 and d69d704.

⛔ Files ignored due to path filters (16)
  • package-lock.json is excluded by !**/package-lock.json
  • src-tauri/icons/128x128.png is excluded by !**/*.png
  • src-tauri/icons/128x128@2x.png is excluded by !**/*.png
  • src-tauri/icons/32x32.png is excluded by !**/*.png
  • src-tauri/icons/Square107x107Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square142x142Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square150x150Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square284x284Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square30x30Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square310x310Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square44x44Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square71x71Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square89x89Logo.png is excluded by !**/*.png
  • src-tauri/icons/StoreLogo.png is excluded by !**/*.png
  • src-tauri/icons/icon.ico is excluded by !**/*.ico
  • src-tauri/icons/icon.png is excluded by !**/*.png
📒 Files selected for processing (15)
  • .github/workflows/desktop-build.yml
  • doc/desktop.md
  • package.json
  • src-tauri/.gitignore
  • src-tauri/Cargo.toml
  • src-tauri/build.rs
  • src-tauri/capabilities/default.json
  • src-tauri/icons/icon.icns
  • src-tauri/src/lib.rs
  • src-tauri/src/main.rs
  • src-tauri/tauri.conf.json
  • tests/unit/desktop-stage.test.mjs
  • tests/unit/npm-package-files.test.mjs
  • tests/unit/run.mjs
  • tools/desktop/prepare-tauri-resources.js
📜 Review details
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/desktop-build.yml

[warning] 34-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 44-44: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 59-59: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🔇 Additional comments (11)
doc/desktop.md (1)

1-61: LGTM!

package.json (1)

45-49: LGTM!

Also applies to: 79-80

tests/unit/npm-package-files.test.mjs (1)

20-25: LGTM!

tests/unit/run.mjs (1)

57-57: LGTM!

src-tauri/.gitignore (1)

1-4: LGTM!

src-tauri/build.rs (1)

1-3: LGTM!

src-tauri/capabilities/default.json (1)

1-11: LGTM!

src-tauri/src/lib.rs (2)

146-158: LGTM!


160-164: LGTM!

src-tauri/src/main.rs (1)

1-6: LGTM!

src-tauri/Cargo.toml (1)

3-3: ⚡ Quick win

Check src-tauri/Cargo.toml for current/secure Rust + Tauri pins

  • crates.io latest: tauri 2.11.2, tauri-build 2.6.2, tauri-plugin-log 2.8.0.
  • Security advisory affects tauri versions >= 2.0.0, <= 2.11.0 (patched in 2.11.1), so the pinned tauri version must be >= 2.11.1.
  • The provided snippet only shows version = "0.0.36"; confirm the actual tauri* dependency versions and [package].rust-version in src-tauri/Cargo.toml and update if they fall into vulnerable/outdated ranges (Rust stable is currently 1.95.0).

Comment thread src-tauri/tauri.conf.json
@awsl233777
Copy link
Copy Markdown
Collaborator Author

Updated the desktop app icons from site/.vitepress/public/images/logo-v.png using npx tauri icon.

Validation run locally:

  • npm run desktop:stage
  • node tests/unit/run.mjs ✅ All 532 tests passed

Notes:

  • The generated icon set updates the configured Tauri bundle icons: 32x32.png, 128x128.png, 128x128@2x.png, icon.icns, and icon.ico.
  • The generator also produced the additional Tauri platform icon assets under src-tauri/icons/, including Appx, iOS, and Android sizes.

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

@awsl233777 Thanks for the icon update! The generated set covers all required Tauri targets (macOS .icns, Windows .ico, and standard PNG sizes) as well as the extra platform assets. Triggering a full re-review now. 🐇

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
.github/workflows/desktop-build.yml (1)

34-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden workflow actions by pinning SHAs and disabling checkout credential persistence.

The workflow still uses floating action refs and keeps default checkout credential persistence. Pin each uses: to a full commit SHA and set persist-credentials: false in the checkout step.

#!/bin/bash
set -euo pipefail

echo "All action references:"
rg -n '^\s*uses:\s*' .github/workflows/desktop-build.yml

echo
echo "Checkout credential persistence setting:"
rg -n 'persist-credentials' .github/workflows/desktop-build.yml || true

Also applies to: 38-38, 44-44, 59-59

🤖 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 @.github/workflows/desktop-build.yml around lines 34 - 35, Replace floating
action refs and enable non-persistent checkout credentials: pin every "uses:"
action to a full commit SHA instead of tags (e.g., replace uses:
actions/checkout@v4 with a specific commit SHA) and add persist-credentials:
false to the checkout step; apply these changes for the checkout occurrences and
any other "uses:" lines referenced in the file so all actions are SHA-pinned and
the Checkout step sets persist-credentials: false.
tools/desktop/prepare-tauri-resources.js (1)

48-53: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align runtime floor with fs.cpSync usage.

fs.cpSync at Line 48 is incompatible with the declared node >=14 support, so desktop:stage can fail on supported runtimes. Either raise the engine floor for desktop-supporting environments or replace this with a Node-14-compatible recursive copy path.

#!/bin/bash
set -euo pipefail

echo "Declared Node engine:"
rg -n '"engines"\s*:\s*\{|"node"\s*:' package.json -n -A2 -B1

echo
echo "Node APIs used by desktop staging script:"
rg -n '\bfs\.cpSync\b|\bfs\.rmSync\b' tools/desktop/prepare-tauri-resources.js -C2
🤖 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 `@tools/desktop/prepare-tauri-resources.js` around lines 48 - 53, The script
prepare-tauri-resources.js currently calls fs.cpSync (symbol: fs.cpSync) which
is only available in Node >=16 and thus conflicts with the declared node >=14
engine; either raise the project's Node engine to a version that supports
fs.cpSync or replace this call with a Node-14-compatible implementation (e.g.,
use fs-extra.copySync or implement a recursive copy helper and use
fs.readdir/fs.copyFile/fs.mkdir) and preserve the existing options (recursive,
force, dereference, filter logic that excludes '.git'); update any
imports/require usage accordingly and ensure prepare-tauri-resources.js
continues to exclude .git paths as before.
src-tauri/src/lib.rs (2)

116-122: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Terminate the spawned backend when readiness times out.

Lines 116–121 return an error without killing/reaping the child process, leaving a stray backend process running.

🛠️ Suggested fix
-  let child = command
+  let mut child = command
     .spawn()
     .map_err(|err| format!("unable to start codexmate backend with Node.js: {err}"))?;
 
   if !wait_for_backend(Duration::from_secs(15)) {
+    let _ = child.kill();
+    let _ = child.wait();
     return Err("codexmate backend did not become ready on 127.0.0.1:3737".into());
   }
🤖 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-tauri/src/lib.rs` around lines 116 - 122, The code spawns a child process
into the variable child but returns early when
wait_for_backend(Duration::from_secs(15)) is false without killing or reaping
that child; update the timeout branch to terminate and reap the process: call
child.kill() (handle/ignore any kill error) and then child.wait() (or
wait_with_output/try_wait) before returning the Err so the spawned backend is
not left running; keep references to the child variable and add minimal error
handling/logging around kill() and wait() in the same scope where
wait_for_backend is checked.

39-44: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make readiness verification backend-specific, not status-only.

Line 43 treats any local HTTP 200 as healthy, which can false-positive against an unrelated service on 127.0.0.1:3737.

✅ Suggested fix
-  response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200")
+  let status_ok = response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200");
+  let identity_ok = response.contains("\"ok\":true");
+  status_ok && identity_ok
🤖 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-tauri/src/lib.rs` around lines 39 - 44, The current readiness check only
inspects the HTTP status line using response.starts_with(...) which can
false-positive against any service on 127.0.0.1:3737; update the readiness logic
(the block that calls stream.read_to_string(&mut response) and currently returns
based on response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0
200")) to validate a backend-specific signature instead: parse the HTTP response
and verify both the 200 status and a unique marker (e.g., a specific response
path/body JSON field or header such as an X-Backend or a known body token) that
your backend exposes so only the intended service is considered healthy. Ensure
you still handle read errors from stream.read_to_string and return false on
failure, but replace the bare status-line check with a combined status +
backend-identifier check using the response string you already populate.
src-tauri/tauri.conf.json (1)

26-28: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enable a restrictive CSP instead of disabling it.

Line 27 sets CSP to null, which removes webview-side injection protections even for localhost content.

🔒 Suggested fix
   "security": {
-    "csp": null
+    "csp": "default-src 'self' http://127.0.0.1:3737; connect-src 'self' http://127.0.0.1:3737; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'"
   }
🤖 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-tauri/tauri.conf.json` around lines 26 - 28, The CSP is currently
disabled by setting the "csp" key to null inside the "security" object; replace
that with a restrictive Content Security Policy string (rather than null) to
re-enable webview-side protections—update the "security"."csp" value to a
restrictive policy that limits sources to self and local dev endpoints (for
example, a policy restricting default-src to 'self' and localhost, limiting
script-src/style-src/connect-src to 'self' and localhost, and allowing images
from 'self' and data:); ensure the "csp" key contains that policy string so the
tauri webview enforces it instead of being disabled.
🤖 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 `@src-tauri/src/lib.rs`:
- Around line 74-76: Remove the unsafe production fallback that adds
current_dir.join("cli.js") to candidates; instead only allow this behavior in
non-production (debug) builds. Locate the block that pushes
current_dir.join("cli.js") into the candidates vector (the current_dir / cli.js
fallback) and wrap it behind a debug-only guard (e.g., cfg!(debug_assertions))
so that in release builds the current working directory is not used as a
resource lookup candidate.

---

Duplicate comments:
In @.github/workflows/desktop-build.yml:
- Around line 34-35: Replace floating action refs and enable non-persistent
checkout credentials: pin every "uses:" action to a full commit SHA instead of
tags (e.g., replace uses: actions/checkout@v4 with a specific commit SHA) and
add persist-credentials: false to the checkout step; apply these changes for the
checkout occurrences and any other "uses:" lines referenced in the file so all
actions are SHA-pinned and the Checkout step sets persist-credentials: false.

In `@src-tauri/src/lib.rs`:
- Around line 116-122: The code spawns a child process into the variable child
but returns early when wait_for_backend(Duration::from_secs(15)) is false
without killing or reaping that child; update the timeout branch to terminate
and reap the process: call child.kill() (handle/ignore any kill error) and then
child.wait() (or wait_with_output/try_wait) before returning the Err so the
spawned backend is not left running; keep references to the child variable and
add minimal error handling/logging around kill() and wait() in the same scope
where wait_for_backend is checked.
- Around line 39-44: The current readiness check only inspects the HTTP status
line using response.starts_with(...) which can false-positive against any
service on 127.0.0.1:3737; update the readiness logic (the block that calls
stream.read_to_string(&mut response) and currently returns based on
response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200")) to
validate a backend-specific signature instead: parse the HTTP response and
verify both the 200 status and a unique marker (e.g., a specific response
path/body JSON field or header such as an X-Backend or a known body token) that
your backend exposes so only the intended service is considered healthy. Ensure
you still handle read errors from stream.read_to_string and return false on
failure, but replace the bare status-line check with a combined status +
backend-identifier check using the response string you already populate.

In `@src-tauri/tauri.conf.json`:
- Around line 26-28: The CSP is currently disabled by setting the "csp" key to
null inside the "security" object; replace that with a restrictive Content
Security Policy string (rather than null) to re-enable webview-side
protections—update the "security"."csp" value to a restrictive policy that
limits sources to self and local dev endpoints (for example, a policy
restricting default-src to 'self' and localhost, limiting
script-src/style-src/connect-src to 'self' and localhost, and allowing images
from 'self' and data:); ensure the "csp" key contains that policy string so the
tauri webview enforces it instead of being disabled.

In `@tools/desktop/prepare-tauri-resources.js`:
- Around line 48-53: The script prepare-tauri-resources.js currently calls
fs.cpSync (symbol: fs.cpSync) which is only available in Node >=16 and thus
conflicts with the declared node >=14 engine; either raise the project's Node
engine to a version that supports fs.cpSync or replace this call with a
Node-14-compatible implementation (e.g., use fs-extra.copySync or implement a
recursive copy helper and use fs.readdir/fs.copyFile/fs.mkdir) and preserve the
existing options (recursive, force, dereference, filter logic that excludes
'.git'); update any imports/require usage accordingly and ensure
prepare-tauri-resources.js continues to exclude .git paths as before.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 20600851-56c4-4332-9661-57ed83462160

📥 Commits

Reviewing files that changed from the base of the PR and between 0e38488 and dcb7205.

⛔ Files ignored due to path filters (54)
  • package-lock.json is excluded by !**/package-lock.json
  • site/.vitepress/public/images/logo-v.png is excluded by !**/*.png
  • site/.vitepress/public/images/logo.png is excluded by !**/*.png
  • site/.vitepress/public/images/logo.svg is excluded by !**/*.svg
  • site/.vitepress/public/images/web-ui-screenshot.png is excluded by !**/*.png
  • src-tauri/icons/128x128.png is excluded by !**/*.png
  • src-tauri/icons/128x128@2x.png is excluded by !**/*.png
  • src-tauri/icons/32x32.png is excluded by !**/*.png
  • src-tauri/icons/64x64.png is excluded by !**/*.png
  • src-tauri/icons/Square107x107Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square142x142Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square150x150Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square284x284Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square30x30Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square310x310Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square44x44Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square71x71Logo.png is excluded by !**/*.png
  • src-tauri/icons/Square89x89Logo.png is excluded by !**/*.png
  • src-tauri/icons/StoreLogo.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-hdpi/ic_launcher.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-hdpi/ic_launcher_foreground.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-hdpi/ic_launcher_round.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-mdpi/ic_launcher.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-mdpi/ic_launcher_foreground.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-mdpi/ic_launcher_round.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xhdpi/ic_launcher.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xhdpi/ic_launcher_foreground.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xhdpi/ic_launcher_round.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xxhdpi/ic_launcher.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_foreground.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_round.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_foreground.png is excluded by !**/*.png
  • src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_round.png is excluded by !**/*.png
  • src-tauri/icons/icon.ico is excluded by !**/*.ico
  • src-tauri/icons/icon.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-20x20@1x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-20x20@2x-1.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-20x20@2x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-20x20@3x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-29x29@1x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-29x29@2x-1.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-29x29@2x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-29x29@3x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-40x40@1x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-40x40@2x-1.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-40x40@2x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-40x40@3x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-512@2x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-60x60@2x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-60x60@3x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-76x76@1x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-76x76@2x.png is excluded by !**/*.png
  • src-tauri/icons/ios/AppIcon-83.5x83.5@2x.png is excluded by !**/*.png
📒 Files selected for processing (17)
  • .github/workflows/desktop-build.yml
  • doc/desktop.md
  • package.json
  • src-tauri/.gitignore
  • src-tauri/Cargo.toml
  • src-tauri/build.rs
  • src-tauri/capabilities/default.json
  • src-tauri/icons/android/mipmap-anydpi-v26/ic_launcher.xml
  • src-tauri/icons/android/values/ic_launcher_background.xml
  • src-tauri/icons/icon.icns
  • src-tauri/src/lib.rs
  • src-tauri/src/main.rs
  • src-tauri/tauri.conf.json
  • tests/unit/desktop-stage.test.mjs
  • tests/unit/npm-package-files.test.mjs
  • tests/unit/run.mjs
  • tools/desktop/prepare-tauri-resources.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Windows
  • GitHub Check: macOS
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/desktop-build.yml

[warning] 34-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 44-44: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 59-59: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🔇 Additional comments (13)
doc/desktop.md (1)

1-61: LGTM!

tests/unit/desktop-stage.test.mjs (1)

54-56: ⚡ Quick win

Assert only the required bundle.resources mapping.

This assertion is brittle because it enforces the entire object shape, not just the contract required by this PR.

Proposed fix
-    assert.deepStrictEqual(tauriConfig.bundle.resources, {
-        '../dist/desktop/codexmate': 'codexmate'
-    });
+    assert.strictEqual(
+        tauriConfig.bundle?.resources?.['../dist/desktop/codexmate'],
+        'codexmate',
+        'tauri bundle.resources must map ../dist/desktop/codexmate to codexmate'
+    );
package.json (1)

45-49: LGTM!

Also applies to: 79-80

tests/unit/npm-package-files.test.mjs (1)

20-25: LGTM!

tests/unit/run.mjs (1)

57-57: LGTM!

src-tauri/.gitignore (1)

1-4: LGTM!

src-tauri/Cargo.toml (1)

1-25: LGTM!

src-tauri/build.rs (1)

1-3: LGTM!

src-tauri/capabilities/default.json (1)

1-11: LGTM!

src-tauri/icons/android/mipmap-anydpi-v26/ic_launcher.xml (1)

1-5: LGTM!

src-tauri/icons/android/values/ic_launcher_background.xml (1)

1-4: LGTM!

src-tauri/src/main.rs (2)

1-2: LGTM!


4-6: ⚡ Quick win

Confirm app_lib matches the Tauri library crate and run() is exported

  • src-tauri/Cargo.toml sets [lib] name = "app_lib", matching app_lib::run() in src-tauri/src/main.rs.
  • src-tauri/src/lib.rs exports pub fn run().
  • While run() returns (), it still handles failures via ? in .setup(...) and .expect("error while running tauri application").

Comment thread src-tauri/src/lib.rs
@ymkiux
Copy link
Copy Markdown
Collaborator

ymkiux commented May 27, 2026

兔子审查 - 必须修复的问题

🔴 Critical (安全/资源泄漏)

1. .github/workflows/desktop-build.yml - Actions 未固定 + 凭证持久化

zizmor 标记风险

  • ❌ 使用浮动引用 @v4 而非固定 commit SHA
  • ❌ checkout 缺少 persist-credentials: false

修复

- uses: actions/checkout@11bd71901bb5bfa8cfe5dfc19f1acec4b1441d73
  with:
    persist-credentials: false

2. src-tauri/src/lib.rs:116-122 - 子进程泄漏

问题wait_for_backend 超时后,spawn 的 Node.js 进程变成孤儿进程

修复

let mut child = command  // 改为 mut
    .spawn()
    .map_err(|err| format!("unable to start codexmate backend with Node.js: {err}"))?;

if !wait_for_backend(Duration::from_secs(15)) {
    let _ = child.kill();
    let _ = child.wait();
    return Err("codexmate backend did not become ready on 127.0.0.1:3737".into());
}

3. src-tauri/src/lib.rs:39-44 - 健康检查误判

问题:只检查 HTTP 200,3737 端口任何服务都会通过

修复:验证 backend 特定响应

let status_ok = response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200");
let identity_ok = response.contains("\"ok\":true");
status_ok && identity_ok

4. src-tauri/tauri.conf.json:26-28 - CSP 被禁用

问题"csp": null 移除了 webview 注入保护

修复

"security": {
  "csp": "default-src 'self' http://127.0.0.1:3737; connect-src 'self' http://127.0.0.1:3737; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'"
}

5. tools/desktop/prepare-tauri-resources.js:48-53 - Node 版本冲突

问题fs.cpSync 需要 Node ≥16,但 package.json 声明 >=14

修复(二选一)

  • A 提升 engines:"engines": { "node": ">=16.14.0" }
  • B 用兼容实现替换 fs.cpSync

🟡 Nice to Have

6. tests/unit/desktop-stage.test.mjs:54-56 - 脆弱断言

改用属性检查而非全对象相等

@awsl233777
Copy link
Copy Markdown
Collaborator Author

@coderabbitai re-review

I addressed the findings from #172 (comment) in commit 6242a2b:

  • Pinned the desktop workflow actions to commit SHAs and set actions/checkout persist-credentials: false.
  • Kill and wait for the spawned backend process when desktop startup readiness times out.
  • Tightened the desktop backend health check to require both HTTP 200 and the Codex Mate {"ok":true} health response.
  • Replaced the disabled Tauri CSP with a restricted policy and made the desktop staging script preserve that CSP.
  • Raised the Node engine requirement to >=16.14.0 in package.json and package-lock.json to match fs.cpSync usage.
  • Replaced the brittle desktop staging resource deep-equality assertion with targeted property checks and added CSP assertions.

Validation actually run:

npm run test:unit
All 532 tests passed.

Local limitation: this environment does not have cargo, so Rust fmt/check must be covered by CI.

Could you please verify whether the previously listed issues are now resolved?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== 1. .github/workflows/desktop-build.yml ==="
cat .github/workflows/desktop-build.yml

echo ""
echo "=== 2 & 3. src-tauri/src/lib.rs ==="
cat src-tauri/src/lib.rs

echo ""
echo "=== 4. src-tauri/tauri.conf.json ==="
cat src-tauri/tauri.conf.json

echo ""
echo "=== 5. package.json (engines) ==="
python3 -c "import json; d=json.load(open('package.json')); print('engines:', d.get('engines')); print('devDeps tauri:', d.get('devDependencies',{}).get('`@tauri-apps/cli`'))"

echo ""
echo "=== 6. tests/unit/desktop-stage.test.mjs ==="
cat tests/unit/desktop-stage.test.mjs

Length of output: 10697



@awsl233777 All six issues from the previous review have been confirmed resolved in commit 6242a2b. Here's the verification summary:

# Issue Status
1 Workflow actions pinned + persist-credentials: false ✅ All four actions (checkout, setup-node, rust-toolchain, upload-artifact) are pinned to full commit SHAs; persist-credentials: false is set.
2 Child process kill on backend readiness timeout child is now mut; child.kill() + child.wait() are called when wait_for_backend returns false.
3 Health-check identity verification ✅ Requires both HTTP 200 (status_ok) and "ok":true in the response body (identity_ok).
4 CSP no longer disabled "csp" is set to a restrictive policy (default-src 'self' http://127.0.0.1:3737; connect-src ...; img-src ...; style-src ...; script-src 'self') instead of null.
5 Node engine requirement raised to >=16.14.0 package.json engines.node is ">=16.14.0".
6 Brittle deep-equality assertion replaced ✅ The test now uses targeted assert.strictEqual / assert.ok / assert.match checks, including explicit CSP pattern assertions.

The only remaining item (Rust fmt/clippy checks) is correctly deferred to the CI environment — that's expected given the local cargo constraint. Everything looks good. 🐇 ✨

✅ Actions performed

Full review triggered.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

@coderabbitai re-review

Follow-up on the remaining desktop review item:

  • Restricted the current_dir.join("cli.js") fallback to debug builds only with #[cfg(debug_assertions)], so release builds only use bundled resource paths.

Latest validation actually run after this change:

npm run test:unit
All 532 tests passed.

The prior six items from #172 (comment) remain addressed, and CI on the previous head was green before this follow-up commit. Please verify whether the remaining review concern is now resolved on the latest head.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

Your plan includes 2 PR reviews per hour. More reviews will be available in 19 minutes and 40 seconds.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

@coderabbitai re-review

Follow-up release packaging change in b13f13a:

  • Split the release workflow into resolve, desktop, and release jobs.
  • Build macOS and Windows Tauri installers during release.
  • Upload only installer-friendly desktop outputs from the desktop jobs:
    • *.dmg
    • *.msi
    • *.exe
  • Download those desktop artifacts in the release job and attach them directly as GitHub Release assets alongside the existing npm .tgz and standalone .tar.gz artifacts.
  • Added a unit contract test so the release workflow keeps attaching desktop installer assets instead of regressing to Actions artifact-only output.
  • Pinned the newly touched release workflow actions to commit SHAs and disabled checkout credential persistence.

Validation actually run:

npm run test:unit
All 533 tests passed.

git diff --check
(no output)

Please verify the latest head, especially the release workflow artifact handoff and release asset globs.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Release workflow ==="
fd -e yml -e yaml . .github/workflows/ --exec cat {}

echo ""
echo "=== Desktop build workflow ==="
cat .github/workflows/desktop-build.yml 2>/dev/null || echo "not found"

echo ""
echo "=== Unit test files (new contract test) ==="
fd 'release' tests/unit/ 2>/dev/null || fd '\.test\.mjs$' tests/unit/ | sort

echo ""
echo "=== tests/unit/run.mjs ==="
cat tests/unit/run.mjs

echo ""
echo "=== All test mjs files listing ==="
fd '\.test\.mjs$' tests/unit/ | sort

Length of output: 25332


🏁 Script executed:

#!/bin/bash
# Read any new release-related test files
echo "=== release workflow contract test (if any) ==="
fd 'release' tests/unit/ 2>/dev/null
fd 'release' tests/ 2>/dev/null

echo ""
echo "=== Newer test files ==="
ls -la tests/unit/

echo ""
echo "=== package.json scripts section ==="
python3 -c "import json; d=json.load(open('package.json')); [print(k,'=',v) for k,v in d.get('scripts',{}).items()]"

echo ""
echo "=== package.json engines ==="
python3 -c "import json; d=json.load(open('package.json')); print(d.get('engines'))"

Length of output: 7351


🏁 Script executed:

#!/bin/bash
echo "=== release-workflow-contract.test.mjs ==="
cat tests/unit/release-workflow-contract.test.mjs

echo ""
echo "=== Checking ci.yml for unpinned actions ==="
cat .github/workflows/ci.yml

echo ""
echo "=== Release workflow: softprops action SHA verification ==="
grep -n "softprops\|download-artifact\|upload-artifact\|checkout\|setup-node\|rust-toolchain" .github/workflows/release.yml

Length of output: 3519



@awsl233777 Verified commit b13f13a. Here's the full check:

# Item Status
1 resolvedesktoprelease job split ✅ Three jobs confirmed; desktop needs resolve, release needs both.
2 Desktop installer upload globs ✅ Desktop job uploads bundle/dmg/*.dmg, bundle/msi/*.msi, bundle/nsis/*.exe with if-no-files-found: error.
3 Artifact download → release asset handoff ✅ Release job downloads via pattern: codexmate-desktop-*, merge-multiple: true into desktop-release-assets/, then attaches `desktop-release-assets/**/*.dmg
4 All actions pinned to commit SHAs ✅ All six actions in release.yml (checkout, setup-node, rust-toolchain, upload-artifact, download-artifact, softprops/action-gh-release) are pinned to full SHAs.
5 persist-credentials: false ✅ All three checkout steps (resolve, desktop, release jobs) have it set.
6 Unit contract test registered tests/unit/release-workflow-contract.test.mjs exists and is imported in run.mjs. It asserts the full pipeline: job ordering, matrix usage, all three upload globs, download pattern, merge-multiple: true, and all three release asset globs.

One minor pre-existing note (out of scope for this PR): .github/workflows/ci.yml still uses floating actions/checkout@v4 and actions/setup-node@v4 references — that workflow was not modified here, so it's not a blocker, but worth a follow-up hardening pass.

Everything in scope for this PR looks solid. 🐇 ✨

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

@coderabbitai re-review

Follow-up commit e91b464 adds a desktop icon contract test after manually verifying the Tauri icon chain:

  • asserts src-tauri/tauri.conf.json bundle.icon references the desktop icons Tauri should consume:
    • icons/32x32.png
    • icons/128x128.png
    • icons/128x128@2x.png
    • icons/icon.icns
    • icons/icon.ico
  • asserts the generated PNG sizes for the desktop and Windows Store icon files
  • parses icon.ico and confirms it contains 16/24/32/48/64/256 px entries
  • parses icon.icns and confirms it contains the expected macOS ICNS entries (ic07 through ic14 plus 16/32 mask entries)

Verified locally after the test addition:

  • targeted desktop-stage tests passed
  • npm run test:unitAll 539 tests passed.
  • npm run lintLint passed for 307 file(s).
  • git diff --check → passed

Please confirm whether the previous desktop icon distribution is correct across the generated sizes and whether the icons are truly used by the Tauri desktop bundle configuration. If there is still an icon packaging/usage gap, please point to the exact missing platform, size, or config path.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
tests/unit/session-usage-backend.test.mjs (1)

254-301: ⚡ Quick win

Add failure-path tests for invalid format and invalid date ranges.

Current coverage is success-only. Please add explicit assertions for invalid format and invalid from/to combinations to lock export contract 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 `@tests/unit/session-usage-backend.test.mjs` around lines 254 - 301, Add tests
that assert exportSessionUsageCore rejects when given an unsupported format and
when given invalid date ranges; specifically, call
usageCore.exportSessionUsageCore with format: 'xml' and assert.rejects the
promise (or try/catch) to ensure an error is thrown, and add cases where from is
after to (e.g., from: '2026-05-10', to: '2026-05-01') and where dates are
malformed (e.g., from: 'invalid'), asserting rejection for each; reference the
exported function usageCore.exportSessionUsageCore and mirror the existing test
style (assert.rejects or async/await with try/catch) to lock the contract for
format and date validation.
tests/unit/analytics-export-args.test.mjs (1)

46-56: ⚡ Quick win

Add missing-value coverage for value-taking flags.

Please add cases like ['--source'] and ['--from', '--to', '2026-05-06'] to assert parser errors. This branch is currently untested and is easy to regress.

🤖 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 `@tests/unit/analytics-export-args.test.mjs` around lines 46 - 56, The test
currently checks invalid tokens and choices but omits coverage for flags that
require values; add additional assertions calling parseAnalyticsExportArgs with
value-taking flags left without values (e.g., ['--source'] and ['--from',
'--to', '2026-05-06']) and assert that parsed.error contains the appropriate
"missing value" messages; update the existing test case (test name
parseAnalyticsExportArgs reports unknown tokens and invalid choices) or add a
new test that calls parseAnalyticsExportArgs with those argument arrays and
asserts parsed.error matches patterns for missing values for --source, --from,
and --to to prevent regressions in the parser logic.
🤖 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 `@cli.js`:
- Line 16025: Update the top-level analytics help string in cli.js so it
includes the --source option to match printAnalyticsUsage(); specifically edit
the console.log call that prints "codexmate analytics export ..." and add the
`--source <SOURCE>` token (or `--source` flag format used elsewhere) into the
same options list so both help surfaces stay consistent with the
printAnalyticsUsage() documentation.

In `@cli/analytics-export-args.js`:
- Around line 10-17: The readValue function is swallowing the next token even
when it's another flag; modify readValue (and the equivalent parsing block
around lines 22–39) to validate that args[index + 1] exists and does not start
with '-' before consuming it—if the next token is missing or begins with '-'
throw or return a clear parse error indicating the flag (e.g., the current
token/flag) is missing its value; keep using token.startsWith(`${flag}=`) for
inline values but otherwise do not unconditionally increment index or return
args[index+1] without this validation.

In `@cli/session-usage.js`:
- Around line 159-162: normalizeUsageExportFormat currently coerces any unknown
format to 'csv', hiding invalid input; update normalizeUsageExportFormat to
validate the normalized value and only return 'json' or 'csv' and otherwise
throw a clear error (e.g., throw new TypeError(`Invalid export format:
${value}`)) so callers receive a validation failure; apply the same validation
change to the other duplicate implementation noted in the diff (the second
normalize/format handler around the later block) so both places consistently
reject unsupported formats instead of silently returning 'csv'.

---

Nitpick comments:
In `@tests/unit/analytics-export-args.test.mjs`:
- Around line 46-56: The test currently checks invalid tokens and choices but
omits coverage for flags that require values; add additional assertions calling
parseAnalyticsExportArgs with value-taking flags left without values (e.g.,
['--source'] and ['--from', '--to', '2026-05-06']) and assert that parsed.error
contains the appropriate "missing value" messages; update the existing test case
(test name parseAnalyticsExportArgs reports unknown tokens and invalid choices)
or add a new test that calls parseAnalyticsExportArgs with those argument arrays
and asserts parsed.error matches patterns for missing values for --source,
--from, and --to to prevent regressions in the parser logic.

In `@tests/unit/session-usage-backend.test.mjs`:
- Around line 254-301: Add tests that assert exportSessionUsageCore rejects when
given an unsupported format and when given invalid date ranges; specifically,
call usageCore.exportSessionUsageCore with format: 'xml' and assert.rejects the
promise (or try/catch) to ensure an error is thrown, and add cases where from is
after to (e.g., from: '2026-05-10', to: '2026-05-01') and where dates are
malformed (e.g., from: 'invalid'), asserting rejection for each; reference the
exported function usageCore.exportSessionUsageCore and mirror the existing test
style (assert.rejects or async/await with try/catch) to lock the contract for
format and date validation.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 12d7b70a-3f19-4dae-a706-67d7e7bb91c5

📥 Commits

Reviewing files that changed from the base of the PR and between b13f13a and c621391.

📒 Files selected for processing (6)
  • cli.js
  • cli/analytics-export-args.js
  • cli/session-usage.js
  • tests/unit/analytics-export-args.test.mjs
  • tests/unit/run.mjs
  • tests/unit/session-usage-backend.test.mjs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Windows
  • GitHub Check: macOS
🔇 Additional comments (2)
tests/unit/run.mjs (1)

30-30: LGTM!

cli.js (1)

165-166: LGTM!

Also applies to: 5208-5212, 9806-9810, 9812-9845, 11128-11141, 16118-16118

Comment thread cli.js
console.log(' codexmate delete-model <模型> 删除模型');
console.log(' codexmate workflow <list|get|validate|run|runs> MCP 工作流中心');
console.log(' codexmate task <plan|run|runs|queue|retry|cancel|logs> 本地任务编排');
console.log(' codexmate analytics export [--format csv|json] [--from YYYY-MM-DD] [--to YYYY-MM-DD] [--model <MODEL>] [--output <PATH|->] [-o <PATH|->] 导出 Usage 数据');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add --source to the top-level analytics help line.

Line [16025] omits --source, but this option is supported and documented in printAnalyticsUsage(). Keeping both help surfaces aligned avoids CLI confusion.

Suggested patch
-    console.log('  codexmate analytics export [--format csv|json] [--from YYYY-MM-DD] [--to YYYY-MM-DD] [--model <MODEL>] [--output <PATH|->] [-o <PATH|->]  导出 Usage 数据');
+    console.log('  codexmate analytics export [--format csv|json] [--from YYYY-MM-DD] [--to YYYY-MM-DD] [--model <MODEL>] [--source <codex|claude|gemini|codebuddy|all>] [--output <PATH|->] [-o <PATH|->]  导出 Usage 数据');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log(' codexmate analytics export [--format csv|json] [--from YYYY-MM-DD] [--to YYYY-MM-DD] [--model <MODEL>] [--output <PATH|->] [-o <PATH|->] 导出 Usage 数据');
console.log(' codexmate analytics export [--format csv|json] [--from YYYY-MM-DD] [--to YYYY-MM-DD] [--model <MODEL>] [--source <codex|claude|gemini|codebuddy|all>] [--output <PATH|->] [-o <PATH|->] 导出 Usage 数据');
🤖 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 `@cli.js` at line 16025, Update the top-level analytics help string in cli.js
so it includes the --source option to match printAnalyticsUsage(); specifically
edit the console.log call that prints "codexmate analytics export ..." and add
the `--source <SOURCE>` token (or `--source` flag format used elsewhere) into
the same options list so both help surfaces stay consistent with the
printAnalyticsUsage() documentation.

Comment on lines +10 to +17
const readValue = (flag) => {
if (token.startsWith(`${flag}=`)) {
return token.slice(flag.length + 1);
}
const value = args[index + 1];
index += 1;
return value;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing-value flags are silently accepted and can swallow the next flag.

For value-taking flags, readValue (Line 10–17) consumes args[index + 1] unconditionally. Inputs like --source --format json can treat --format as the --source value instead of raising a parse error.

💡 Suggested fix
 function parseAnalyticsExportArgs(args = []) {
+    const valueFlags = new Set(['--format', '--from', '--to', '--model', '--source', '--output', '-o']);
     const options = {
         format: 'csv',
         source: 'all',
         output: ''
     };
@@
         const token = String(args[index] || '');
+        const tokenKey = token.split('=')[0];
         const readValue = (flag) => {
             if (token.startsWith(`${flag}=`)) {
                 return token.slice(flag.length + 1);
             }
             const value = args[index + 1];
+            const nextKey = String(value || '').split('=')[0];
+            if (value === undefined || valueFlags.has(nextKey)) {
+                errors.push(`${flag} 需要提供值`);
+                return '';
+            }
             index += 1;
             return value;
         };

Also applies to: 22-39

🤖 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 `@cli/analytics-export-args.js` around lines 10 - 17, The readValue function is
swallowing the next token even when it's another flag; modify readValue (and the
equivalent parsing block around lines 22–39) to validate that args[index + 1]
exists and does not start with '-' before consuming it—if the next token is
missing or begins with '-' throw or return a clear parse error indicating the
flag (e.g., the current token/flag) is missing its value; keep using
token.startsWith(`${flag}=`) for inline values but otherwise do not
unconditionally increment index or return args[index+1] without this validation.

Comment thread cli/session-usage.js
Comment on lines +159 to +162
function normalizeUsageExportFormat(value) {
const normalized = typeof value === 'string' ? value.trim().toLowerCase() : '';
return normalized === 'json' ? 'json' : 'csv';
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Invalid export formats are silently coerced to CSV.

normalizeUsageExportFormat currently maps any unknown value to csv, so API callers can pass bad input (e.g. xml) and still get success instead of a validation error.

💡 Suggested fix
 function normalizeUsageExportFormat(value) {
     const normalized = typeof value === 'string' ? value.trim().toLowerCase() : '';
-    return normalized === 'json' ? 'json' : 'csv';
+    if (!normalized) return 'csv';
+    return normalized === 'json' || normalized === 'csv' ? normalized : '';
 }
@@
     const format = normalizeUsageExportFormat(params.format);
+    if (!format) {
+        return { error: 'Invalid format' };
+    }

Also applies to: 284-289

🤖 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 `@cli/session-usage.js` around lines 159 - 162, normalizeUsageExportFormat
currently coerces any unknown format to 'csv', hiding invalid input; update
normalizeUsageExportFormat to validate the normalized value and only return
'json' or 'csv' and otherwise throw a clear error (e.g., throw new
TypeError(`Invalid export format: ${value}`)) so callers receive a validation
failure; apply the same validation change to the other duplicate implementation
noted in the diff (the second normalize/format handler around the later block)
so both places consistently reject unsupported formats instead of silently
returning 'csv'.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== 1. tauri.conf.json bundle.icon section ==="
python3 -c "import json; d=json.load(open('src-tauri/tauri.conf.json')); print(json.dumps(d.get('bundle',{}).get('icon','NOT FOUND'), indent=2))"

echo ""
echo "=== 2. New icon contract test file (if present) ==="
fd 'icon' tests/unit/ | sort
fd 'icon.*contract\|contract.*icon\|desktop.*icon\|icon.*test' tests/unit/ | sort
# List all test files to find new ones
ls -la tests/unit/

echo ""
echo "=== 3. run.mjs ==="
cat tests/unit/run.mjs

echo ""
echo "=== 4. Icon files present under src-tauri/icons/ ==="
find src-tauri/icons/ -type f | sort

echo ""
echo "=== 5. Read the newest test file content ==="
fd '\.test\.mjs$' tests/unit/ | sort | xargs ls -la

Length of output: 20157


🏁 Script executed:

#!/bin/bash
echo "=== desktop-stage.test.mjs (full) ==="
cat tests/unit/desktop-stage.test.mjs

echo ""
echo "=== Check if run.mjs has any new imports vs expected ==="
grep -c "import" tests/unit/run.mjs

echo ""
echo "=== Check icon file binary headers to verify they're real PNGs, ICO, ICNS ==="
python3 - <<'EOF'
import struct, os

icons_dir = "src-tauri/icons"

# Check key PNGs have correct PNG header
for name in ["32x32.png","128x128.png","128x128@2x.png","icon.png","64x64.png"]:
    p = os.path.join(icons_dir, name)
    with open(p,"rb") as f:
        hdr = f.read(8)
        ok = hdr == b'\x89PNG\r\n\x1a\n'
        print(f"PNG {name}: {'OK' if ok else 'BAD'}")

# Check ICO header
ico_path = os.path.join(icons_dir, "icon.ico")
with open(ico_path,"rb") as f:
    data = f.read()
    reserved, img_type, count = struct.unpack_from("<HHH", data, 0)
    print(f"\nICO reserved={reserved}, type={img_type}, count={count}")
    sizes = []
    for i in range(count):
        off = 6 + i * 16
        w, h = data[off], data[off+1]
        sizes.append(w if w != 0 else 256)
    print(f"ICO sizes (width): {sorted(sizes)}")

# Check ICNS header
icns_path = os.path.join(icons_dir, "icon.icns")
with open(icns_path,"rb") as f:
    data = f.read()
    magic = data[:4]
    total_len = struct.unpack_from(">I", data, 4)[0]
    print(f"\nICNS magic={magic}, declared_len={total_len}, file_len={len(data)}")
    # Read all block type tags
    offset = 8
    tags = []
    while offset < len(data) - 8:
        tag = data[offset:offset+4].decode('ascii', errors='replace')
        blk_len = struct.unpack_from(">I", data, offset+4)[0]
        tags.append(tag)
        if blk_len < 8: break
        offset += blk_len
    print(f"ICNS block tags: {tags}")
EOF

Length of output: 8269



@awsl233777 Verified commit e91b464. All icon contract assertions are confirmed correct against the actual binary files.

# Item Status
1 bundle.icon references in tauri.conf.json ✅ Exactly the 5 expected paths: icons/32x32.png, icons/128x128.png, icons/128x128@2x.png, icons/icon.icns, icons/icon.ico
2 All 5 bundle icon files physically present ✅ All confirmed in src-tauri/icons/
3 PNG headers and dimensions ✅ All 15 named PNGs (desktop sizes + all Windows Store Square*Logo.png + StoreLogo.png) have valid \x89PNG magic; readPngSize correctly reads from the IHDR chunk at offset 16/20
4 ICO entries reserved=0, type=1, 6 images at [16, 24, 32, 48, 64, 256] — exactly matches the test assertion
5 ICNS blocks ✅ Magic icns, declared_len == file_len (2,545,705 bytes), block tags sorted: ['ic07','ic08','ic09','ic10','ic11','ic12','ic13','ic14','il32','is32','l8mk','s8mk'] — exact match
6 Test registered ✅ Icon contract test lives in desktop-stage.test.mjs (already imported in run.mjs)

No icon packaging or config gap. The icon chain is complete and the binary file assertions are accurate. 🐇 ✨

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

@coderabbitai re-review

Follow-up commit 90cab7f addresses Windows MSI startup crash debugging by making the desktop executable self-diagnosing instead of requiring users to manually dig through Event Viewer.

What changed:

  • codexmate-desktop.exe --debug-console enables Windows console diagnostics for release builds.
  • CODEXMATE_DESKTOP_LOG=1 enables the same mode through environment variables.
  • CODEXMATE_DESKTOP_LOG_FILE=<path> can override the log file location.
  • Windows release builds keep the GUI subsystem for normal double-click behavior, but debug mode tries to AttachConsole(ATTACH_PARENT_PROCESS) and writes console output through CONOUT$ as a fallback.
  • Desktop startup, backend spawn, backend readiness timeout, backend stop, and Rust panics are logged.
  • The bundled Node backend no longer drops logs:
    • debug console mode inherits stdout/stderr
    • normal GUI mode appends backend stdout/stderr to %LOCALAPPDATA%\\CodexMate\\logs\\desktop.log
  • Added docs in doc/desktop.md and a source contract test in tests/unit/desktop-diagnostics-contract.test.mjs.

Verified locally:

  • targeted desktop diagnostics/stage tests passed
  • npm run test:unitAll 541 tests passed.
  • npm run lintLint passed for 308 file(s).
  • git diff --check → passed

Local blocker: this machine does not have cargo, so Rust compile validation is intentionally left to GitHub Actions Windows/macOS desktop-build checks.

Please check whether this approach is correct for Tauri Windows release diagnostics and whether there are any remaining concerns with console attachment, backend stdio inheritance, file logging, or normal double-click UX.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== src-tauri/src/lib.rs ==="
cat src-tauri/src/lib.rs

echo ""
echo "=== tests/unit/desktop-diagnostics-contract.test.mjs ==="
cat tests/unit/desktop-diagnostics-contract.test.mjs

echo ""
echo "=== doc/desktop.md ==="
cat doc/desktop.md

echo ""
echo "=== tests/unit/run.mjs (confirm new test registered) ==="
cat tests/unit/run.mjs

echo ""
echo "=== src-tauri/src/main.rs ==="
cat src-tauri/src/main.rs

Length of output: 19421



@awsl233777 Reviewed commit 90cab7f. The overall approach is sound. Here's the full check:

# Item Status
1 AttachConsole(ATTACH_PARENT_PROCESS) + SAFETY comment extern "system" via #[link(name = "kernel32")], constant 0xFFFF_FFFF, SAFETY documented.
2 CONOUT$ fallback ✅ Inside #[cfg(windows)] only; falls through to stderr on non-Windows and when no console is attached.
3 Normal double-click UX unaffected ✅ No args/env → desktop_debug_requested() = false → DESKTOP_CONSOLE_LOGGING = false → CREATE_NO_WINDOW applied → no black console.
4 Backend stdio in debug mode Stdio::inherit() for both stdout/stderr when DESKTOP_CONSOLE_LOGGING is true.
5 Backend stdio in normal GUI mode ✅ Both streams redirected to desktop_log_stdio() (append to desktop.log).
6 File logging always active desktop_log() writes to file regardless of console logging flag — useful for post-mortem even without debug mode.
7 Panic hook std::panic::set_hook captures panics to the log file.
8 init_desktop_diagnostics() called before run() main.rs calls init_desktop_diagnostics() first; it is exported as pub.
9 Contract tests ✅ Both tests registered in run.mjs, covering all env vars/flags, AttachConsole, CONOUT$, log path, panic hook, stdio inheritance, CREATE_NO_WINDOW guard.

🟡 Minor items (non-blocking)

1. desktop_log_stdio() called twice → two file handles on the same path

command.stdout(desktop_log_stdio()).stderr(desktop_log_stdio());

Both calls open the log file with append(true). On Windows, Rust's OpenOptions sets FILE_SHARE_READ | FILE_SHARE_WRITE by default, so there's no sharing violation. However, two concurrent append handles can interleave partial writes if Node writes to both stdout and stderr simultaneously. In practice, low-volume startup logs make this a non-issue, but if you want belt-and-suspenders safety you could open once and clone the handle:

let log_stdio_out = desktop_log_stdio();
let log_stdio_err = desktop_log_stdio(); // separate append handle is fine for low-volume
command.stdout(log_stdio_out).stderr(log_stdio_err);

(No change actually needed — current code is fine for this use case.)

2. Raw epoch-ms timestamps are hard to read directly

Log lines like [1748000000000] backend spawned require external conversion. If you ever want human-readable output, replacing with an ISO 8601 formatter (even a simple YYYY-MM-DDTHH:MM:SSZ via SystemTime) would help. Not required for diagnostics but nice for usability.


Edge-case worth documenting (already implied by the docs): If a user runs codexmate-desktop.exe --debug-console by double-clicking (no parent console), AttachConsole returns 0 → DESKTOP_CONSOLE_LOGGING stays false → no console output appears, but the file log at %LOCALAPPDATA%\CodexMate\logs\desktop.log still captures everything. The doc's "尝试附着父控制台" wording correctly sets the expectation. ✅

Everything critical is correct. The diagnostics chain (env → console attach → CONOUT$ → file log → backend stdio → panic hook) is complete and the contract tests protect the key paths. 🐇 ✨

@awsl233777
Copy link
Copy Markdown
Collaborator Author

Updated the desktop startup behavior for occupied backend port cases.

What changed:

  • After attempting backend port cleanup, the desktop app now checks whether 127.0.0.1:3737 is still occupied before spawning a new backend.
  • If the port is still occupied but the existing service is not a healthy CodexMate backend, startup now fails fast with clear user guidance instead of spawning another backend and waiting for the 15s readiness timeout.
  • The guidance tells the user to close stale Codex Mate / codexmate run instances and, if needed, run Codex Mate as administrator to clean up leftover elevated processes.
  • The diagnostic log also records backend port remains occupied after cleanup for supportability.

Validation performed:

  • npm run test:unit — All 543 tests passed
  • npm run ci:lint — passed for 308 files
  • git diff --check — passed
  • GitHub checks on latest commit e60905b:
    • Node 18/20/22 CI — passed
    • pages build — passed
    • desktop-build Windows — passed
    • desktop-build macOS — passed
    • CodeRabbit — passed

Remaining note:

  • This does not require administrator privileges for normal startup. Admin launch is only surfaced as user guidance when another process still owns the backend port after cleanup.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

Updated again based on the latest Windows install/startup feedback.

Fixes in 77f6d43:

  • Startup failures now show a native Windows MessageBoxW dialog (Codex Mate 启动失败) instead of relying only on the Tauri setup error/log path.
  • If backend port 127.0.0.1:3737 remains occupied after cleanup, the app now shows user-facing guidance to close the old Codex Mate / codexmate run process or restart Codex Mate as administrator to clean up elevated leftovers.
  • Backend readiness timeout now also surfaces a user-visible startup error instead of silently leaving the user with a stuck app.
  • Windows NSIS installer now runs a preinstall hook to close an already-running codexmate-desktop.exe before copying files, improving overwrite/reinstall behavior when the old desktop app is still running.
  • Windows installer config now uses NSIS installMode: both and pins the MSI upgradeCode to keep upgrade identity stable.

Validation performed:

  • tauri.conf.json parses as valid JSON
  • npm run test:unit — All 544 tests passed
  • npm run ci:lint — passed for 308 files
  • git diff --check — passed
  • GitHub checks on latest commit 77f6d43:
    • Node 18/20/22 CI — passed
    • pages build — passed
    • desktop-build Windows — passed
    • desktop-build macOS — passed
    • CodeRabbit — passed

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== src-tauri/src/lib.rs ==="
cat src-tauri/src/lib.rs

echo ""
echo "=== src-tauri/src/main.rs ==="
cat src-tauri/src/main.rs

echo ""
echo "=== src-tauri/tauri.conf.json ==="
cat src-tauri/tauri.conf.json

echo ""
echo "=== tests/unit/run.mjs ==="
cat tests/unit/run.mjs

echo ""
echo "=== New test files ==="
fd '\.test\.mjs$' tests/unit/ | sort

echo ""
echo "=== Any new contract/diagnostics test files ==="
fd 'diagnostics\|installer\|nsis\|message\|startup' tests/unit/ | sort

echo ""
echo "=== desktop-diagnostics-contract.test.mjs ==="
cat tests/unit/desktop-diagnostics-contract.test.mjs 2>/dev/null || echo "not found"

Length of output: 31698



@awsl233777 Verified commit 77f6d43. All five items are confirmed:

# Item Status
1 MessageBoxW startup error dialog windows_dialog::show_error("Codex Mate 启动失败", message) with MB_OK | MB_ICONERROR | MB_TOPMOST, null owner, SAFETY comment, #[cfg(windows)]-gated — correct.
2 Port-occupied user guidance ✅ After release_stale_backend_port(), backend_port_occupied() is checked and startup_error(backend_port_occupied_message()) is called. The message includes administrator-restart guidance and 详情见 startup.log.
3 Readiness timeout user-visible error ✅ After child.kill() + child.wait(), startup_error("Codex Mate 后端启动后没有及时就绪...") is called — surfaces the dialog and returns Err instead of leaving the user with a stuck app.
4 NSIS preinstall hook closes running codexmate-desktop.exe "installerHooks": "windows/installer-hooks.nsh" in tauri.conf.json; contract test asserts NSIS_HOOK_PREINSTALL and taskkill /IM "codexmate-desktop.exe" /F in the hook file.
5 installMode: both + pinned upgradeCode "installMode": "both" and "upgradeCode": "e84da745-7b0b-5548-85ed-a4a0be7b55ae" confirmed in tauri.conf.json. "allowDowngrades": true also present.
6 Contract tests registered ✅ All five items covered by desktop-diagnostics-contract.test.mjs, which is imported in run.mjs.

The startup_error helper is a clean pattern — it logs, shows the dialog (Windows-only), and returns Err in one call, keeping the call sites readable. Everything looks solid. 🐇 ✨

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@doc/desktop.md`:
- Around line 67-71: Update the documentation text that currently states backend
stdout/stderr are appended to "%LOCALAPPDATA%\CodexMate\logs\desktop.log" so it
documents both "%LOCALAPPDATA%\CodexMate\logs\desktop.log" and
"%LOCALAPPDATA%\CodexMate\logs\startup.log" (the latter is used when console
logging is off), and change the nearby heading text "如需指定日志位置:" to "如需指定
`desktop.log` 位置:"; apply the same wording/path changes to the other occurrence
referenced (the block around lines 73-78) so both places correctly list
desktop.log and startup.log.

In `@src-tauri/src/lib.rs`:
- Around line 333-348: The cleanup currently kills any PID found on port 3737;
update the logic in the backend cleanup path (the block using
windows_command_line_for_pid, command_output and taskkill and the analogous
non-Windows path that uses lsof and kill) to first verify the PID is a Codex
Mate backend and that the socket is in LISTEN state before issuing a kill: check
the process command line (via windows_command_line_for_pid or parsing lsof
output) for a trusted identifier such as the backend binary name or a specific
CLI flag, and for non-Windows use lsof's state or netstat to ensure the socket
is LISTEN; if verification fails, do not call taskkill/kill and instead call
desktop_log to surface user guidance about the occupied port and how to manually
stop the backend. Ensure you change both the Windows block (where you currently
call taskkill via command_output) and the non-Windows block (where you call lsof
-ti tcp:3737 and kill -9) to follow this verification-before-kill flow and only
increment released when a verified kill succeeds.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1bceb29f-ca6d-4396-ac7b-0689485ce344

📥 Commits

Reviewing files that changed from the base of the PR and between c621391 and 77f6d43.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (10)
  • doc/desktop.md
  • package.json
  • src-tauri/Cargo.toml
  • src-tauri/src/lib.rs
  • src-tauri/src/main.rs
  • src-tauri/tauri.conf.json
  • src-tauri/windows/installer-hooks.nsh
  • tests/unit/desktop-diagnostics-contract.test.mjs
  • tests/unit/desktop-stage.test.mjs
  • tests/unit/run.mjs
✅ Files skipped from review due to trivial changes (1)
  • src-tauri/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (4)
  • src-tauri/src/main.rs
  • src-tauri/tauri.conf.json
  • package.json
  • tests/unit/desktop-stage.test.mjs
📜 Review details
🔇 Additional comments (3)
src-tauri/windows/installer-hooks.nsh (1)

1-4: LGTM!

tests/unit/run.mjs (1)

60-60: LGTM!

tests/unit/desktop-diagnostics-contract.test.mjs (1)

1-88: LGTM!

Comment thread doc/desktop.md
Comment on lines +67 to +71
启用后,桌面壳会尝试附着父控制台,打印 Rust/Tauri 启动日志,并让内置 Node backend 的 stdout/stderr 继承到当前终端。无论是否启用控制台,桌面壳都会写入本地文件日志;未启用控制台时,backend stdout/stderr 也会追加到同一个日志文件:

```text
%LOCALAPPDATA%\CodexMate\logs\desktop.log
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Document backend log file as startup.log (not only desktop.log).

The text says backend stdout/stderr are appended to the same file shown below (desktop.log), but current implementation routes backend stdio to startup.log when console logging is off. Please update the wording and paths to avoid misdirected troubleshooting.

Suggested doc patch
-启用后,桌面壳会尝试附着父控制台,打印 Rust/Tauri 启动日志,并让内置 Node backend 的 stdout/stderr 继承到当前终端。无论是否启用控制台,桌面壳都会写入本地文件日志;未启用控制台时,backend stdout/stderr 也会追加到同一个日志文件:
+启用后,桌面壳会尝试附着父控制台,打印 Rust/Tauri 启动日志,并让内置 Node backend 的 stdout/stderr 继承到当前终端。无论是否启用控制台,桌面壳都会写入本地文件日志;未启用控制台时,backend stdout/stderr 会写入 `startup.log`:
 
 ```text
 %LOCALAPPDATA%\CodexMate\logs\desktop.log
+%LOCALAPPDATA%\CodexMate\logs\startup.log

-如需指定日志位置:
+如需指定 desktop.log 位置:

</details>


Also applies to: 73-78

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @doc/desktop.md around lines 67 - 71, Update the documentation text that
currently states backend stdout/stderr are appended to
"%LOCALAPPDATA%\CodexMate\logs\desktop.log" so it documents both
"%LOCALAPPDATA%\CodexMate\logs\desktop.log" and
"%LOCALAPPDATA%\CodexMate\logs\startup.log" (the latter is used when console
logging is off), and change the nearby heading text "如需指定日志位置:" to "如需指定
desktop.log 位置:"; apply the same wording/path changes to the other occurrence
referenced (the block around lines 73-78) so both places correctly list
desktop.log and startup.log.


</details>

<!-- fingerprinting:phantom:poseidon:hawk -->

<!-- This is an auto-generated comment by CodeRabbit -->

Comment thread src-tauri/src/lib.rs
@awsl233777
Copy link
Copy Markdown
Collaborator Author

Update pushed: 21cc4d4 fix(desktop): elevate stale backend cleanup

What changed:

  • Reuse an already healthy 127.0.0.1:3737 backend before attempting stale-port cleanup.
  • When Windows detects a managed stale Codex Mate backend listener, the non-elevated desktop instance now relaunches itself with UAC/admin privileges instead of trying to half-clean the port.
  • The elevated instance performs the forced cleanup, then starts a fresh backend before the app window is shown.
  • Cleanup is restricted to verified managed codexmate/cli.js run listeners and LISTEN sockets; unrelated processes on port 3737 are skipped and surfaced as occupied-port guidance.
  • Updated the desktop diagnostics contract test for the admin-cleanup flow.

Validation run locally:

  • git diff --check
  • targeted desktop-diagnostics-contract loader: 5/5 passed

Not run locally:

  • Rust cargo fmt / cargo check because this environment does not have cargo/rustc; CI is now the compile gate.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

Update pushed: 4be79e9 fix(desktop): require admin via Windows manifest

Adjusted per the requested packaging-level UAC strategy:

  • Removed the internal runtime self-elevation / relaunch flow.
  • Added src-tauri/app.manifest with requestedExecutionLevel level="requireAdministrator" uiAccess="false".
  • Updated src-tauri/build.rs to inject that manifest through tauri_build::WindowsAttributes::app_manifest(...).
  • Startup now force-cleans verified managed Codex Mate backend listeners before spawning a fresh backend, instead of reusing an existing healthy backend.
  • If the port is still occupied after cleanup, the app surfaces occupied-port guidance instead of trying to relaunch itself.

Validation run locally:

  • git diff --check
  • targeted desktop-diagnostics-contract loader: 6/6 passed

Not run locally:

  • Rust cargo fmt / cargo check because this environment does not have cargo/rustc; GitHub Windows/macOS CI is the compile gate.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

Updated the desktop Windows verification path after the reported UAC behavior did not reproduce reliably from installed artifacts.

Changes in this round:

  • Keep extracting the final Windows app manifest from src-tauri/target/release/codexmate-desktop.exe with mt.exe and asserting requireAdministrator.
  • Copy the extracted manifest into the build output.
  • Upload the raw Windows app exe and extracted manifest alongside the installer bundles so the PR artifact can distinguish final exe behavior from installer/old-version replacement behavior.

Validated locally:

  • git diff --check
  • targeted desktop-diagnostics-contract.test.mjs contract tests: 7/7 passed

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== src-tauri/app.manifest ==="
cat src-tauri/app.manifest 2>/dev/null || echo "not found"

echo ""
echo "=== src-tauri/build.rs ==="
cat src-tauri/build.rs

echo ""
echo "=== .github/workflows/desktop-build.yml ==="
cat .github/workflows/desktop-build.yml

echo ""
echo "=== tests/unit/desktop-diagnostics-contract.test.mjs ==="
cat tests/unit/desktop-diagnostics-contract.test.mjs

echo ""
echo "=== src-tauri/src/lib.rs (key sections: port cleanup, startup_error, manifest) ==="
# Show full lib.rs
cat src-tauri/src/lib.rs

echo ""
echo "=== tests/unit/run.mjs ==="
cat tests/unit/run.mjs

Length of output: 34061



@awsl233777 Verified the latest changes. All items are confirmed:

# Item Status
1 app.manifest content requestedExecutionLevel level="requireAdministrator" uiAccess="false", assemblyIdentity name="ai.codexmate.desktop", Microsoft.Windows.Common-Controls all present
2 build.rs wiring WindowsAttributes::new().app_manifest(include_str!("app.manifest")) passed through tauri_build::try_build(attrs)
3 Workflow UAC verification step ✅ Finds built exe, locates mt.exe from Windows Kits 10 (recurse + descending sort for latest SDK), extracts manifest via -inputresource:$exe;#1``, asserts requireAdministrator regex, errors out if any step fails
4 Manifest copied to release dir Copy-Item $manifest ... codexmate-desktop.manifest.xml lands in src-tauri/target/release/ — matches the artifact upload glob
5 Artifact upload includes exe + manifest ✅ Upload path: block explicitly lists codexmate-desktop.exe and codexmate-desktop.manifest.xml alongside bundle/**
6 Contract test 5 (desktop build workflow verifies the final Windows exe UAC manifest) ✅ Asserts all key patterns: mt.exe, Copy-Item, codexmate-desktop.manifest.xml, raw exe upload path, and the requireAdministrator regex string
7 All 7 contract tests registered desktop-diagnostics-contract.test.mjs imported in run.mjs; covers diagnostics, stdio, port cleanup (no ShellExecuteW/runas), manifest requirement, workflow verification, occupied-port guidance, and NSIS reinstall flow

The Sort-Object FullName -Descending heuristic for finding mt.exe is a reasonable approach on GitHub-hosted Windows runners where the Windows 10 SDK is present — lexicographic ordering of full paths correlates correctly with SDK version numbers in the path. Everything looks solid. 🐇 ✨

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@awsl233777
Copy link
Copy Markdown
Collaborator Author

Update pushed: ba938f1 fix(desktop): bundle node runtime for packaged startup

What changed:

  • Stage the build machine's Node.js executable into dist/desktop/codexmate/node-runtime/ and record it in codexmate-desktop.json.
  • Make the release desktop backend launcher prefer the bundled Node runtime from Tauri resources instead of relying on node from the user's system PATH.
  • Keep CODEXMATE_NODE as an explicit diagnostic override.
  • Add regression tests for the staged Node runtime and launcher contract.
  • Update desktop docs to state that packaged desktop builds include Node and do not require users to preinstall Node.js.

Validation performed:

  • npm run test:unit -- --match desktop passed; the runner reported all 547 unit tests passed.
  • npm run desktop:stage produced dist/desktop/codexmate/codexmate-desktop.json with nodeRuntime: "node-runtime/node" on Linux and staged a bundled Node executable (~124 MB), increasing the staged runtime directory to ~131 MB.
  • Local Rust/Tauri check/build could not be run on this host because cargo is not installed.
  • GitHub desktop-build run passed on both Windows and macOS: https://github.com/SakuraByteCore/codexmate/actions/runs/26621899483
  • New Windows artifact size is ~67.7 MB and macOS artifact size is ~95.9 MB.
  • The new Windows MSI contains node-runtime, node.exe, and codexmate-desktop.json markers, confirming the installer payload includes the bundled runtime path.

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== tools/desktop/prepare-tauri-resources.js ==="
cat tools/desktop/prepare-tauri-resources.js

echo ""
echo "=== src-tauri/src/lib.rs (node launcher section) ==="
cat src-tauri/src/lib.rs

echo ""
echo "=== tests/unit/desktop-stage.test.mjs ==="
cat tests/unit/desktop-stage.test.mjs

echo ""
echo "=== tests/unit/desktop-diagnostics-contract.test.mjs ==="
cat tests/unit/desktop-diagnostics-contract.test.mjs

echo ""
echo "=== doc/desktop.md ==="
cat doc/desktop.md

echo ""
echo "=== tests/unit/run.mjs ==="
cat tests/unit/run.mjs

Length of output: 48779



@awsl233777 Verified commit ba938f1. All items are confirmed:

# Item Status
1 copyNodeRuntime() stages build-machine Node via process.execPath ✅ Copies to node-runtime/node(.exe), sets +x, records relative path in codexmate-desktop.json.nodeRuntime
2 validateStagedResources() checks bundled Node exists ✅ Asserts manifest.nodeRuntime is non-empty and the file resolves under the stage dir
3 find_node_runtime_path() prefers bundled over system PATH ✅ Priority: CODEXMATE_NODE env → resource_dir/codexmate/node-runtime/node(.exe)resource_dir/node-runtime/node(.exe) → debug-only PathBuf::from("node") fallback → release: startup_error(...)
4 No silent PATH fallback in release builds #[cfg(not(debug_assertions))] block calls startup_error(...) with a clear "bundled node-runtime/node is missing" message; contract test asserts doesNotMatch for unwrap_or_else(|_| "node".to_string())
5 CODEXMATE_NODE override retained ✅ First branch in find_node_runtime_path, checked before any resource lookup
6 desktop-stage.test.mjs updated node-runtime in required entries, manifest.nodeRuntime validated against /^node-runtime\/node(\.exe)?$/, file-exists check on the manifest path
7 Contract test registered ✅ Both desktop-stage.test.mjs and desktop-diagnostics-contract.test.mjs in run.mjs; contract test 3 covers all 8 staging/launcher patterns
8 Docs updated doc/desktop.md states packaged builds include bundled Node, no system Node required by users, CODEXMATE_NODE override documented
9 CI evidence ✅ Windows (~67.7 MB) and macOS (~95.9 MB) artifacts match expected size with embedded Node binary

One inherent architectural note (not a blocker): process.execPath bundles the Node binary of the build machine, so the installer's bundled node binary architecture matches the CI runner — x64 on windows-latest, arm64 on macos-latest (Apple Silicon). This means a macOS Intel user getting the GitHub Actions artifact would run into an arch mismatch. If universal macOS support (Intel + Apple Silicon) is needed later, a follow-up could download a fat/universal Node binary during staging instead of copying process.execPath. For now, CI-built artifacts are architecture-correct for typical targets.

Everything in scope is solid. 🐇 ✨

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
doc/desktop.md (1)

69-73: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

The past review comment about documenting both desktop.log and startup.log remains unresolved.

The previous review flagged that backend stdout/stderr are routed to startup.log when console logging is off, but the documentation still only mentions desktop.log at line 72. Line 69 should clarify that backend stdio goes to a different file (startup.log), and the heading at line 75 should specify it controls only desktop.log location. This mismatch could misdirect users during troubleshooting.

🤖 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 `@doc/desktop.md` around lines 69 - 73, Update the documentation to clearly
state two log files: explain that when the console/parent terminal is attached
backend stdout/stderr are inherited by the terminal and desktop writes local
file logs to desktop.log, but when console logging is disabled the backend
stdout/stderr are appended to a separate startup.log (not desktop.log); also
update the nearby heading that currently references desktop.log so it explicitly
says it controls only the desktop.log location and does not cover startup.log,
and mention both filenames (desktop.log and startup.log) where relevant.
🤖 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.

Duplicate comments:
In `@doc/desktop.md`:
- Around line 69-73: Update the documentation to clearly state two log files:
explain that when the console/parent terminal is attached backend stdout/stderr
are inherited by the terminal and desktop writes local file logs to desktop.log,
but when console logging is disabled the backend stdout/stderr are appended to a
separate startup.log (not desktop.log); also update the nearby heading that
currently references desktop.log so it explicitly says it controls only the
desktop.log location and does not cover startup.log, and mention both filenames
(desktop.log and startup.log) where relevant.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2b53d913-09e3-4606-98a8-451bc206eff6

📥 Commits

Reviewing files that changed from the base of the PR and between 64b24bc and ba938f1.

📒 Files selected for processing (5)
  • doc/desktop.md
  • src-tauri/src/lib.rs
  • tests/unit/desktop-diagnostics-contract.test.mjs
  • tests/unit/desktop-stage.test.mjs
  • tools/desktop/prepare-tauri-resources.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/unit/desktop-diagnostics-contract.test.mjs
  • tests/unit/desktop-stage.test.mjs
  • tools/desktop/prepare-tauri-resources.js
  • src-tauri/src/lib.rs
📜 Review details
🔇 Additional comments (4)
doc/desktop.md (4)

12-12: LGTM!


29-29: LGTM!


33-33: LGTM!


52-52: LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(analytics): export usage data as CSV or JSON

2 participants