Prompt agent skill installation during CLI install/upgrade#841
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds interactive, TTY-gated post-install prompts to Linux and macOS installer scripts that detect existing OSMO agent skill installs, offer to run Changes
Sequence DiagramsequenceDiagram
participant User
participant Installer as Installer Script
participant FS as File System
participant NPX as npx/Node
participant Skills as Skills Registry
User->>Installer: Run postinstall
Installer->>FS: Check for SKILL.md in candidate dirs
alt Skill found
Installer->>User: Skip prompt
else Skill not found and TTY
Installer->>User: Prompt "Install OSMO skill? (Y/n)"
User->>Installer: Respond (yes/no or empty)
alt User confirms
Installer->>NPX: Check `npx` availability
alt npx available
Installer->>NPX: `npx skills add nvidia/osmo`
NPX->>Skills: Request install
Skills-->>NPX: Install success/failure
NPX-->>Installer: Result
Installer->>User: Show success (and removal cmd) or retry guidance
else npx missing
Installer->>User: Show Node.js install instructions + follow-up `npx skills add nvidia/osmo`
end
else User declines
Installer->>User: Exit without installing
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli/packaging/linux/installer.sh (1)
18-18:⚠️ Potential issue | 🟠 MajorGuard
readunderset -eso EOF does not fail install.With
set -e, an EOF/Ctrl-D on the unguardedread -pcommand can cause the script to exit early, making the optional agent skill installation prompt cause a hard failure instead of gracefully handling user cancellation.Also applies to:
src/cli/packaging/macos/postinstall:74Proposed fix
- read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL - INSTALL_SKILL="${INSTALL_SKILL:-y}" + if ! read -r -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL; then + INSTALL_SKILL="n" + fi + INSTALL_SKILL="${INSTALL_SKILL:-y}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` at line 18, The script has set -e so an EOF/Ctrl-D on the interactive prompt (the unguarded read -p used to ask about optional agent skill installation) will cause the whole installer to exit; wrap that read so it cannot trigger an exit by temporarily disabling the errexit behavior or by appending a safe fallback. Specifically, around the read -p invocation in installer.sh (and the similar read in macos/postinstall), disable errexit (e.g., save/set +e), perform read -r -p ... and handle the case where read returns non-zero (treat as a canceled/no response), then restore set -e (or restore the saved errexit state); ensure subsequent logic interprets a non-successful read as “do not install” rather than letting the script terminate.src/cli/packaging/macos/postinstall (1)
18-18:⚠️ Potential issue | 🟠 MajorGuard the
readprompt against EOF to prevent unexpected script abort underset -e.With
set -eat line 18, an unhandled EOF (Ctrl-D) at the prompt returns non-zero and terminates the script. Apply the proposed fix to default tonon EOF:Proposed fix
- read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL - INSTALL_SKILL="${INSTALL_SKILL:-y}" + if ! read -r -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL; then + INSTALL_SKILL="n" + fi + INSTALL_SKILL="${INSTALL_SKILL:-y}"Also applies to: lines 74–75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/macos/postinstall` at line 18, The script uses set -e which causes an unhandled EOF (Ctrl-D) at interactive prompts to abort; update each interactive read invocation (the read prompts near the top under set -e and the two reads around lines 74–75) to guard against EOF by defaulting the reply to "n" when read fails—for example replace plain read (or IFS= read -r reply) with a guarded form like: IFS= read -r reply || reply="n" (then use "${reply}" in the subsequent checks), ensuring all prompt reads are similarly protected.
🧹 Nitpick comments (1)
src/cli/packaging/linux/installer.sh (1)
123-163: Consider centralizing this prompt flow to avoid Linux/macOS drift.This block is effectively duplicated across both installer scripts; extracting a shared helper/template will reduce future divergence risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` around lines 123 - 163, Extract the duplicated OSMO skill prompt flow into a shared helper (e.g., a function named prompt_install_osmo_skill or a sourced script like packaging/shared/osmo_prompt.sh) and call it from both installer scripts instead of repeating the loop and interactive logic; move the OSMO_SKILL_INSTALLED detection (the for agent_dir loop checking "$agent_dir/skills/osmo-agent/SKILL.md"), the interactive prompt (INSTALL_SKILL read and default), and the npx invoke (npx skills add nvidia/osmo and its success/failure messages) into that helper, ensure the helper respects non-interactive tty checks and command -v npx, and update both linux and mac installers to source/call the new helper to prevent drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/cli/packaging/linux/installer.sh`:
- Line 18: The script has set -e so an EOF/Ctrl-D on the interactive prompt (the
unguarded read -p used to ask about optional agent skill installation) will
cause the whole installer to exit; wrap that read so it cannot trigger an exit
by temporarily disabling the errexit behavior or by appending a safe fallback.
Specifically, around the read -p invocation in installer.sh (and the similar
read in macos/postinstall), disable errexit (e.g., save/set +e), perform read -r
-p ... and handle the case where read returns non-zero (treat as a canceled/no
response), then restore set -e (or restore the saved errexit state); ensure
subsequent logic interprets a non-successful read as “do not install” rather
than letting the script terminate.
In `@src/cli/packaging/macos/postinstall`:
- Line 18: The script uses set -e which causes an unhandled EOF (Ctrl-D) at
interactive prompts to abort; update each interactive read invocation (the read
prompts near the top under set -e and the two reads around lines 74–75) to guard
against EOF by defaulting the reply to "n" when read fails—for example replace
plain read (or IFS= read -r reply) with a guarded form like: IFS= read -r reply
|| reply="n" (then use "${reply}" in the subsequent checks), ensuring all prompt
reads are similarly protected.
---
Nitpick comments:
In `@src/cli/packaging/linux/installer.sh`:
- Around line 123-163: Extract the duplicated OSMO skill prompt flow into a
shared helper (e.g., a function named prompt_install_osmo_skill or a sourced
script like packaging/shared/osmo_prompt.sh) and call it from both installer
scripts instead of repeating the loop and interactive logic; move the
OSMO_SKILL_INSTALLED detection (the for agent_dir loop checking
"$agent_dir/skills/osmo-agent/SKILL.md"), the interactive prompt (INSTALL_SKILL
read and default), and the npx invoke (npx skills add nvidia/osmo and its
success/failure messages) into that helper, ensure the helper respects
non-interactive tty checks and command -v npx, and update both linux and mac
installers to source/call the new helper to prevent drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61cb5790-e58c-49d5-a122-de6e7f6f51a6
📒 Files selected for processing (2)
src/cli/packaging/linux/installer.shsrc/cli/packaging/macos/postinstall
jiaenren
force-pushed
the
jiaenr/agent-skill-install
branch
from
0e69b4d to
0735ec7
Compare
jiaenren
force-pushed
the
jiaenr/agent-skill-install
branch
from
0735ec7 to
ed5821f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/packaging/linux/installer.sh`:
- Line 146: Replace the unsafe read invocation with the raw-read form: change
the prompt call from read -p "Install the OSMO agent skill? [Y/n]: "
INSTALL_SKILL to read -r -p "Install the OSMO agent skill? [Y/n]: "
INSTALL_SKILL (and apply the same change to the equivalent prompt in the macOS
postinstall script), so backslashes in user input are not treated as escape
sequences.
- Around line 127-132: The installer currently uses $HOME and runs npx as the
effective user, causing detection/installation under /root when invoked with
sudo; update the agent_dir detection loop (where OSMO_SKILL_INSTALLED is set)
and any npx invocations to resolve the invoking user's home and run commands as
that user by using SUDO_USER (e.g., derive target_home via getent passwd
"$SUDO_USER" or eval echo "~$SUDO_USER" when SUDO_USER is set) and use sudo -u
"$SUDO_USER" for npx; also change any read -p usages to read -r -p to avoid
backslash interpretation (affecting the prompt handling). Ensure these changes
are applied to the OSMO_SKILL_INSTALLED detection loop, the npx install/run
sites, and the interactive read prompts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1683fe0b-feaa-4cd2-8420-dd77a106bbb7
📒 Files selected for processing (2)
src/cli/packaging/linux/installer.shsrc/cli/packaging/macos/postinstall
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/packaging/macos/postinstall
There was a problem hiding this comment.
♻️ Duplicate comments (4)
src/cli/packaging/linux/installer.sh (2)
146-146:⚠️ Potential issue | 🟡 MinorSwitch to
read -rfor prompt parsing safety.Line 146 should be
read -r -p ...to avoid backslash mangling.Verification script
#!/bin/bash # Expect no output after replacing all prompt reads with `read -r -p`. rg -n --type=sh 'read\s+-p\s+"Install the OSMO agent skill\? \[Y/n\]:"' \ src/cli/packaging/linux/installer.sh \ src/cli/packaging/macos/postinstall🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` at line 146, The prompt read uses unsafe parsing; change the read invocation that assigns INSTALL_SKILL (the line using read -p "Install the OSMO agent skill? [Y/n]: ") to use read -r -p so backslashes aren't interpreted; update the read call that sets INSTALL_SKILL to use the -r flag and keep the same prompt and variable name.
127-129:⚠️ Potential issue | 🟠 MajorHandle sudo/root installs using the invoking user’s home and user context.
Line 127 and Line 149 still bind detection/install to the effective user; with sudo/root invocation this can detect/install in
/rootinstead of the caller’s account.Suggested fix
+TARGET_USER="${SUDO_USER:-$(id -un)}" +if [ "$TARGET_USER" = "root" ]; then + TARGET_HOME="$HOME" +else + TARGET_HOME="$(getent passwd "$TARGET_USER" | cut -d: -f6)" +fi + OSMO_SKILL_INSTALLED=false -for agent_dir in "$HOME/.claude" "$HOME/.codex" "$HOME/.agents"; do +for agent_dir in "$TARGET_HOME/.claude" "$TARGET_HOME/.codex" "$TARGET_HOME/.agents"; do if [ -f "$agent_dir/skills/osmo-agent/SKILL.md" ]; then OSMO_SKILL_INSTALLED=true break @@ - if npx skills add nvidia/osmo; then + if [ "$(id -u)" -eq 0 ] && [ -n "${SUDO_USER:-}" ] && [ "$SUDO_USER" != "root" ]; then + INSTALL_CMD=(sudo -u "$SUDO_USER" -H npx skills add nvidia/osmo) + else + INSTALL_CMD=(npx skills add nvidia/osmo) + fi + if "${INSTALL_CMD[@]}"; thenAlso applies to: 149-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` around lines 127 - 129, The current detection loop using "$HOME" (for example the for loop that checks "$agent_dir/skills/osmo-agent/SKILL.md" and sets OSMO_SKILL_INSTALLED) will use the effective user (root under sudo); change it to detect and use the invoking user's home and UID by resolving an INVOKING_USER/INVOKING_HOME (e.g. from SUDO_USER or fall back to USER/getent passwd) and then iterate those paths (e.g. "$INVOKING_HOME/.claude" "$INVOKING_HOME/.codex" "$INVOKING_HOME/.agents"); apply the same INVOKING_HOME usage wherever "$HOME" is used later (including the install/detect at the other occurrence around line 149) so detection and installation operate in the caller’s account context rather than root.src/cli/packaging/macos/postinstall (2)
75-75:⚠️ Potential issue | 🟡 MinorUse raw
readfor prompt input.Line 75 should use
read -r -p ...so backslashes are not interpreted.Verification script
#!/bin/bash # Expect no output after fixing both packaging scripts. rg -n --type=sh 'read\s+-p\s+"Install the OSMO agent skill\? \[Y/n\]:"' \ src/cli/packaging/macos/postinstall \ src/cli/packaging/linux/installer.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/macos/postinstall` at line 75, Replace the raw read invocation with a raw-safe read by changing the prompt call `read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL` to use the -r flag (`read -r -p ...`) so backslashes are not interpreted; update this exact invocation in both places where it appears (the `read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL` line and the duplicate occurrence in the other installer script) so both scripts use `read -r -p`.
56-58:⚠️ Potential issue | 🟠 MajorInstall/detect the skill in the real user account, not root.
Line 56 and Line 78 use the effective account (
$HOME, directnpx) inpostinstall; this commonly resolves to root on macOS package installs and can install/detect under the wrong home directory.Suggested fix
+TARGET_USER="${SUDO_USER:-$(stat -f '%Su' /dev/console 2>/dev/null || true)}" +if [ -z "$TARGET_USER" ] || [ "$TARGET_USER" = "root" ]; then + TARGET_USER="$(id -un)" +fi +TARGET_HOME="$(dscl . -read "/Users/$TARGET_USER" NFSHomeDirectory 2>/dev/null | awk '{print $2}')" +[ -z "$TARGET_HOME" ] && TARGET_HOME="$HOME" + OSMO_SKILL_INSTALLED=false -for agent_dir in "$HOME/.claude" "$HOME/.codex" "$HOME/.agents"; do +for agent_dir in "$TARGET_HOME/.claude" "$TARGET_HOME/.codex" "$TARGET_HOME/.agents"; do if [ -f "$agent_dir/skills/osmo-agent/SKILL.md" ]; then OSMO_SKILL_INSTALLED=true break @@ - if npx skills add nvidia/osmo; then + if [ "$(id -u)" -eq 0 ] && [ "$TARGET_USER" != "root" ]; then + INSTALL_CMD=(sudo -u "$TARGET_USER" -H npx skills add nvidia/osmo) + else + INSTALL_CMD=(npx skills add nvidia/osmo) + fi + if "${INSTALL_CMD[@]}"; thenAlso applies to: 78-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/macos/postinstall` around lines 56 - 58, The postinstall currently uses $HOME and direct npx (see the agent_dir loop checking "$HOME/.claude" etc and the npx invocation) which runs as root during macOS pkg installs; change it to detect the real target user via SUDO_USER (fall back to logname/USER) and compute a USER_HOME variable (e.g. derive NFSHomeDirectory for SUDO_USER on macOS or use eval "~$SUDO_USER"), then replace occurrences of "$HOME" in the agent_dir loop and SKILL.md checks with "$USER_HOME" and invoke npx under that user (e.g. sudo -u "$SUDO_USER" HOME="$USER_HOME" npx ... or at least set HOME="$USER_HOME" when running npx) so install/detection operate on the real user account; update OSMO_SKILL_INSTALLED checks to use the new USER_HOME variable accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/cli/packaging/linux/installer.sh`:
- Line 146: The prompt read uses unsafe parsing; change the read invocation that
assigns INSTALL_SKILL (the line using read -p "Install the OSMO agent skill?
[Y/n]: ") to use read -r -p so backslashes aren't interpreted; update the read
call that sets INSTALL_SKILL to use the -r flag and keep the same prompt and
variable name.
- Around line 127-129: The current detection loop using "$HOME" (for example the
for loop that checks "$agent_dir/skills/osmo-agent/SKILL.md" and sets
OSMO_SKILL_INSTALLED) will use the effective user (root under sudo); change it
to detect and use the invoking user's home and UID by resolving an
INVOKING_USER/INVOKING_HOME (e.g. from SUDO_USER or fall back to USER/getent
passwd) and then iterate those paths (e.g. "$INVOKING_HOME/.claude"
"$INVOKING_HOME/.codex" "$INVOKING_HOME/.agents"); apply the same INVOKING_HOME
usage wherever "$HOME" is used later (including the install/detect at the other
occurrence around line 149) so detection and installation operate in the
caller’s account context rather than root.
In `@src/cli/packaging/macos/postinstall`:
- Line 75: Replace the raw read invocation with a raw-safe read by changing the
prompt call `read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL` to
use the -r flag (`read -r -p ...`) so backslashes are not interpreted; update
this exact invocation in both places where it appears (the `read -p "Install the
OSMO agent skill? [Y/n]: " INSTALL_SKILL` line and the duplicate occurrence in
the other installer script) so both scripts use `read -r -p`.
- Around line 56-58: The postinstall currently uses $HOME and direct npx (see
the agent_dir loop checking "$HOME/.claude" etc and the npx invocation) which
runs as root during macOS pkg installs; change it to detect the real target user
via SUDO_USER (fall back to logname/USER) and compute a USER_HOME variable (e.g.
derive NFSHomeDirectory for SUDO_USER on macOS or use eval "~$SUDO_USER"), then
replace occurrences of "$HOME" in the agent_dir loop and SKILL.md checks with
"$USER_HOME" and invoke npx under that user (e.g. sudo -u "$SUDO_USER"
HOME="$USER_HOME" npx ... or at least set HOME="$USER_HOME" when running npx) so
install/detection operate on the real user account; update OSMO_SKILL_INSTALLED
checks to use the new USER_HOME variable accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e0348f4-3f90-4e9f-83a8-84a848c18aef
📒 Files selected for processing (2)
src/cli/packaging/linux/installer.shsrc/cli/packaging/macos/postinstall
jiaenren
force-pushed
the
jiaenr/agent-skill-install
branch
from
ed5821f to
fdae1b1
Compare
jiaenren
force-pushed
the
jiaenr/agent-skill-install
branch
from
fdae1b1 to
e993369
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/cli/packaging/linux/installer.sh (1)
127-149:⚠️ Potential issue | 🟠 MajorInstall/check the skill in the invoking user context, not root.
Lines 127-149 still use
$HOMEand directnpx, sosudo ./installer.shchecks/installs under/rootinstead of the caller’s home. Also, Line 146 should useread -r -pto avoid backslash mangling.Suggested patch
+TARGET_USER="${SUDO_USER:-$(id -un)}" +if [ "$TARGET_USER" = "root" ]; then + TARGET_HOME="$HOME" +else + TARGET_HOME="$(getent passwd "$TARGET_USER" | cut -d: -f6)" +fi + OSMO_SKILL_INSTALLED=false -for agent_dir in "$HOME/.claude" "$HOME/.codex" "$HOME/.agents"; do +for agent_dir in "$TARGET_HOME/.claude" "$TARGET_HOME/.codex" "$TARGET_HOME/.agents"; do if [ -f "$agent_dir/skills/osmo-agent/SKILL.md" ]; then OSMO_SKILL_INSTALLED=true break @@ - read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL + read -r -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL INSTALL_SKILL="${INSTALL_SKILL:-y}" if [ "$INSTALL_SKILL" = "y" ] || [ "$INSTALL_SKILL" = "Y" ]; then - if npx skills add nvidia/osmo; then + if [ "$(id -u)" -eq 0 ] && [ -n "${SUDO_USER:-}" ] && [ "$SUDO_USER" != "root" ]; then + INSTALL_CMD=(sudo -u "$SUDO_USER" -H npx skills add nvidia/osmo) + else + INSTALL_CMD=(npx skills add nvidia/osmo) + fi + if "${INSTALL_CMD[@]}"; then echo "" echo "To remove: npx skills remove osmo-agent"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` around lines 127 - 149, The script currently inspects and installs the OSMO skill using $HOME and runs npx as the current effective user (root when invoked via sudo), so change checks and installs to operate in the invoking user's context: compute the caller home (e.g., OWNER_HOME=$(eval echo "~${SUDO_USER:-$USER}") or use getent if available) and use that variable instead of $HOME when checking OSMO_SKILL_INSTALLED and any file paths (refer to OSMO_SKILL_INSTALLED and the SKILL.md check); run the installer command as the invoking user (e.g., sudo -u "$SUDO_USER" npx skills add nvidia/osmo or use su -c) so npx executes in the caller environment (refer to the npx skills add nvidia/osmo invocation), and change the prompt to use read -r -p when reading INSTALL_SKILL to avoid backslash mangling.src/cli/packaging/macos/postinstall (1)
56-78:⚠️ Potential issue | 🟠 MajorUse the logged-in user context for skill detection/install in postinstall.
Lines 56-78 currently use root’s
$HOMEand runnpxas root in postinstall. That installs/detects the skill in the wrong user profile. Line 75 should also useread -r -p.Suggested patch
+TARGET_USER="${SUDO_USER:-$(stat -f%Su /dev/console 2>/dev/null)}" +[ -z "$TARGET_USER" ] && TARGET_USER="$USER" +TARGET_HOME="$(eval echo "~$TARGET_USER")" + OSMO_SKILL_INSTALLED=false -for agent_dir in "$HOME/.claude" "$HOME/.codex" "$HOME/.agents"; do +for agent_dir in "$TARGET_HOME/.claude" "$TARGET_HOME/.codex" "$TARGET_HOME/.agents"; do if [ -f "$agent_dir/skills/osmo-agent/SKILL.md" ]; then OSMO_SKILL_INSTALLED=true break @@ - read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL + read -r -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL INSTALL_SKILL="${INSTALL_SKILL:-y}" if [ "$INSTALL_SKILL" = "y" ] || [ "$INSTALL_SKILL" = "Y" ]; then - if npx skills add nvidia/osmo; then + if [ "$(id -u)" -eq 0 ] && [ "$TARGET_USER" != "root" ]; then + INSTALL_CMD=(sudo -u "$TARGET_USER" -H npx skills add nvidia/osmo) + else + INSTALL_CMD=(npx skills add nvidia/osmo) + fi + if "${INSTALL_CMD[@]}"; then echo "" echo "To remove: npx skills remove osmo-agent"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/macos/postinstall` around lines 56 - 78, The postinstall uses root's $HOME and runs npx as root causing detection/installation in the wrong profile; change the agent discovery and installation to use the actual logged-in user's home (derive AGENT_HOME from SUDO_USER or fallback to $USER/logname) and replace occurrences of "$HOME" in the agent_dir loop (symbol: agent_dir and OSMO_SKILL_INSTALLED) with that AGENT_HOME, and run npx as that user (use sudo -u or equivalent while preserving HOME/env) so installs land in the correct user profile; also change the prompt read call to use read -r -p when setting INSTALL_SKILL (symbol: INSTALL_SKILL) to avoid backslash interpretation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/packaging/macos/postinstall`:
- Line 63: The interactive installation guard in the postinstall
script—specifically the condition if [ -t 0 ] && [ "$OSMO_SKILL_INSTALLED" =
false ]; then and its corresponding interactive prompt block—will never run in
real macOS installs because postinstall runs without a TTY; remove the entire
interactive prompt block (the code guarded by that condition) from the
postinstall script, or alternatively move the interactive install logic into a
first-run/setup script that runs in an interactive context and checks
OSMO_SKILL_INSTALLED there before prompting.
---
Duplicate comments:
In `@src/cli/packaging/linux/installer.sh`:
- Around line 127-149: The script currently inspects and installs the OSMO skill
using $HOME and runs npx as the current effective user (root when invoked via
sudo), so change checks and installs to operate in the invoking user's context:
compute the caller home (e.g., OWNER_HOME=$(eval echo "~${SUDO_USER:-$USER}") or
use getent if available) and use that variable instead of $HOME when checking
OSMO_SKILL_INSTALLED and any file paths (refer to OSMO_SKILL_INSTALLED and the
SKILL.md check); run the installer command as the invoking user (e.g., sudo -u
"$SUDO_USER" npx skills add nvidia/osmo or use su -c) so npx executes in the
caller environment (refer to the npx skills add nvidia/osmo invocation), and
change the prompt to use read -r -p when reading INSTALL_SKILL to avoid
backslash mangling.
In `@src/cli/packaging/macos/postinstall`:
- Around line 56-78: The postinstall uses root's $HOME and runs npx as root
causing detection/installation in the wrong profile; change the agent discovery
and installation to use the actual logged-in user's home (derive AGENT_HOME from
SUDO_USER or fallback to $USER/logname) and replace occurrences of "$HOME" in
the agent_dir loop (symbol: agent_dir and OSMO_SKILL_INSTALLED) with that
AGENT_HOME, and run npx as that user (use sudo -u or equivalent while preserving
HOME/env) so installs land in the correct user profile; also change the prompt
read call to use read -r -p when setting INSTALL_SKILL (symbol: INSTALL_SKILL)
to avoid backslash interpretation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0179f452-8359-4b9d-b70b-9979d2da7c3a
📒 Files selected for processing (2)
src/cli/packaging/linux/installer.shsrc/cli/packaging/macos/postinstall
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/cli/packaging/linux/installer.sh (3)
146-146:⚠️ Potential issue | 🟡 MinorAdd
-rflag toreadcommand for safe input handling.Without the
-rflag, backslashes in user input are interpreted as escape sequences, which can lead to unexpected behavior.🛡️ Proposed fix
- read -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILL + read -r -p "Install the OSMO agent skill? [Y/n]: " INSTALL_SKILLAs per static analysis hints, ShellCheck SC2162: read without -r will mangle backslashes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` at line 146, The read invocation prompting "Install the OSMO agent skill? [Y/n]: " (the command that sets INSTALL_SKILL) needs the -r flag to prevent backslashes in user input being treated as escape sequences; update that read command to include -r (e.g., use read with -r and preserve the existing prompt and variable name INSTALL_SKILL) so input is handled safely and ShellCheck SC2162 is satisfied.
127-132:⚠️ Potential issue | 🔴 CriticalFix user context detection when installer runs with
sudo.When the installer is invoked with
sudo,$HOMEresolves to/rootinstead of the invoking user's home directory. This causes the skill detection to check root's directories rather than the actual user's, leading to incorrect detection results.🔧 Proposed fix to resolve target user and home directory
Add this before line 127:
+# Determine the actual user and their home directory +TARGET_USER="${SUDO_USER:-$(id -un)}" +if [ "$TARGET_USER" = "root" ]; then + TARGET_HOME="$HOME" +else + TARGET_HOME="$(getent passwd "$TARGET_USER" | cut -d: -f6)" +fi + OSMO_SKILL_INSTALLED=false -for agent_dir in "$HOME/.claude" "$HOME/.codex" "$HOME/.agents"; do +for agent_dir in "$TARGET_HOME/.claude" "$TARGET_HOME/.codex" "$TARGET_HOME/.agents"; do if [ -f "$agent_dir/skills/osmo-agent/SKILL.md" ]; thenNote: This was previously flagged and also affects line 149 where
npxis executed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` around lines 127 - 132, The installer currently uses $HOME which becomes /root under sudo; detect the actual invoking user and their home and use that instead: set TARGET_USER to SUDO_USER when present (fall back to current user), derive TARGET_HOME from the passwd entry (or fallback to $HOME), then update the agent_dir loop (the for ... if [ -f "$agent_dir/skills/osmo-agent/SKILL.md" ]; then ... OSMO_SKILL_INSTALLED=true ...) to iterate "$TARGET_HOME/.claude" "$TARGET_HOME/.codex" "$TARGET_HOME/.agents" and change the later npx invocation to run as the target user / with HOME set to TARGET_HOME (e.g., sudo -u "$TARGET_USER" or HOME="$TARGET_HOME" before npx) so detection and installation run in the invoking user's context.
149-149:⚠️ Potential issue | 🔴 CriticalExecute
npxas the invoking user, not root.When the installer is run with
sudo, line 149 executesnpxas root, installing the skill to root's home directory instead of the invoking user's. The user's AI coding agents won't be able to access the skill.🔧 Proposed fix to run npx in correct user context
This fix depends on the
TARGET_USERvariable introduced in the previous comment:if [ "$INSTALL_SKILL" = "y" ] || [ "$INSTALL_SKILL" = "Y" ]; then - if npx skills add nvidia/osmo; then + # Run npx as the actual user if invoked via sudo + if [ "$(id -u)" -eq 0 ] && [ -n "${SUDO_USER:-}" ] && [ "$SUDO_USER" != "root" ]; then + INSTALL_CMD=(sudo -u "$SUDO_USER" -H npx skills add nvidia/osmo) + else + INSTALL_CMD=(npx skills add nvidia/osmo) + fi + if "${INSTALL_CMD[@]}"; then echo "" echo "To remove:"Note: This issue was previously flagged and requires the
TARGET_USER/TARGET_HOMEresolution from the first fix to be fully addressed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/packaging/linux/installer.sh` at line 149, The npx call is being executed as root when the installer runs under sudo; change the invocation so npx runs as the invoking user by executing it with that user's context (use the previously-introduced TARGET_USER/TARGET_HOME), e.g. run npx under the invoking user via sudo -u "$TARGET_USER" -H (or runuser -l "$TARGET_USER" --) so the skill is installed into the invoking user's home; update the conditional that runs 'npx skills add nvidia/osmo' to use this user-context execution and ensure TARGET_USER is validated before use.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/cli/packaging/linux/installer.sh`:
- Line 146: The read invocation prompting "Install the OSMO agent skill? [Y/n]:
" (the command that sets INSTALL_SKILL) needs the -r flag to prevent backslashes
in user input being treated as escape sequences; update that read command to
include -r (e.g., use read with -r and preserve the existing prompt and variable
name INSTALL_SKILL) so input is handled safely and ShellCheck SC2162 is
satisfied.
- Around line 127-132: The installer currently uses $HOME which becomes /root
under sudo; detect the actual invoking user and their home and use that instead:
set TARGET_USER to SUDO_USER when present (fall back to current user), derive
TARGET_HOME from the passwd entry (or fallback to $HOME), then update the
agent_dir loop (the for ... if [ -f "$agent_dir/skills/osmo-agent/SKILL.md" ];
then ... OSMO_SKILL_INSTALLED=true ...) to iterate "$TARGET_HOME/.claude"
"$TARGET_HOME/.codex" "$TARGET_HOME/.agents" and change the later npx invocation
to run as the target user / with HOME set to TARGET_HOME (e.g., sudo -u
"$TARGET_USER" or HOME="$TARGET_HOME" before npx) so detection and installation
run in the invoking user's context.
- Line 149: The npx call is being executed as root when the installer runs under
sudo; change the invocation so npx runs as the invoking user by executing it
with that user's context (use the previously-introduced
TARGET_USER/TARGET_HOME), e.g. run npx under the invoking user via sudo -u
"$TARGET_USER" -H (or runuser -l "$TARGET_USER" --) so the skill is installed
into the invoking user's home; update the conditional that runs 'npx skills add
nvidia/osmo' to use this user-context execution and ensure TARGET_USER is
validated before use.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 866b5ff8-0a5b-43cd-ae89-0ff663681ba0
📒 Files selected for processing (2)
src/cli/packaging/linux/installer.shsrc/cli/packaging/macos/postinstall
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/packaging/macos/postinstall
Add an interactive prompt to the macOS postinstall and Linux installer scripts that offers to install the OSMO agent skill for AI coding agents via `npx skills add nvidia/osmo`. The prompt: - Only appears in interactive terminals (skipped for MDM, CI) - Only appears when npx is available (prints manual instructions otherwise) - Defaults to yes on enter - Lets npx handle agent selection, scope, and install method interactively - Non-fatal: installation failure does not abort the CLI install Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jiaenren
force-pushed
the
jiaenr/agent-skill-install
branch
from
e993369 to
ec06b89
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #841 +/- ##
==========================================
- Coverage 42.82% 42.64% -0.19%
==========================================
Files 203 203
Lines 27123 27123
Branches 7759 7759
==========================================
- Hits 11616 11566 -50
- Misses 15398 15443 +45
- Partials 109 114 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
cypres
approved these changes
Apr 9, 2026
ethany-nv
approved these changes
Apr 9, 2026
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
3 participants
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.
Description
Add an interactive prompt to the CLI install/upgrade scripts that offers
to install the OSMO agent skill via
npx skills add nvidia/osmo.When a user installs or upgrades the OSMO CLI, the post-install script
asks if they'd like to install the agent skill for their AI coding agents
(Claude Code, Codex, etc.). The
npx skillsCLI handles agent selection,scope (global/project), and install method (symlink/copy) interactively.
Behavior:
~/.claude,~/.codex, or~/.agents)npxis available: prompts to install (defaults to yes on enter)npxis not available: shows Node.js install instructions (brew install nodeif brew available, plus nodejs.org link)Caveat: Project-scope skill installations (installed into a specific project directory rather than globally) are not detected by the install script, since the script runs from a temporary location with no knowledge of the user's project directories. Users with only project-scope installations will be prompted again on CLI upgrade.
Files:
postinstall(macOS) — added agent skill prompt blockinstaller.sh(Linux) — added identical agent skill prompt blockIssue - None
Checklist
Test
Manually verified on Macbook locally