fix: remove sudo requirement from install flow#880
Open
fireddd wants to merge 6 commits intoComposioHQ:mainfrom
Open
fix: remove sudo requirement from install flow#880fireddd wants to merge 6 commits intoComposioHQ:mainfrom
fireddd wants to merge 6 commits intoComposioHQ:mainfrom
Conversation
- setup.sh: replace sudo npm link fallback with automatic user-local npm prefix configuration (~/.npm-global) when system prefix is not writable. Persists PATH to shell profile. - README.md: replace sudo suggestion with npm prefix fix and npx option - SETUP.md: remove sudo as option, reorder to put recommended no-sudo path first Closes ComposioHQ#878 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The in-script `export PATH` only affected the script's subprocess, not the user's shell. Replace with an explicit message to reload the shell after npm prefix reconfiguration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of printing a manual reload instruction, use exec $SHELL -l to transparently restart the user's shell with the updated PATH. This ensures 'ao' is immediately available after setup without requiring the user to manually source their profile. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
fireddd
commented
Apr 2, 2026
fireddd
commented
Apr 2, 2026
fireddd
commented
Apr 2, 2026
- setup.sh: guard exec $SHELL -l behind interactive check so CI and parent scripts are not replaced with a login shell - setup.sh: add error handling if mkdir or npm config set fails, with manual fallback instructions - README.md: remove sudo mention from EACCES guidance - SETUP.md: remove sudo mention from EACCES guidance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All three scripts that call npm link (setup.sh, ao-update.sh, ao-doctor.sh) now source a single ensure-npm-prefix.sh helper. This eliminates the sudo npm link fallback from all paths and ensures consistent behavior across install, update, and doctor. The shared script: - Detects if npm prefix is user-writable - Auto-configures ~/.npm-global if not - Persists PATH to shell profile (idempotent) - Exports NEEDS_SHELL_RELOAD for callers that want to exec $SHELL - Fails with manual instructions if mkdir/npm-config fails Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ensure-npm-prefix.sh: use return 1 instead of exit 1 so sourcing scripts (ao-doctor) aren't killed on failure - setup.sh: use $SHELL_RC instead of hardcoded ~/.zshrc in non-interactive reload message so bash users get the correct path - ao-update.sh: notify user when npm prefix was reconfigured during update Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
Test Plan ResultsAll tests passing on
Review comment resolution
🤖 Generated with Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| if [ "${NEEDS_SHELL_RELOAD:-}" = true ] && [ "$INTERACTIVE" = true ]; then | ||
| echo "Restarting shell to pick up new PATH..." | ||
| exec "$SHELL" -l |
There was a problem hiding this comment.
Shell restart without PATH persistence breaks ao availability
Medium Severity
When NEEDS_SHELL_RELOAD is true and INTERACTIVE is true, exec "$SHELL" -l fires unconditionally — even when SHELL_RC is empty because neither ~/.zshrc nor ~/.bashrc exists. On a fresh macOS (where zsh is default but ~/.zshrc hasn't been created yet), the PATH entry for ~/.npm-global/bin is never persisted to any profile. The exec replaces the current process (which had the correct PATH via in-process export) with a login shell that lacks it, making ao unavailable right after "Setup complete!" is printed. The guard at line 169 needs an additional check that SHELL_RC is non-empty before calling exec.
Additional Locations (1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sudo npm linkfallback with automatic user-local npm prefix configuration (~/.npm-global). When the system npm prefix is not user-writable, the script now auto-configures a user-local prefix and persists the PATH entry to the shell profile.npxalternative.sudoas an install option entirely, reorder to put the recommended no-sudo path first.Closes #878
Test plan
scripts/setup.shon a fresh macOS/Linux system where npm global prefix is system-owned — verify it auto-configures~/.npm-globalwithout prompting for sudoaocommand works after setup on both macOS and Linuxnpx @composio/ao startworks without any global install🤖 Generated with Claude Code