Skip to content

Conversation

@kshivang
Copy link
Owner

Summary

  • Keep sudo credentials alive until ALL commands complete (fixes Starship password prompt)
  • Clean up partial npm installations before reinstalling AI assistants (fixes ENOTEMPTY error)

Test plan

  • Run wizard and select Starship - should install without password prompt
  • Interrupt AI assistant installation, re-run wizard - should succeed without ENOTEMPTY error

🤖 Generated with Claude Code

Starship's install script internally uses sudo to copy the binary to
/usr/local/bin. Previously, the sudo keepalive was killed right after
explicit sudo commands, causing Starship to prompt for password.

Now sudo credentials are kept alive until all commands complete.

Generated with [Claude Code](https://claude.com/claude-code)
…ants

Fixes ENOTEMPTY error that occurs when a previous npm installation was
interrupted, leaving a non-empty directory that npm can't rename.

Now removes existing package directories before npm install -g runs.

Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link

claude bot commented Jan 15, 2026

Code Review

This PR addresses two important issues:

  1. Sudo credential timeout - Keeps sudo alive until ALL commands complete
  2. npm ENOTEMPTY errors - Cleans up partial npm installations

Strengths

  • Correctly identifies Starship install script uses sudo at line 943
  • Cross-platform cleanup for Unix and Windows
  • Safe error handling with proper suppression
  • Logical code flow with clear comments

Suggestions

  1. Sudo detection at line 1123 uses string matching. Consider explicit check with selections.shellCustomization
  2. Missing unit tests for buildInstallCommandInternal
  3. Minor code duplication at lines 1050-1051

Security

  • No credential leakage
  • Sudo keepalive properly terminated
  • Scoped file deletion
  • Hardcoded package names prevent injection

Verdict

Approve with minor suggestions. Successfully fixes real bugs. Code is functional and safe.

Recommendations (non-blocking): Add unit tests, make sudo detection explicit, reduce duplication

@kshivang kshivang merged commit 65634fb into master Jan 15, 2026
7 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.

2 participants