Skip to content

Handle --version and --help before the GUI initializes state - #300

Merged
shingonoide merged 1 commit into
mainfrom
task-07d83c5c-0b80-4b40-a431-6b2a7f06317c
Nov 3, 2026
Merged

shingonoide merged 1 commit into
mainfrom
task-07d83c5c-0b80-4b40-a431-6b2a7f06317c

Conversation

@shingonoide

@shingonoide shingonoide commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

slashit-ui ignored its command-line arguments. main() always started the GUI and initialized application state, so --version and --help opened a window and wrote to the default user state directories.

This change parses arguments in main() before the Linux real-time priority guard and before any state (PTY state, application paths, storage writers) is created.

Behavior

  • --version and -V print the version and exit 0.
  • --help and -h print usage and exit 0.
  • Unknown arguments print usage to stderr and exit 2, without initializing state.
  • No-argument startup is unchanged.

Files

  • src-tauri/src/main.rs: argument parsing with clap (already a dependency), run before any state is created.
  • src-tauri/tests/cli_arguments.rs: runs the built binary for each flag with HOME, XDG_CONFIG_HOME, XDG_DATA_HOME, XDG_STATE_HOME and XDG_RUNTIME_DIR pointing at empty temporary directories. It does not set XDG_CACHE_HOME, so the child inherits the ambient cache location; this gap is tracked in Task 6dc27514. The test asserts exit codes, output, that the sandbox directories stay empty, and uses a 20-second deadline so a regression that opens the GUI fails instead of hanging.

Test evidence

  • cargo test -p slashit-ui --test cli_arguments: 3 passed.
  • cargo test -p slashit-ui --bin slashit-ui -- no_arguments version_flags help_flags unknown_arguments: 4 passed.
  • cargo clippy -p slashit-ui -p slashit -- -D warnings (the CI form): exit 0, no diagnostics.
  • Not claimed: cargo clippy --all-targets reports pre-existing diagnostics in src-tauri/src/commands/pr.rs and src-tauri/src/commands/pr/publication.rs. Neither file is changed here. Those diagnostics are a separate baseline and are not addressed by this PR.

Independent review

A read-only review of the exact commit 04b3078 found no blocking or major defects. Its minor notes are test gaps (argument combinations, non-UTF-8 input, stderr assertions for help and version) and are not blockers.

Known limitations

  • Windows release builds use windows_subsystem = "windows", so --help and --version print nothing in a normal console, and usage errors exit without visible output. This is a nonblocking compatibility item to resolve before a Windows release. No console-attachment API is introduced here.
  • Any unrecognized argument now exits with status 2 before the window opens. Historically, macOS Launch Services has passed a -psn_0_<number> argument to GUI applications launched from Finder. Current macOS behavior was not verified here: this change was not run on macOS, and the only available sources date from 2002 to 2007. If macOS still sends that argument, a Finder launch would fail silently with status 2. Release implication: verify a Finder or open launch of the bundled macOS app before a macOS release. A narrow correction (ignore only the exact -psn_ prefix) would need a new commit and a new Human Review approval.
  • Desktop acceptance on hosted CI is the authoritative check that the normal GUI startup still works under the WebDriver harness.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Walkthrough

The desktop binary now parses command-line arguments before startup. It accepts no options, exits successfully for help and version requests, and returns an error for unsupported options or positional arguments. Tests cover these outcomes and verify that the binary does not launch the GUI or write to the sandbox directories for these requests.

Change: Bug fix

Merge Risk: Low · up to 04b30

The change appears mergeable, with follow-up needed to close two gaps in the startup-side-effect tests.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: handling --version and --help before GUI state initialization.
Persistent State Integrity Passed The pull request adds argument parsing and tests only. In src-tauri/src/main.rs, parse_arguments either returns for a plain launch or exits through clap before slashit_ui_lib::run(), which is th…
Recoverable Resource Lifecycle Passed The pull request changes only CLI argument parsing and integration tests. It does not change worktree, task, migration, or cleanup lifecycle code. The existing GUI startup call remains unchanged after…
Platform Boundary Safety Passed The changed entrypoint uses portable Rust and clap APIs. The Linux-only realtime guard remains correctly gated with #[cfg(target_os = "linux")]. The new integration test uses portable tempfile, fi…
Description check Passed The description states the problem (slashit-ui ignored command-line arguments, so --version and --help opened the GUI and wrote state). It names the changed files (src-tauri/src/main.rs and src-tauri/…
Full details: Persistent State Integrity

Explanation

The pull request adds argument parsing and tests only. In src-tauri/src/main.rs, parse_arguments either returns for a plain launch or exits through clap before slashit_ui_lib::run(), which is the unchanged path that resolves paths and hydrates state. The changed code contains no persisted-state mutation, shared-memory publication, or read-modify-write logic. The integration tests use temporary directories only.

Full details: Recoverable Resource Lifecycle

Explanation

The pull request changes only CLI argument parsing and integration tests. It does not change worktree, task, migration, or cleanup lifecycle code. The existing GUI startup call remains unchanged after argument parsing, and the tests use temporary directories without clearing or destroying persisted resource references.

Full details: Platform Boundary Safety

Explanation

The changed entrypoint uses portable Rust and clap APIs. The Linux-only realtime guard remains correctly gated with #[cfg(target_os = "linux")]. The new integration test uses portable tempfile, filesystem, and process APIs; setting XDG environment variables does not require Unix APIs and does not create an unsupported-target code path. No changed code introduces an evident cross-platform compile failure.

Full details: Description check

Explanation

The description states the problem (slashit-ui ignored command-line arguments, so --version and --help opened the GUI and wrote state). It names the changed files (src-tauri/src/main.rs and src-tauri/tests/cli_arguments.rs), lists the expected behavior for each flag, and gives test evidence. It is directly related to the changeset, so the check passes.

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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: BarraDev/slashit
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: e6cda00
  • Layers: Repository YAML (base), Repository UI (inherited)
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 8cd3c9a4-7db5-4895-a185-02d6f3b0c628
Commits

Reviewing files that changed from the base of the PR and between e6cda00 and 04b3078.

Files selected for processing: 2
  • src-tauri/src/main.rs
  • src-tauri/tests/cli_arguments.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@shingonoide
shingonoide marked this pull request as draft November 3, 2026 07:19
Comment on lines +38 to +42
self.dir("config"),
self.dir("data"),
self.dir("state"),
self.dir("runtime"),
self.dir("home"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trivial · Maintainability & Code Quality · Quick win

Supported by static analysis

Script executed:

sed -n '1,180p' src-tauri/tests/cli_arguments.rs
sed -n '1,240p' src-tauri/src/config/paths.rs
rg -n 'XDG_CACHE_HOME|cache_dir|AppPaths::new' src-tauri/src

Repository: BarraDev/slashit

Length of output: 16108


Script executed:

sed -n '228,315p' src-tauri/src/config/paths.rs
sed -n '600,680p' src-tauri/src/config/paths.rs
rg -n -F --glob '*.rs' -e 'cache_dir()' -e 'cache_dir' src-tauri

Repository: BarraDev/slashit

Length of output: 7395


Redirect the cache directory into the sandbox.

When the parent sets XDG_CACHE_HOME, the child inherits that path. The test does not inspect it, although AppPaths resolves a separate cache directory. A future startup regression can therefore write cache state outside the sandbox without failing assert_untouched.

Add a sandbox cache directory, set XDG_CACHE_HOME, and include it in the existing untouched-directory checks.

Suggested fix
         self.dir("runtime"),
         self.dir("home"),
+        self.dir("cache"),
     ]
 }
@@
             .env("XDG_RUNTIME_DIR", &dirs[3])
             .env("HOME", &dirs[4])
+            .env("XDG_CACHE_HOME", &dirs[5])

Comment on lines +99 to +100
if file_type.is_dir() {
is_empty_tree(&entry.path())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trivial · Functional Correctness · Quick win

Supported by static analysis

Script executed:

sed -n '1,180p' src-tauri/tests/cli_arguments.rs
sed -n '1,240p' src-tauri/src/config/paths.rs
rg -n 'AppPaths::new|create_dir_all|create_dir' src-tauri/src

Repository: BarraDev/slashit

Length of output: 41001


Script executed:

sed -n '240,380p' src-tauri/src/config/paths.rs
printf '\n--- entrypoint references ---\n'
rg -n -F --glob '*.rs' -e 'fn main' -e 'AppPaths::new' -e 'parse_args' -e 'clap' src-tauri/src src-tauri/tests
printf '\n--- lib startup ---\n'
sed -n '1,190p' src-tauri/src/lib.rs
printf '\n--- binary entrypoint candidates ---\n'
rg --files src-tauri/src | rg '(^|/)(main|bin|lib)\.rs$'

Repository: BarraDev/slashit

Length of output: 15930


Script executed:

sed -n '1,130p' src-tauri/src/main.rs

Repository: BarraDev/slashit

Length of output: 3610


Reject empty directories in is_empty_tree.

The current helper treats an empty application directory as unchanged. A startup-avoidance test can therefore pass after startup modifies the sandbox. Make the untouched-sandbox check fail when any entry exists, including an empty directory.

Suggested fix
--- "a/src-tauri/tests/cli_arguments.rs"
+++ "b/src-tauri/tests/cli_arguments.rs"
@@ -91,18 +91,12 @@
 }
 
 /// True when the directory holds no entries at any depth.
 fn is_empty_tree(path: &Path) -> bool {
-    let entries = std::fs::read_dir(path).expect("read sandbox directory");
-    entries.into_iter().all(|entry| {
-        let entry = entry.expect("read sandbox entry");
-        let file_type = entry.file_type().expect("stat sandbox entry");
-        if file_type.is_dir() {
-            is_empty_tree(&entry.path())
-        } else {
-            false
-        }
-    })
+    std::fs::read_dir(path)
+        .expect("read sandbox directory")
+        .next()
+        .is_none()
 }
 
 #[test]
 fn version_prints_the_version_and_exits_zero_without_state() {

@shingonoide
shingonoide marked this pull request as ready for review November 3, 2026 08:00
@shingonoide
shingonoide merged commit 9f3d91a into main Nov 3, 2026
16 checks passed
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.

1 participant