Skip to content

Conversation

@michaelneale
Copy link
Collaborator

image

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a CLI command for launching the Staged application directly from the terminal, enabling users to quickly open repositories without navigating through the GUI. The feature includes both an installation mechanism through the application menu and automatic installation via the install script.

Changes:

  • Added bin/staged CLI launcher script that finds and launches the app with a specified directory
  • Implemented backend support for CLI argument parsing and privilege-escalated CLI installation
  • Integrated CLI installation into the application menu with user feedback dialogs

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
bin/staged New bash script that resolves paths and launches the Staged.app bundle with directory arguments
src-tauri/src/lib.rs Added get_initial_path and install_cli commands; updated menu system to include CLI installation option
src/lib/services/window.ts Added TypeScript wrappers for the new CLI-related Tauri commands
src/lib/stores/repoState.svelte.ts Modified initialization to check for CLI arguments before falling back to current directory
src/App.svelte Added menu event handler and UI feedback for CLI installation feature
install.sh Added install_cli function to automatically install the CLI during setup
README.md Updated documentation with CLI usage examples and installation notes
.gitignore Added entries for development artifacts (unrelated to main feature)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

let path = std::path::Path::new(arg);
if let Ok(canonical) = path.canonicalize() {
if canonical.is_dir() {
return canonical.to_str().map(|s| s.to_string());
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The get_initial_path function uses canonical.to_str() which returns None if the path contains invalid UTF-8 sequences. This will silently fail to return valid paths on systems with non-UTF-8 filesystem paths. Consider using to_string_lossy() or handling the error explicitly to provide better feedback to users.

Suggested change
return canonical.to_str().map(|s| s.to_string());
return Some(canonical.to_string_lossy().into_owned());

Copilot uses AI. Check for mistakes.
Comment on lines 836 to 837
// Clean up temp file
let _ = std::fs::remove_file(&temp_path);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The temp file cleanup happens immediately after launching osascript (line 837), but if osascript hasn't finished reading the temp file yet (it may buffer or delay), this could cause the installation to fail. While unlikely in practice due to osascript's synchronous nature, consider using output.wait() or keeping the temp file until after verifying success to be more defensive.

Copilot uses AI. Check for mistakes.
Comment on lines 823 to 828
let script = format!(
r#"do shell script "cp '{}' '{}' && chmod +x '{}'" with administrator privileges"#,
temp_path.display(),
install_path,
install_path
);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The install_cli function uses shell command injection through AppleScript's "do shell script". While temp_path.display() and install_path are controlled in this case, if this pattern is copied or modified in the future, it could introduce security risks. Consider using a safer method for privilege escalation, such as the sudo-prompt npm package or a dedicated helper binary that uses macOS's Authorization Services API.

Copilot uses AI. Check for mistakes.
Comment on lines +285 to +296
async function handleMenuInstallCli() {
try {
const path = await installCli();
alert(
`CLI installed successfully!\n\nYou can now run:\n staged # open current directory\n staged /path # open specific directory\n\nInstalled to: ${path}`
);
} catch (e) {
const error = e as Error;
alert(
`Failed to install CLI:\n\n${error.message || error}\n\nYou may need to run manually:\n sudo cp /path/to/staged/bin/staged /usr/local/bin/`
);
}
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The alert messages use backticks for code examples in plain text alerts. While functional, this might not render well in native alert dialogs. Consider creating a custom modal component (similar to other modals in the codebase like CommitModal, SettingsModal) for the CLI installation feedback to provide better formatting and user experience.

Copilot uses AI. Check for mistakes.
Comment on lines 104 to 106
try {
const canonicalPath = await getRepoRoot('.');
const canonicalPath = await getRepoRoot(initialPath);

Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

When the app is launched with a CLI path that's not a git repository, the try-catch block will swallow the error and return null, but the user gets no feedback about why the path wasn't opened. Since this is a new CLI feature, consider adding error handling to detect when a CLI path was provided but couldn't be opened as a git repo, and display an error message to inform the user.

Copilot uses AI. Check for mistakes.
print_success "CLI installed to $INSTALL_PATH"
else
print_warning "Failed to install CLI (you can manually copy bin/staged to your PATH)"
fi
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The install_cli function in the install script only runs on macOS (line 168), but there's no message or explanation for Linux users about how to manually install the CLI. Consider adding an informational message for non-macOS systems similar to how install_to_system handles Linux (lines 159-163).

Suggested change
fi
fi
elif [ "$OS" = "linux" ]; then
print_info "Linux CLI installation not yet implemented"
print_info "You can manually copy bin/staged to a directory in your PATH (e.g., ~/.local/bin or /usr/local/bin)"

Copilot uses AI. Check for mistakes.
Comment on lines +788 to +806
fn get_initial_path() -> Option<String> {
let args: Vec<String> = std::env::args().collect();

// Skip the binary name, look for a path argument (not starting with -)
for arg in args.iter().skip(1) {
if arg.starts_with('-') {
continue;
}

// Try to canonicalize the path
let path = std::path::Path::new(arg);
if let Ok(canonical) = path.canonicalize() {
if canonical.is_dir() {
return canonical.to_str().map(|s| s.to_string());
}
}
}
None
}
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The new get_initial_path function lacks test coverage. Given that the codebase has test coverage for other modules (git/, ai/, review/, etc.), consider adding unit tests to verify the path argument parsing logic, especially edge cases like non-existent paths, paths with special characters, multiple arguments, and flag handling.

Copilot uses AI. Check for mistakes.
Comment on lines 811 to 859
fn install_cli() -> Result<String, String> {
let cli_script = include_str!("../../bin/staged");
let install_path = "/usr/local/bin/staged";

// Write script to a temp file first
let temp_path = std::env::temp_dir().join("staged-cli-install");
std::fs::write(&temp_path, cli_script)
.map_err(|e| format!("Failed to write temp file: {}", e))?;

// Use osascript to run sudo with a GUI prompt (macOS)
#[cfg(target_os = "macos")]
{
let script = format!(
r#"do shell script "cp '{}' '{}' && chmod +x '{}'" with administrator privileges"#,
temp_path.display(),
install_path,
install_path
);

let output = std::process::Command::new("osascript")
.arg("-e")
.arg(&script)
.output()
.map_err(|e| format!("Failed to run installer: {}", e))?;

// Clean up temp file
let _ = std::fs::remove_file(&temp_path);

if output.status.success() {
Ok(install_path.to_string())
} else {
let stderr = String::from_utf8_lossy(&output.stderr);
if stderr.contains("User canceled") || stderr.contains("(-128)") {
Err("Installation cancelled".to_string())
} else {
Err(format!("Installation failed: {}", stderr))
}
}
}

#[cfg(not(target_os = "macos"))]
{
let _ = std::fs::remove_file(&temp_path);
Err(
"CLI installation is only supported on macOS. Copy bin/staged to your PATH manually."
.to_string(),
)
}
}
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The install_cli function lacks test coverage. Given that the codebase has test coverage for other modules, consider adding unit tests to verify the error handling paths (user cancellation, permission errors, etc.), or at least integration tests to ensure the installation process works correctly on macOS.

Copilot uses AI. Check for mistakes.
Comment on lines +788 to +806
fn get_initial_path() -> Option<String> {
let args: Vec<String> = std::env::args().collect();

// Skip the binary name, look for a path argument (not starting with -)
for arg in args.iter().skip(1) {
if arg.starts_with('-') {
continue;
}

// Try to canonicalize the path
let path = std::path::Path::new(arg);
if let Ok(canonical) = path.canonicalize() {
if canonical.is_dir() {
return canonical.to_str().map(|s| s.to_string());
}
}
}
None
}
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The get_initial_path function only processes the first non-flag argument. If a user passes multiple path arguments (e.g., staged /path1 /path2), only the first one will be used and the rest will be silently ignored. Consider either documenting this behavior clearly in the CLI help or adding validation to warn/error if multiple paths are provided.

Copilot uses AI. Check for mistakes.
} catch (e) {
const error = e as Error;
alert(
`Failed to install CLI:\n\n${error.message || error}\n\nYou may need to run manually:\n sudo cp /path/to/staged/bin/staged /usr/local/bin/`
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The error message suggests running sudo cp /path/to/staged/bin/staged /usr/local/bin/ as a fallback, but this requires the user to know where the repository is located. Consider providing a more specific instruction or including the actual binary location in the error message if available.

Suggested change
`Failed to install CLI:\n\n${error.message || error}\n\nYou may need to run manually:\n sudo cp /path/to/staged/bin/staged /usr/local/bin/`
`Failed to install CLI:\n\n${error.message || error}\n\nYou may need to manually copy the 'staged' binary into a directory on your PATH (for example, /usr/local/bin).\n\nPlease refer to the installation documentation for the exact location of the installed binary on your system.`

Copilot uses AI. Check for mistakes.
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.

2 participants