-
Notifications
You must be signed in to change notification settings - Fork 61
feat: implement shell integration for automatic cx fix error capture #744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,54 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/bin/bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Copyright (c) 2026 AI Venture Holdings LLC | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Licensed under the Business Source License 1.1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # CX Terminal Shell Integration for Bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Captures stderr of failed commands for 'cx fix' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Only run in interactive shells | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ $- != *i* ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| __cx_err_capture() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local exit_code=$? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ $exit_code -ne 0 ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -s "$HOME/.cx/stderr.tmp" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Get the last command from history | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local last_cmd=$(history 1 | sed 's/^[ ]*[0-9]*[ ]*//') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Don't capture if it was 'cx fix' or similar | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ "$last_cmd" =~ ^cx\ fix ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+22
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Don't capture if it was 'cx fix' or similar | |
| if [[ "$last_cmd" =~ ^cx\ fix ]]; then | |
| # Don't capture if it was 'cx fix' or common prefixed variants (e.g. sudo cx fix) | |
| if [[ "$last_cmd" =~ ^([[:space:]]*(sudo|command|env|nice|nohup)[[:space:]]+)*cx[[:space:]]+fix ]]; then |
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition: The stderr.tmp and last_error files are accessed without any locking mechanism. If multiple shell sessions are running concurrently (e.g., in tmux/screen), they will all write to the same files, causing corrupted captures or lost errors. Consider using session-specific files (e.g., stderr.tmp.$$ or using $PPID) or implement file locking.
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PROMPT_COMMAND modification uses string matching to check if __cx_err_capture is already present, but this could have false positives if the user has a variable or function name containing this substring. Consider using a more robust check or setting a guard variable to prevent double-initialization.
| if [[ ! "$PROMPT_COMMAND" =~ __cx_err_capture ]]; then | |
| if [[ -z "$PROMPT_COMMAND" ]]; then | |
| PROMPT_COMMAND="__cx_err_capture" | |
| else | |
| PROMPT_COMMAND="__cx_err_capture; $PROMPT_COMMAND" | |
| if [[ -z "${__CX_ERR_CAPTURE_INSTALLED:-}" ]]; then | |
| __CX_ERR_CAPTURE_INSTALLED=1 | |
| if [[ -z "${PROMPT_COMMAND:-}" ]]; then | |
| PROMPT_COMMAND="__cx_err_capture" | |
| else | |
| case ";${PROMPT_COMMAND};" in | |
| *";__cx_err_capture;"*) | |
| # Already present as a standalone command; do nothing | |
| ;; | |
| *) | |
| PROMPT_COMMAND="__cx_err_capture; ${PROMPT_COMMAND}" | |
| ;; | |
| esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create ~/.cx with restricted permissions to protect potentially sensitive error output.
Command stderr can contain credentials, tokens, or PII in error messages. The directory and its files should not be world-readable.
🛡️ Proposed fix
-mkdir -p "$HOME/.cx"
+mkdir -p "$HOME/.cx"
+chmod 700 "$HOME/.cx"Alternatively, combine into one step (Linux mkdir -m):
-mkdir -p "$HOME/.cx"
+mkdir -p -m 700 "$HOME/.cx"📝 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.
| # Ensure .cx directory exists | |
| mkdir -p "$HOME/.cx" | |
| # Ensure .cx directory exists | |
| mkdir -p "$HOME/.cx" | |
| chmod 700 "$HOME/.cx" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/shell/bash_integration.sh` around lines 48 - 49, The current creation
of the config directory using mkdir -p "$HOME/.cx" leaves default permissions
which may be world-readable; change the creation to set restrictive permissions
(700) and ensure existing directories are corrected: use a creation command that
sets mode (e.g., mkdir -m 0700 for the "$HOME/.cx" directory) or call chmod 0700
after mkdir -p "$HOME/.cx", so any files or error output stored there are not
world-readable; update the code paths that call mkdir -p "$HOME/.cx" accordingly
and ensure permission correction runs unconditionally for existing directories.
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mkdir -p command at line 49 will succeed even if $HOME is not set or empty, creating directories in unexpected locations. While setup_shell checks for empty HOME in Rust, the shell script itself should also validate $HOME before creating directories to prevent issues if the script is sourced manually without running setup first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two issues with exec 2> >(tee ...): async race and missing idempotency guard.
- Race condition: the tee process runs asynchronously. When PROMPT_COMMAND fires, tee may not have finished flushing all bytes to
stderr.tmp, causing[ -s "$HOME/.cx/stderr.tmp" ]to see an empty or truncated file. - Fd chain on re-source: there is no guard around line 54. If this script is sourced a second time (e.g.,
source ~/.bashrcmid-session), a new tee is chained through the previous one, producing double-writes tostderr.tmpand leaving orphaned background processes.
🔧 Proposed fix
+# Guard against double-hooking stderr
+if [[ -z "$__CX_STDERR_HOOKED" ]]; then
+ __CX_STDERR_HOOKED=1
exec 2> >(tee "$HOME/.cx/stderr.tmp" >&2)
+fiFor the flush race, consider writing the PID of the tee process and briefly waiting on it before reading, or using a named pipe with blocking reads. A pragmatic short-term option that adds minimal latency:
__cx_err_capture() {
local exit_code=$?
+ # Give async tee a moment to flush before reading
+ sleep 0.05
if [ $exit_code -ne 0 ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/shell/bash_integration.sh` around lines 51 - 54, The current global
stderr redirection using "exec 2> >(tee \"$HOME/.cx/stderr.tmp\" >&2)" creates
an async flush race and can chain multiple tee processes when the script is
re-sourced; modify the stderr capture logic around the exec and the error
handler __cx_err_capture so it is idempotent and waits briefly for the async tee
to flush before checking stderr.tmp: add an idempotency guard (e.g., a sentinel
variable or check to avoid rebinding exec/rehooking PROMPT_COMMAND), record the
tee PID when you start the process so you can avoid starting another or kill the
orphan on re-source, and before reading "$HOME/.cx/stderr.tmp" (inside
__cx_err_capture or PROMPT_COMMAND) wait briefly or wait on the tee PID to
ensure it has flushed (a short sleep like 0.05s or a wait on the saved PID);
ensure the logic references the same stderr.tmp file name and properly
cleans/rotates the PID sentinel to prevent multiple chained tee processes.
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stderr.tmp file is created without checking available disk space, and repeated writes could fill up the disk if a command produces large stderr output continuously. The file is cleared on each command, but during long-running commands with streaming stderr, it could grow large. Consider adding a size limit or using tail -c to capture only the last N bytes of stderr.
| # We redirect stderr (2) through tee which also writes to stderr.tmp | |
| exec 2> >(tee "$HOME/.cx/stderr.tmp" >&2) | |
| # We redirect stderr (2) through tee which also writes a bounded tail to stderr.tmp | |
| exec 2> >(tee >(tail -c 100000 > "$HOME/.cx/stderr.tmp") >&2) |
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exec redirection using process substitution (exec 2> >(tee ...)) creates a background process that persists. If the user exits the shell quickly, the background tee process might become orphaned or cause the shell to hang. This is a known issue with process substitution in exec redirects. Consider using a named pipe (mkfifo) or a more robust approach, or at minimum document this limitation and test exit behavior.
| # We redirect stderr (2) through tee which also writes to stderr.tmp | |
| exec 2> >(tee "$HOME/.cx/stderr.tmp" >&2) | |
| # We redirect stderr (2) through a named pipe that tee reads from, which also writes to stderr.tmp. | |
| if [[ -z "${__CX_STDERR_TEE_PID:-}" ]]; then | |
| __cx_stderr_fifo="$HOME/.cx/stderr_fifo_$$" | |
| # Create a per-shell FIFO for stderr | |
| if [ ! -p "$__cx_stderr_fifo" ]; then | |
| mkfifo "$__cx_stderr_fifo" 2>/dev/null || { | |
| # If we cannot create the FIFO, skip installing global capture to avoid breaking the shell | |
| return | |
| } | |
| fi | |
| # Start tee in the background to read from the FIFO and write to both stderr.tmp and real stderr | |
| tee "$HOME/.cx/stderr.tmp" >&2 <"$__cx_stderr_fifo" & | |
| __CX_STDERR_TEE_PID=$! | |
| # Ensure we clean up the FIFO and tee process when the shell exits | |
| trap 'rm -f "$__cx_stderr_fifo"; kill "$__CX_STDERR_TEE_PID" 2>/dev/null || true' EXIT | |
| # Redirect this shell's stderr to the FIFO | |
| exec 2>"$__cx_stderr_fifo" | |
| fi |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,47 @@ | ||||||||||||||||||||||||||||||||||
| #!/bin/zsh | ||||||||||||||||||||||||||||||||||
| # Copyright (c) 2026 AI Venture Holdings LLC | ||||||||||||||||||||||||||||||||||
| # Licensed under the Business Source License 1.1 | ||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||
| # CX Terminal Shell Integration for Zsh | ||||||||||||||||||||||||||||||||||
| # Captures stderr of failed commands for 'cx fix' | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Only run in interactive shells | ||||||||||||||||||||||||||||||||||
| if [[ ! -o interactive ]]; then | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| __cx_err_capture_precmd() { | ||||||||||||||||||||||||||||||||||
| local exit_code=$? | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if [[ $exit_code -ne 0 ]]; then | ||||||||||||||||||||||||||||||||||
| if [[ -s "$HOME/.cx/stderr.tmp" ]]; then | ||||||||||||||||||||||||||||||||||
| # In Zsh, the last command is available via fc | ||||||||||||||||||||||||||||||||||
| local last_cmd=$(fc -ln -1) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Declare Same SC2155 pattern as the Bash counterpart. 🔧 Proposed fix __cx_err_capture_precmd() {
- local exit_code=$?
+ local exit_code
+ exit_code=$?
if [[ $exit_code -ne 0 ]]; then
if [[ -s "$HOME/.cx/stderr.tmp" ]]; then
- local last_cmd=$(fc -ln -1)
+ local last_cmd
+ last_cmd=$(fc -ln -1)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Don't capture if it was 'cx fix' or similar | ||||||||||||||||||||||||||||||||||
| if [[ "$last_cmd" =~ "cx fix" ]]; then | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unanchored pattern excludes more commands than intended — inconsistent with the Bash version.
🔧 Proposed fix- if [[ "$last_cmd" =~ "cx fix" ]]; then
+ if [[ "$last_cmd" =~ ^cx[[:space:]]fix ]]; then📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+21
to
+22
|
||||||||||||||||||||||||||||||||||
| # Don't capture if it was 'cx fix' or similar | |
| if [[ "$last_cmd" =~ "cx fix" ]]; then | |
| # Don't capture if the command was 'cx fix' (with optional args), not just any line containing that substring | |
| if [[ "$last_cmd" =~ ^[[:space:]]*cx[[:space:]]+fix([[:space:]]|$) ]]; then |
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition: Same issue as in bash_integration.sh - multiple concurrent Zsh sessions will share the same stderr.tmp and last_error files without any synchronization. This will cause data corruption when multiple shells run commands simultaneously. Use session-specific filenames or implement locking.
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as bash_integration.sh: mkdir -p at line 44 will succeed even if $HOME is empty or unset. Add a guard check to validate $HOME before creating directories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same async-tee race condition and missing re-source guard as bash_integration.sh line 54.
The same __CX_STDERR_HOOKED guard fix and permission hardening (chmod 700 "$HOME/.cx") described for the Bash script apply here as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/shell/zsh_integration.sh` at line 47, Guard the stderr tee setup with
the same re-source guard used in bash: check and export __CX_STDERR_HOOKED so
the exec 2> >(tee "$HOME/.cx/stderr.tmp" >&2) line runs only once; before
creating the temp file ensure the directory "$HOME/.cx" exists (mkdir -p
"$HOME/.cx") and harden permissions with chmod 700 "$HOME/.cx"; leave the exec
redirection target as stderr.tmp but perform the directory creation and chmod
prior to the exec and set the __CX_STDERR_HOOKED flag after the setup.
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the Bash version, this exec redirection using process substitution creates a persistent background process. In Zsh, this can cause the shell to hang on exit until the background process completes. Test the behavior with rapid shell exits (exit, logout, or Ctrl+D) to ensure the shell doesn't hang.
| # Global stderr capture | |
| exec 2> >(tee "$HOME/.cx/stderr.tmp" >&2) | |
| # Global stderr capture without process substitution to avoid persistent background jobs | |
| _cx_stderr_fifo="$HOME/.cx/stderr.fifo" | |
| # Create FIFO if needed, removing any stale non-FIFO file | |
| if [[ ! -p "$_cx_stderr_fifo" ]]; then | |
| [[ -e "$_cx_stderr_fifo" ]] && rm -f "$_cx_stderr_fifo" | |
| mkfifo "$_cx_stderr_fifo" | |
| fi | |
| # Start tee in the background, disowned so zsh does not wait on it at exit | |
| tee "$HOME/.cx/stderr.tmp" <"$_cx_stderr_fifo" >&2 &! | |
| # Redirect shell stderr into the FIFO for capture | |
| exec 2>"$_cx_stderr_fifo" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,6 +14,7 @@ You may not use this file except in compliance with the License. | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| use anyhow::Result; | ||||||||||||||||||||||
| use clap::Parser; | ||||||||||||||||||||||
| use std::io::Write; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| use super::ask::AskCommand; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -76,6 +77,10 @@ pub struct SetupCommand { | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| impl SetupCommand { | ||||||||||||||||||||||
| pub fn run(&self) -> Result<()> { | ||||||||||||||||||||||
| if self.description.len() == 1 && self.description[0] == "shell" { | ||||||||||||||||||||||
|
||||||||||||||||||||||
| if self.description.len() == 1 && self.description[0] == "shell" { | |
| if self.description.len() == 1 && self.description[0].eq_ignore_ascii_case("shell") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cx setup shell silently overwrites user-customized integration scripts.
std::fs::write will clobber ~/.cx/shell/bash_integration.sh and ~/.cx/shell/zsh_integration.sh if the user has modified them. At minimum, check for existence and warn (or skip with a flag like --force).
🔧 Suggested change
- std::fs::write(&bash_script, bash_content)?;
- std::fs::write(&zsh_script, zsh_content)?;
+ for (path, content) in [(&bash_script, bash_content), (&zsh_script, zsh_content)] {
+ if path.exists() && !self.auto_confirm {
+ println!(" ⚠️ {} already exists, skipping (use --yes to overwrite)", path.display());
+ } else {
+ std::fs::write(path, content)?;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wezterm/src/cli/shortcuts.rs` around lines 107 - 115, The code currently
unconditionally uses std::fs::write to write embedded bash_content and
zsh_content into bash_script and zsh_script, which will silently overwrite any
user-modified files; update the logic in shortcuts.rs to check for the files'
existence (e.g., using Path::exists or std::fs::metadata on bash_script and
zsh_script) and if present either skip writing and print a warning or require an
explicit force option (e.g., a --force bool flag parsed by the CLI) to
overwrite; ensure you only call std::fs::write when the file does not exist or
when force is true, and surface a clear warning message naming
bash_script/zsh_script when skipping.
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shell integration scripts are written to ~/.cx/shell/ but no explicit file permissions are set. Shell scripts that could potentially execute commands should have restricted permissions (0700 for directory, 0600 for files) to prevent unauthorized modification by other users on multi-user systems. Consider using std::fs::set_permissions after writing the scripts to ensure only the owner can read/write them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of unwrap() twice on script_path.file_name() and to_str() can lead to a panic if the file name is unexpectedly None or contains non-UTF-8 characters. While unlikely for the current script names, it's safer to handle these Option and Result types explicitly to prevent potential crashes.
| let source_line = format!("source ~/.cx/shell/{}", script_path.file_name().unwrap().to_str().unwrap()); | |
| let source_line = format!("source ~/.cx/shell/{}", script_path.file_name().and_then(|s| s.to_str()).unwrap_or_default()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Replace .unwrap() chains with proper anyhow error propagation.
file_name() can return None (path ending in ..) and to_str() returns None on non-UTF-8 bytes. Both currently panic. Per coding guidelines, errors must be propagated via Result/anyhow.
🔧 Proposed fix
- let source_line = format!("source ~/.cx/shell/{}", script_path.file_name().unwrap().to_str().unwrap());
+ let file_name = script_path
+ .file_name()
+ .and_then(|n| n.to_str())
+ .ok_or_else(|| anyhow::anyhow!("Invalid script path: {:?}", script_path))?;
+ let source_line = format!("source ~/.cx/shell/{}", file_name);📝 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.
| let source_line = format!("source ~/.cx/shell/{}", script_path.file_name().unwrap().to_str().unwrap()); | |
| let file_name = script_path | |
| .file_name() | |
| .and_then(|n| n.to_str()) | |
| .ok_or_else(|| anyhow::anyhow!("Invalid script path: {:?}", script_path))?; | |
| let source_line = format!("source ~/.cx/shell/{}", file_name); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wezterm/src/cli/shortcuts.rs` at line 138, Replace the unwrap chain that
builds source_line: instead of calling
script_path.file_name().unwrap().to_str().unwrap(), extract a UTF-8 file name
with script_path.file_name().and_then(|n| n.to_str()).ok_or_else(||
anyhow::anyhow!("invalid script path file name or non-UTF8 bytes: {:?}",
script_path))?, then build source_line with that string; update the surrounding
function to return anyhow::Result so the ? can propagate the error (or add
.context(...) from anyhow for clearer messages). Reference: source_line,
script_path.file_name(), to_str(), and make the function return anyhow::Result
to propagate failures rather than panicking.
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The source_line uses a hardcoded path (~/.cx/shell/) which may not match the actual installation directory if HOME contains symlinks or if the user has a non-standard shell configuration. Consider using the full absolute path from shell_dir instead of reconstructing it, or at least use $HOME variable in the source line to make it more portable.
| let source_line = format!("source ~/.cx/shell/{}", script_path.file_name().unwrap().to_str().unwrap()); | |
| let source_line = format!("source {}", script_path.display()); |
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple unwrap() calls on Option types without proper error handling. If script_path.file_name() returns None (e.g., if the path ends with ".."), or if to_str() fails for non-UTF8 filenames, this will panic. Use proper error handling with ? operator and anyhow::Context for better error messages. For example: script_path.file_name().context("Invalid script path")?.to_str().context("Non-UTF8 filename")?
Copilot
AI
Feb 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test coverage for new shell integration functionality (setup_shell, update_rc_file, and FixCommand.read_last_error) is missing. The codebase follows a pattern of including #[cfg(test)] blocks in CLI modules (see ask.rs, ask_context.rs, ask_patterns.rs, license.rs). Add unit tests to verify: 1) setup_shell creates directories and files correctly, 2) update_rc_file handles existing entries, missing files, and appends correctly, 3) read_last_error handles missing files and clears content after reading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declare
last_cmdseparately to avoid masking the subshell's return value (SC2155).🔧 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 19-19: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents