Add cli2 plugin wrapping official cli plugin#1757
Conversation
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new tauri-plugin-cli2 crate (Rust + JS), registers it in workspace manifests and Tauri config, exposes CLI install/uninstall/status commands and a CliPluginExt API (symlink-based install), provides a CLI handler with a "hello" subcommand that opens https://hyprnote.com, and adds docs and capability permission. Changes
Sequence Diagram(s)sequenceDiagram
participant Build as build.rs
participant App as apps/desktop (lib.rs)
participant Plugin as tauri-plugin-cli2
participant OS as Operating System
Note over Build,Plugin: build-time registration
Build->>Plugin: register COMMANDS (install/uninstall/check)
Note over App,Plugin: app startup & plugin init
App->>Plugin: init() registers invoke handlers & setup hook
App->>Plugin: setup() -> tauri_plugin_cli::get_matches()
alt help/version present
App->>OS: exit(0)
else subcommand present
Plugin->>Plugin: handler::entrypoint(matches)
alt subcommand == "hello"
Plugin->>OS: open URL (https://hyprnote.com)
Plugin->>OS: exit(0)
else unknown subcommand
Plugin->>App: log warning
Plugin->>OS: exit(1)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
Comment |
Prevent runtime panic when initializing Tauri CLI plugin by adding a default "cli" object to the plugins section of apps/desktop/src-tauri/tauri.conf.json. The Tauri runtime was calling unwrap() on a deserialized Plugin configuration and failed when "plugins.cli" was null/missing, causing a panic. Providing a basic structure with description and empty args ensures deserialization succeeds and the application can start without crashing. v cli args support cli args support v cli args support
Move CLI handling logic out of the main app into a new tauri-plugin-cli-handler plugin for better separation and maintainability. Add plugin crate (Cargo.toml, build.rs), TypeScript bindings and package.json, command implementation (ping), error type, extension trait for handling CLI matches, plugin library wiring (init, specta export), and test/tsconfig to generate bindings and build metadata. Rename CLI plugin to cli2 and wire into workspace Rename the existing tauri plugin from cli/tauri-plugin-cli-handler to tauri-plugin-cli2 (package names, links, specta plugin name and JS package name) and update build/test scripts and dependencies. This also registers the new cli2 plugin in Cargo.toml and Tauri app manifests, adds the cli2 capability to desktop capabilities, and enables the plugin at app initialization. Several small code cleanups and formatting changes to commands, build.rs, and error handling were made to match the renamed plugin. v plugin cli v v
yujonglee
force-pushed
the
add-default-cli-plugin-config
branch
from
11dcfca to
4112e36
Compare
yujonglee
changed the title
yujonglee
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
plugins/cli2/src/commands.rs (3)
1-6: Consider removingasynckeyword.The function doesn't perform any async operations or
.awaitcalls. Sinceinstall_cli_to_path()is synchronous (based on the trait definition in ext.rs), theasynckeyword adds unnecessary overhead.Apply this diff:
-#[tauri::command] -#[specta::specta] -pub(crate) async fn install_cli<R: tauri::Runtime>(app: tauri::AppHandle<R>) -> Result<(), String> { - use crate::CliPluginExt; - app.install_cli_to_path().map_err(|e| e.to_string()) -} +#[tauri::command] +#[specta::specta] +pub(crate) fn install_cli<R: tauri::Runtime>(app: tauri::AppHandle<R>) -> Result<(), String> { + use crate::CliPluginExt; + app.install_cli_to_path().map_err(|e| e.to_string()) +}
8-15: Consider removingasynckeyword.Same as
install_cli, this function doesn't perform any async operations. Theasynckeyword is unnecessary.Apply this diff:
-#[tauri::command] -#[specta::specta] -pub(crate) async fn uninstall_cli<R: tauri::Runtime>( - app: tauri::AppHandle<R>, -) -> Result<(), String> { - use crate::CliPluginExt; - app.uninstall_cli_from_path().map_err(|e| e.to_string()) -} +#[tauri::command] +#[specta::specta] +pub(crate) fn uninstall_cli<R: tauri::Runtime>( + app: tauri::AppHandle<R>, +) -> Result<(), String> { + use crate::CliPluginExt; + app.uninstall_cli_from_path().map_err(|e| e.to_string()) +}
17-24: Consider removingasynckeyword.Same as the other commands, this function doesn't perform any async operations. The
asynckeyword is unnecessary.Apply this diff:
-#[tauri::command] -#[specta::specta] -pub(crate) async fn check_cli_status<R: tauri::Runtime>( - app: tauri::AppHandle<R>, -) -> Result<crate::CliStatus, String> { - use crate::CliPluginExt; - app.check_cli_status().map_err(|e| e.to_string()) -} +#[tauri::command] +#[specta::specta] +pub(crate) fn check_cli_status<R: tauri::Runtime>( + app: tauri::AppHandle<R>, +) -> Result<crate::CliStatus, String> { + use crate::CliPluginExt; + app.check_cli_status().map_err(|e| e.to_string()) +}plugins/cli2/src/lib.rs (1)
26-32: Replaceprintln!with proper logging and clarify setup logic.Two observations:
Line 29 uses
println!for debugging. Consider using thetracingorlogcrate for structured logging, especially since other plugins in the codebase likely use proper logging.The setup hook reads CLI matches but only prints them. Should this be calling
handle_cli_matches()from theCliPluginExttrait (which handles help/version and exits on error)?Apply this diff to use proper logging:
.setup(|app, _api| { use tauri_plugin_cli::CliExt; let matches = app.cli().matches(); - println!("matches: {matches:?}"); + tracing::debug!("CLI matches: {matches:?}"); Ok(()) })If the intent is to handle CLI matches at startup, consider:
.setup(|app, _api| { - use tauri_plugin_cli::CliExt; - let matches = app.cli().matches(); - tracing::debug!("CLI matches: {matches:?}"); - - Ok(()) + use crate::CliPluginExt; + app.handle_cli_matches() })plugins/cli2/src/ext.rs (2)
43-45: Simplify by removing the exists check.The check-then-remove pattern is unnecessary. Simply attempt to remove the file and ignore
NotFounderrors:- if symlink_path.exists() { - std::fs::remove_file(&symlink_path)?; - } + // Remove existing symlink if present (ignore if not found) + match std::fs::remove_file(&symlink_path) { + Ok(()) | Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}, + Err(e) => return Err(e.into()), + }This eliminates the TOCTOU gap and simplifies the code.
52-60: Apply the same simplification as ininstall_cli_to_path.The same check-then-remove pattern appears here. Apply the same refactor for consistency:
+ #[cfg(unix)] fn uninstall_cli_from_path(&self) -> Result<(), crate::Error> { let symlink_path = self.get_cli_symlink_path(); - if symlink_path.exists() { - std::fs::remove_file(&symlink_path)?; - } + match std::fs::remove_file(&symlink_path) { + Ok(()) | Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}, + Err(e) => return Err(e.into()), + } Ok(()) } + + #[cfg(not(unix))] + fn uninstall_cli_from_path(&self) -> Result<(), crate::Error> { + Err(crate::Error::Io(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "CLI installation is only supported on Unix platforms" + ))) + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
.cursor/commands/manage-cli.mdis excluded by!**/.cursor/**Cargo.lockis excluded by!**/*.lockplugins/cli2/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/cli2/permissions/autogenerated/commands/check-cli-status.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/autogenerated/commands/install-cli.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/autogenerated/commands/ping.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/autogenerated/commands/uninstall-cli.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/cli2/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
Cargo.toml(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src-tauri/src/lib.rs(1 hunks)apps/desktop/src-tauri/tauri.conf.json(1 hunks)apps/web/content/docs/cli.mdx(1 hunks)plugins/cli2/.gitignore(1 hunks)plugins/cli2/Cargo.toml(1 hunks)plugins/cli2/build.rs(1 hunks)plugins/cli2/js/index.ts(1 hunks)plugins/cli2/package.json(1 hunks)plugins/cli2/src/commands.rs(1 hunks)plugins/cli2/src/error.rs(1 hunks)plugins/cli2/src/ext.rs(1 hunks)plugins/cli2/src/lib.rs(1 hunks)plugins/cli2/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- plugins/cli2/.gitignore
- apps/web/content/docs/cli.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/tauri.conf.json
🧰 Additional context used
🧬 Code graph analysis (5)
plugins/cli2/src/lib.rs (3)
apps/desktop/src-tauri/src/lib.rs (3)
make_specta_builder(145-153)make_specta_builder(161-161)export_types(160-170)plugins/cli2/src/commands.rs (3)
install_cli(3-6)uninstall_cli(10-15)check_cli_status(19-24)plugins/cli2/src/ext.rs (2)
check_cli_status(9-9)check_cli_status(62-80)
apps/desktop/src-tauri/src/lib.rs (1)
plugins/cli2/src/lib.rs (1)
init(21-34)
plugins/cli2/src/commands.rs (1)
plugins/cli2/src/ext.rs (2)
check_cli_status(9-9)check_cli_status(62-80)
plugins/cli2/build.rs (1)
apps/desktop/src-tauri/src/lib.rs (1)
main(11-143)
plugins/cli2/src/ext.rs (1)
plugins/cli2/src/commands.rs (1)
check_cli_status(19-24)
⏰ 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). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (14)
plugins/cli2/js/index.ts (1)
1-1: LGTM!Standard barrel export pattern for exposing generated TypeScript bindings from the CLI plugin.
plugins/cli2/tsconfig.json (1)
1-5: LGTM!Standard TypeScript configuration for the CLI plugin, correctly extending the base config and including the plugin's JS files.
apps/desktop/package.json (1)
33-33: LGTM!Correctly adds the CLI plugin as a workspace dependency, consistent with the pattern used for other plugins in this project.
apps/desktop/src-tauri/src/lib.rs (1)
48-48: LGTM!Correctly initializes the CLI2 plugin in the Tauri plugin chain. The placement after
tauri_plugin_cliis appropriate.apps/desktop/src-tauri/Cargo.toml (1)
27-27: LGTM!Correctly adds the CLI2 plugin as a workspace dependency, consistent with the pattern used for other Tauri plugins in the desktop app.
Cargo.toml (1)
114-114: LGTM!Correctly defines the workspace dependency for the CLI2 plugin with the appropriate path. The placement maintains alphabetical ordering among plugin dependencies.
apps/desktop/src-tauri/capabilities/default.json (1)
78-78: LGTM!Correctly adds the default permission for the CLI2 plugin, enabling the frontend to invoke the plugin's commands.
plugins/cli2/Cargo.toml (1)
10-25: LGTM! Dependencies are correctly configured.The dependency configuration properly uses workspace dependencies and includes all necessary crates for a Tauri plugin with specta type generation support.
plugins/cli2/src/error.rs (1)
1-18: LGTM! Clean error handling implementation.The error module follows best practices:
- Uses
thiserrorfor ergonomic error derivation- Implements
Serializeto enable IPC error transmission- Provides a convenient
Result<T>alias- The single
Iovariant is sufficient for the current symlink operationsplugins/cli2/src/lib.rs (2)
10-19: LGTM! Specta builder correctly configured.The builder properly registers the three CLI commands with appropriate error handling mode for Result types.
40-51: LGTM! Type export test follows established patterns.The test correctly exports TypeScript bindings with appropriate formatting and configuration, consistent with the main app's approach.
plugins/cli2/src/ext.rs (2)
83-89: LGTM! Well-defined status structure.The
CliStatusstruct is properly configured with appropriate derives for JSON serialization and TypeScript type generation.
62-81: Hyprnote is currently macOS-only; Windows support is planned for Q1 2026.The concerns about cross-platform symlink behavior are premature. Windows and Linux versions are planned for Q1 2026, meaning the current codebase targets macOS exclusively. On macOS,
std::fs::read_linkworks without privilege restrictions. The code is not claiming cross-platform compatibility today and does not require platform guards for the current implementation.When Windows support is added in the future, platform-specific considerations may become relevant then, but that is outside the scope of the current changes.
Likely an incorrect or invalid review comment.
plugins/cli2/package.json (1)
1-11: Package configuration verified as correct.@tauri-apps/api v2.9.0 is the latest published version, and there are no direct vulnerabilities reported for this version. The package.json follows standard conventions with a proper TypeScript entry point and appropriate dependency version.
Implement a new CLI handler in plugins/cli2/src/handler.rs to process parsed Matches from tauri_plugin_cli. The handler exits immediately for --help and --version, handles a "hello" subcommand by attempting to open a URL and logging errors, and logs+exits with failure for unknown subcommands. This establishes basic CLI behavior required by the plan and marks the first to-do as in_progress by providing the implemented functionality. Use Tauri AppHandle in CLI entrypoint and open URLs via OpenerExt Accept an AppHandle in the CLI entrypoint and use the tauri-plugin-opener extension to open URLs. The CLI handler previously used open::that and returned a bool; this change makes the function accept &AppHandle<R>, imports OpenerExt, and calls app.opener().open_url(...). This enables using the plugin's opener API and integrates the CLI with the Tauri app context. c c
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugins/cli2/src/ext.rs (1)
13-29: Library code should not callprocess::exit.This issue was previously flagged: calling
std::process::exitin library code prevents proper cleanup, breaks Drop implementations, and makes testing difficult. The calling application should control shutdown behavior by handling returned errors.
🧹 Nitpick comments (5)
plugins/cli2/build.rs (1)
1-12: Consider handling errors fromtauri_plugin::Builder::build()Assuming
build()returns aResult, ignoring it can hide failures in plugin metadata generation during builds. It’s safer to assert on success:- tauri_plugin::Builder::new(COMMANDS).build(); + tauri_plugin::Builder::new(COMMANDS) + .build() + .expect("failed to generate Tauri plugin metadata");Please confirm the current
tauri_plugin::Builder::build()return type/behavior against the version used in this repo’sCargo.tomlbefore changing this.plugins/cli2/src/error.rs (1)
15-21: Minor: usecollect_strto avoid an extra allocation inSerializeThe custom
Serializeimpl is fine functionally, but you can avoid creating an intermediateStringand use the standard serde pattern:impl Serialize for Error { fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> where S: serde::Serializer, { - serializer.serialize_str(self.to_string().as_ref()) + serializer.collect_str(self) } }Please double-check serde’s current
Serializer::collect_strAPI in the version you depend on to ensure this signature still matches.plugins/cli2/src/ext.rs (3)
40-46: Prefer chainedjoin()calls for cross-platform path construction.Line 43 embeds backslashes in the string passed to
join(), which is not idiomatic and may not behave correctly on all Windows configurations.Apply this diff to use chained
join()calls:#[cfg(windows)] { if let Some(home) = std::env::var_os("USERPROFILE") { - return PathBuf::from(home).join(".local\\bin\\hyprnote.exe"); + return PathBuf::from(home).join(".local").join("bin").join("hyprnote.exe"); } PathBuf::from("C:\\Program Files\\hyprnote\\hyprnote.exe") }
31-52: Document that fallback paths may require elevated privileges.The fallback paths
/usr/local/bin(line 37) andC:\Program Files(line 45) typically require administrator/sudo privileges to write. Users without elevated access will encounter cryptic permission errors during installation.Consider documenting this requirement in the function docs or providing clearer error messages when permission is denied.
77-84: Document Windows symlink privilege requirement.Creating symlinks on Windows requires the
SeCreateSymbolicLinkPrivilege, which typically requires administrator privileges or Developer Mode enabled (Windows 10+). Users may encounter cryptic "permission denied" errors without this context.Consider:
- Adding a doc comment explaining the Windows requirement
- Providing a more informative error message when
symlink_filefails with permission errors- Documenting the Developer Mode alternative in user-facing installation instructions
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockplugins/cli2/permissions/autogenerated/commands/check_cli_status.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/autogenerated/commands/install_cli.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/autogenerated/commands/uninstall_cli.tomlis excluded by!plugins/**/permissions/**plugins/cli2/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/cli2/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**
📒 Files selected for processing (7)
apps/desktop/src-tauri/tauri.conf.json(1 hunks)plugins/cli2/Cargo.toml(1 hunks)plugins/cli2/build.rs(1 hunks)plugins/cli2/src/error.rs(1 hunks)plugins/cli2/src/ext.rs(1 hunks)plugins/cli2/src/handler.rs(1 hunks)plugins/cli2/src/lib.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/cli2/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (3)
plugins/cli2/build.rs (1)
apps/desktop/src-tauri/src/lib.rs (1)
main(11-143)
plugins/cli2/src/lib.rs (4)
apps/desktop/src-tauri/src/lib.rs (3)
make_specta_builder(145-153)make_specta_builder(161-161)export_types(160-170)plugins/cli2/src/commands.rs (3)
install_cli(3-6)uninstall_cli(10-15)check_cli_status(19-24)plugins/cli2/src/ext.rs (2)
check_cli_status(9-9)check_cli_status(109-127)plugins/cli2/src/handler.rs (1)
entrypoint(5-23)
plugins/cli2/src/ext.rs (1)
plugins/cli2/src/commands.rs (1)
check_cli_status(19-24)
⏰ 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). (6)
- GitHub Check: build (macos, aarch64-apple-darwin, macos-14)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (5)
apps/desktop/src-tauri/tauri.conf.json (1)
55-63: CLI “hello” subcommand wiring looks consistentThe
hellosubcommand name/description here aligns with the routing inplugins/cli2/src/handler.rs, so the config and handler are in sync.plugins/cli2/src/lib.rs (1)
6-9: Clean public API surface for the CLI2 pluginRe‑exporting
Error,Result, and theexttraits/types plus a singleinit()entrypoint gives a tidy, minimal surface for consumers and keeps internal modules nicely encapsulated.plugins/cli2/src/ext.rs (3)
54-56: LGTM!Clean implementation using the standard library's
current_exe()with proper error propagation.
109-128: LGTM!The status check appropriately handles the case where the symlink exists but
read_linkfails by settingtarget_pathtoNone, which accurately represents the state.
130-136: LGTM!The struct is well-designed with appropriate derives for Tauri integration and follows JavaScript naming conventions with
camelCase.
1 participant
No description provided.