Skip to content

Move shell environment setup into install-hooks#1971

Open
svarlamov wants to merge 1 commit into
mainfrom
feat/move-shell-env-mutation-to-install-cmd
Open

Move shell environment setup into install-hooks#1971
svarlamov wants to merge 1 commit into
mainfrom
feat/move-shell-env-mutation-to-install-cmd

Conversation

@svarlamov

@svarlamov svarlamov commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

  • move Unix shell profile detection and PATH mutation into the Rust install-hooks command
  • move Windows user PATH and Git Bash profile configuration into the Rust install-hooks command
  • keep dry-run, idempotency, shell preference, fallback, skip-path, output, and MDM ownership behavior aligned with the existing installers
  • remove the migrated logic from install.sh and install.ps1 and replace the obsolete BATS coverage with Rust unit and TestRepo integration tests

Impact

The install scripts are shorter, while direct install-hooks invocations now perform the same shell environment setup as package installation.

Validation

  • task fmt
  • task lint
  • task build
  • task test
  • Windows target test compilation with cargo check --tests --target x86_64-pc-windows-msvc
  • bash -n install.sh
  • git diff --check

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@svarlamov
svarlamov force-pushed the feat/move-shell-env-mutation-to-install-cmd branch from 0f9ca8f to f23291e Compare July 23, 2026 15:59
@svarlamov
svarlamov marked this pull request as ready for review July 23, 2026 18:49

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 1 potential issue.

Open in Devin Review


// Run async operations and convert result.
let statuses = crate::tokio_runtime::block_on(async_run_install(&params, &options))?;
shell_env::configure(&params.binary_path, options.dry_run)?;

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.

🟡 A shell profile that cannot be written makes hook installation report failure

Setting up shell PATH entries can now abort the whole hook-installation command (shell_env::configure(...)? at src/commands/install_hooks.rs:350) whenever a shell profile cannot be written, so hooks that actually installed fine are reported as failed.
Impact: If any shell profile (e.g. an unwritable ~/.bashrc/.zshrc) can't be updated, the user sees a misleading "Failed to set up IDE/agent hooks" message and a non-zero exit even though hooks were installed successfully.

How the coupling changes failure behavior

Previously the shell PATH/profile setup lived in install.sh / install.ps1 and was independent of hook installation: the installer ran install-hooks first (warning on failure) and then did shell configuration separately. Now run() calls shell_env::configure(...)? right after async_run_install (src/commands/install_hooks.rs:349-350). configure_unix propagates any IO error from OpenOptions::open/writeln!/create_dir_all (src/commands/install_hooks/shell_env.rs:107-118) via ?. When that error bubbles up, run() returns Err, which in src/commands/git_ai_handlers.rs:166-169 causes eprintln!("Install hooks failed") and std::process::exit(1). In install.sh the branch if ! ... install-hooks; then warn "Warning: Failed to set up IDE/agent hooks..." then fires even though every hook installed correctly. It also skips cleanup_legacy_envelope_logs() (src/commands/install_hooks.rs:354-356).

Prompt for agents
In src/commands/install_hooks.rs:350, shell_env::configure(...) is invoked with the `?` operator, which makes any failure to write a user's shell profile (e.g. an unwritable ~/.bashrc) fatal to the entire install-hooks command. This diverges from the previous behavior where shell environment setup lived in install.sh/install.ps1 and was best-effort/independent of hook installation success. The consequence is that a shell-profile write error now causes install-hooks to exit non-zero (see git_ai_handlers.rs:166-169) and makes install.sh print the misleading 'Failed to set up IDE/agent hooks' warning even though all hooks installed fine. Consider making shell_env::configure best-effort: log a warning on error (matching the previous non-fatal semantics) instead of propagating the error out of run(). Note that configure_unix itself already accumulates results and only fails on IO errors; you likely want to catch/log those rather than return Err from run().
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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