fix(onboard): make installer and session agent-aware#1721
Conversation
The installer's print_done() hardcoded "Your OpenClaw Sandbox is live"
and "openclaw tui" regardless of which agent was onboarded. Now reads
the agent from the onboard session and adjusts messaging accordingly.
Also adds the missing agent_setup step to defaultSteps() in
onboard-session.ts — without it, markStepStarted("agent_setup")
silently no-ops because the step key doesn't exist in the session.
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughInstaller script now resolves the selected agent from the onboard session and adjusts onboarding output and extraction behavior accordingly; the onboarding session default steps gained an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Banner subtitle said "Launch OpenClaw" and install spinner said "Preparing OpenClaw package" regardless of agent. Now uses NEMOCLAW_AGENT env var to show the correct agent name in the banner, and renames the spinner to "Preparing agent dependencies". Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
The installer ran pre_extract_openclaw() and showed "Preparing OpenClaw package" even when onboarding a non-OpenClaw agent like Hermes. Now guards the pre-extraction step behind a NEMOCLAW_AGENT check — only runs for the default openclaw agent. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
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 `@scripts/install.sh`:
- Line 214: The `${var^}` uppercase expansion is Bash 4+ and fails on Bash 3.2;
replace uses of `${NEMOCLAW_AGENT^}` and the value returned from
resolve_onboarded_agent() with a portable transformation that uppercases only
the first character using substring + tr: compute first="${var:0:1}",
rest="${var:1}", first_upper="$(tr '[:lower:]' '[:upper:]' <<<"$first")" and
then use "${first_upper}${rest}" in place of `${var^}` (apply this change where
NEMOCLAW_AGENT is printed and where resolve_onboarded_agent() result is used).
- Around line 178-191: The script uses Bash 4+ specific parameter expansion
`${var^}` (occurrences of `${var^}` in this file) which breaks on macOS's
default Bash 3.2; update the script to either (A) avoid `${var^}` and convert to
a portable transform (e.g., use `tr '[:lower:]' '[:upper:]'` or `awk` when
uppercasing variables) for all places that uppercase values, or (B) add an early
runtime check for bash version (inspect BASH_VERSINFO or `bash --version`) and
print a clear error telling users to run with Bash 4+ before proceeding; ensure
you change the relevant expansions where used (replace `${var^}` usages) and
keep resolve_onboarded_agent() behavior unchanged.
🪄 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: 24f52a5b-a70f-4d24-ad12-50e4e947b68f
📒 Files selected for processing (1)
scripts/install.sh
| resolve_onboarded_agent() { | ||
| local session_file="${HOME}/.nemoclaw/onboard-session.json" | ||
| if [[ -f "$session_file" ]] && command_exists node; then | ||
| node -e ' | ||
| const fs = require("fs"); | ||
| try { | ||
| const data = JSON.parse(fs.readFileSync(process.argv[1], "utf8")); | ||
| process.stdout.write(data.agent || "openclaw"); | ||
| } catch { process.stdout.write("openclaw"); } | ||
| ' "$session_file" 2>/dev/null || printf "openclaw" | ||
| else | ||
| printf "openclaw" | ||
| fi | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether onboarding persists `session.agent` before install.sh consumes it.
rg -nP --type=ts -C3 'markStep(Start|Complete)\("agent_setup"'
rg -nP --type=ts -C2 '\bagent\s*:'Repository: NVIDIA/NemoClaw
Length of output: 7820
🏁 Script executed:
# Verify how agent is persisted in session and what markStepComplete does
rg -nP --type=ts 'sandboxConfig\s*=' -A 5 src/lib/agent-onboard.ts
rg -nP --type=ts 'markStepComplete' -B 2 -A 2 src/lib/
rg -nP --type=ts 'const markStepComplete' -A 15 src/lib/onboard-session.tsRepository: NVIDIA/NemoClaw
Length of output: 6722
🏁 Script executed:
# Get the full markStepComplete function implementation
sed -n '463,500p' src/lib/onboard-session.tsRepository: NVIDIA/NemoClaw
Length of output: 1292
🏁 Script executed:
# Get the filterSafeUpdates function to see what fields are whitelisted
rg -nP --type=ts 'function filterSafeUpdates|const filterSafeUpdates' -A 30 src/lib/onboard-session.tsRepository: NVIDIA/NemoClaw
Length of output: 1877
🏁 Script executed:
# Check if agent is ever passed to markStepComplete or other session updates
rg -nP --type=ts 'markStepComplete|completeSession|updateSession' -B 2 -A 2 | grep -E '(agent|markStepComplete|completeSession)' | head -50Repository: NVIDIA/NemoClaw
Length of output: 2808
🏁 Script executed:
# Check where agent is actually written to the session
# Look for any direct assignment or explicit passes to session updates
rg -nP --type=ts 'session\.agent\s*=' -B 2 -A 2
rg -nP --type=ts '\bagent:' src/lib/onboard.ts -B 1 -A 1 | head -40Repository: NVIDIA/NemoClaw
Length of output: 319
🏁 Script executed:
# Check if agent field is ever persisted to session anywhere
# by searching for direct updates that might bypass filterSafeUpdates
rg -nP --type=ts 'agent.*null|agent.*string' src/lib/onboard-session.ts
rg -nP --type=ts 'saveSession|updateSession' src/lib/agent-onboard.tsRepository: NVIDIA/NemoClaw
Length of output: 279
🏁 Script executed:
# Check the context of line 4328 where s.agent is assigned
sed -n '4320,4340p' src/lib/onboard.tsRepository: NVIDIA/NemoClaw
Length of output: 741
🏁 Script executed:
# Verify the Bash version requirement for parameter expansion ${var^}
# Check what shell directive or version requirement the scripts use
head -5 scripts/install.sh
# Also check if there are any shell version comments or tests
rg -nP '#!/bin/bash|bash.*version|Bash' scripts/install.shRepository: NVIDIA/NemoClaw
Length of output: 614
🏁 Script executed:
# Verify if the agent field persists through the full onboarding by checking updateSession
# and whether agent is preserved in the in-memory session object
rg -nP --type=ts 'updateSession.*function|function updateSession' -A 20 src/lib/onboard-session.ts | head -30Repository: NVIDIA/NemoClaw
Length of output: 836
🏁 Script executed:
# Check where ${var^} syntax is used in the install.sh
rg -nP '\$\{[^}]+\^' scripts/install.shRepository: NVIDIA/NemoClaw
Length of output: 258
🏁 Script executed:
# Verify that updateSession actually persists the agent field to file
# by checking saveSession
rg -nP --type=ts 'function saveSession|const saveSession' -A 10 src/lib/onboard-session.tsRepository: NVIDIA/NemoClaw
Length of output: 593
Use Bash 4+ parameter expansion cautiously on macOS, which defaults to Bash 3.2.
Lines 214 and 237 use ${var^} to uppercase variables, which requires Bash 4.0+. On macOS systems, the default bash is version 3.2. Either document the Bash 4+ requirement or use a portable alternative like tr '[:lower:]' '[:upper:]' or awk.
The agent session persistence is sound: the agent field is set at the start of onboarding (src/lib/onboard.ts line 4328) via updateSession() and persists through the session file. The fallback to openclaw in resolve_onboarded_agent() is safe.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/install.sh` around lines 178 - 191, The script uses Bash 4+ specific
parameter expansion `${var^}` (occurrences of `${var^}` in this file) which
breaks on macOS's default Bash 3.2; update the script to either (A) avoid
`${var^}` and convert to a portable transform (e.g., use `tr '[:lower:]'
'[:upper:]'` or `awk` when uppercasing variables) for all places that uppercase
values, or (B) add an early runtime check for bash version (inspect
BASH_VERSINFO or `bash --version`) and print a clear error telling users to run
with Bash 4+ before proceeding; ensure you change the relevant expansions where
used (replace `${var^}` usages) and keep resolve_onboarded_agent() behavior
unchanged.
| printf "\n" | ||
| printf " ${C_DIM}Launch OpenClaw in an OpenShell sandbox.%s${C_RESET}\n" "$version_suffix" | ||
| if [[ -n "${NEMOCLAW_AGENT:-}" && "${NEMOCLAW_AGENT}" != "openclaw" ]]; then | ||
| printf " ${C_DIM}Launch %s in an OpenShell sandbox.%s${C_RESET}\n" "${NEMOCLAW_AGENT^}" "$version_suffix" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Bash-4-only uppercase-first expansions and check whether a minimum Bash version is documented.
rg -nP --type=sh '\$\{[^}]*\^\}'
rg -nP -S 'minimum bash|bash 4|bash >= ?4|Bash >= ?4'Repository: NVIDIA/NemoClaw
Length of output: 296
🏁 Script executed:
# Check shebang and context around the problematic lines
head -20 scripts/install.sh | cat -n
# Read the specific problematic lines with context
sed -n '210,220p' scripts/install.sh | cat -n
sed -n '233,242p' scripts/install.sh | cat -n
# Search for Bash version requirements in common places
grep -r "bash" README.md 2>/dev/null | head -20
grep -r "minimum" README.md 2>/dev/null
find . -name ".github" -o -name "Makefile" -o -name "*.yaml" -o -name "*.yml" | head -10Repository: NVIDIA/NemoClaw
Length of output: 2713
🏁 Script executed:
# Check if ShellCheck config allows/disables checks for this pattern
cat .shellcheckrc 2>/dev/null || echo "No .shellcheckrc found"
# Verify if there are any disable directives in install.sh
grep -n "shellcheck" scripts/install.sh || echo "No shellcheck directives found"Repository: NVIDIA/NemoClaw
Length of output: 629
🏁 Script executed:
# Search for version checks or minimum Bash requirements
grep -n "BASH_VERSION\|bash.*version\|version.*bash" scripts/install.sh
# Check README and documentation for Bash requirements
grep -i "bash\|requirement\|require" README.md | head -20
# Check CI/CD workflows for Bash version info
cat .github/workflows/*.yaml 2>/dev/null | grep -i "bash\|shell" | head -20
# Check if there's a shebang or version constraint anywhere
find . -name "*.sh" -type f -exec head -1 {} \; | sort | uniqRepository: NVIDIA/NemoClaw
Length of output: 2184
🏁 Script executed:
# Check GitHub Actions environment details and how bash is invoked
find .github/workflows -name "*.yaml" -o -name "*.yml" | xargs cat 2>/dev/null | grep -A5 -B5 "bash\|shell"Repository: NVIDIA/NemoClaw
Length of output: 6495
🏁 Script executed:
# Check if there are any conditional guards that prevent this code from running
grep -B10 "NEMOCLAW_AGENT\^" scripts/install.sh | head -30
# Check the actual usage of these variables to see if they're always defined
grep -n "NEMOCLAW_AGENT=" scripts/install.sh
grep -n "agent_name=" scripts/install.sh
# Verify the flow - are these code paths actually reachable in a real installation?
grep -B20 "Launch.*OpenShell" scripts/install.sh | tail -25Repository: NVIDIA/NemoClaw
Length of output: 2491
🏁 Script executed:
# Check if there's conditional logic that guards whether this installer even runs on Bash 3.2
grep -n "BASH_VERSINFO\|bash.*version" scripts/install.sh
grep -n "version_check\|check.*version\|minimum" scripts/install.sh | head -10Repository: NVIDIA/NemoClaw
Length of output: 504
🏁 Script executed:
# Get the exact line 237 and surrounding context more precisely
sed -n '235,242p' scripts/install.sh | cat -n
# Verify what resolve_onboarded_agent returns and where agent_name is used
sed -n '233,250p' scripts/install.sh | cat -nRepository: NVIDIA/NemoClaw
Length of output: 1511
🏁 Script executed:
# Test if Bash 4.0+ parameter expansion is really needed or if it's just for formatting
# Check what these variables contain
grep -A5 "NEMOCLAW_AGENT=" scripts/install.sh | head -10
grep -A5 "resolve_onboarded_agent" scripts/install.sh | head -15Repository: NVIDIA/NemoClaw
Length of output: 546
Replace ${var^} parameter expansion with a Bash 3.2-compatible alternative.
Lines 214 and 237 use ${...^} string capitalization, which is Bash 4+ syntax and will throw bad substitution on Bash 3.2. The script uses #!/usr/bin/env bash with no minimum version documented, making it vulnerable on systems with older Bash defaults (e.g., macOS 10.15 and earlier).
These code paths are reachable when:
- Line 214:
NEMOCLAW_AGENTenvironment variable is set to a non-default agent name - Line 237:
resolve_onboarded_agent()returns a non-default agent name
🔧 Portable alternative
+capitalize_first() {
+ local value="$1"
+ [[ -z "$value" ]] && return 0
+ printf '%s%s' "$(printf '%s' "${value:0:1}" | tr '[:lower:]' '[:upper:]')" "${value:1}"
+}
+
if [[ -n "${NEMOCLAW_AGENT:-}" && "${NEMOCLAW_AGENT}" != "openclaw" ]]; then
- printf " ${C_DIM}Launch %s in an OpenShell sandbox.%s${C_RESET}\n" "${NEMOCLAW_AGENT^}" "$version_suffix"
+ printf " ${C_DIM}Launch %s in an OpenShell sandbox.%s${C_RESET}\n" "$(capitalize_first "${NEMOCLAW_AGENT}")" "$version_suffix"
fi
...
- printf " ${C_GREEN}Your %s Sandbox is live.${C_RESET}\n" "${agent_name^}"
+ printf " ${C_GREEN}Your %s Sandbox is live.${C_RESET}\n" "$(capitalize_first "${agent_name}")"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/install.sh` at line 214, The `${var^}` uppercase expansion is Bash 4+
and fails on Bash 3.2; replace uses of `${NEMOCLAW_AGENT^}` and the value
returned from resolve_onboarded_agent() with a portable transformation that
uppercases only the first character using substring + tr: compute
first="${var:0:1}", rest="${var:1}", first_upper="$(tr '[:lower:]' '[:upper:]'
<<<"$first")" and then use "${first_upper}${rest}" in place of `${var^}` (apply
this change where NEMOCLAW_AGENT is printed and where resolve_onboarded_agent()
result is used).
## Summary
- Installer `print_done()` hardcoded "Your OpenClaw Sandbox is live" and
`openclaw tui` regardless of which agent was onboarded. Now reads the
agent from the onboard session and adjusts the completion message
accordingly — non-OpenClaw agents get their own name and no `openclaw
tui` hint.
- Adds missing `agent_setup` step to `defaultSteps()` in
`onboard-session.ts`. Without it, `markStepStarted("agent_setup")`
silently no-ops because the step key doesn't exist in the session,
making the Hermes setup step invisible to resume logic and session
tracking.
## Test plan
- [ ] `NEMOCLAW_INSTALL_REF=main NEMOCLAW_AGENT=hermes curl -fsSL
.../install.sh | bash` — verify completion message says "Your Hermes
Sandbox is live" and does not show `openclaw tui`
- [ ] Same flow with default OpenClaw agent — verify message still says
"Your OpenClaw Sandbox is live" with `openclaw tui`
- [ ] `npm test` passes (1277 tests, 0 failures)
Closes NVIDIA#1618 (partial — installer integration for multi-agent onboard)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Onboarding completion messages now show the configured agent name
(defaults to OpenClaw).
* Onboarding session now tracks an "agent setup" step.
* Next-step instructions are tailored per agent and omitted when not
applicable.
* **Bug Fixes / Improvements**
* Installer skips pre-extraction for non-OpenClaw agents and updates
progress text to reflect preparing agent dependencies.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
## Summary
- Installer `print_done()` hardcoded "Your OpenClaw Sandbox is live" and
`openclaw tui` regardless of which agent was onboarded. Now reads the
agent from the onboard session and adjusts the completion message
accordingly — non-OpenClaw agents get their own name and no `openclaw
tui` hint.
- Adds missing `agent_setup` step to `defaultSteps()` in
`onboard-session.ts`. Without it, `markStepStarted("agent_setup")`
silently no-ops because the step key doesn't exist in the session,
making the Hermes setup step invisible to resume logic and session
tracking.
## Test plan
- [ ] `NEMOCLAW_INSTALL_REF=main NEMOCLAW_AGENT=hermes curl -fsSL
.../install.sh | bash` — verify completion message says "Your Hermes
Sandbox is live" and does not show `openclaw tui`
- [ ] Same flow with default OpenClaw agent — verify message still says
"Your OpenClaw Sandbox is live" with `openclaw tui`
- [ ] `npm test` passes (1277 tests, 0 failures)
Closes NVIDIA#1618 (partial — installer integration for multi-agent onboard)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Onboarding completion messages now show the configured agent name
(defaults to OpenClaw).
* Onboarding session now tracks an "agent setup" step.
* Next-step instructions are tailored per agent and omitted when not
applicable.
* **Bug Fixes / Improvements**
* Installer skips pre-extraction for non-OpenClaw agents and updates
progress text to reflect preparing agent dependencies.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
print_done()hardcoded "Your OpenClaw Sandbox is live" andopenclaw tuiregardless of which agent was onboarded. Now reads the agent from the onboard session and adjusts the completion message accordingly — non-OpenClaw agents get their own name and noopenclaw tuihint.agent_setupstep todefaultSteps()inonboard-session.ts. Without it,markStepStarted("agent_setup")silently no-ops because the step key doesn't exist in the session, making the Hermes setup step invisible to resume logic and session tracking.Test plan
NEMOCLAW_INSTALL_REF=main NEMOCLAW_AGENT=hermes curl -fsSL .../install.sh | bash— verify completion message says "Your Hermes Sandbox is live" and does not showopenclaw tuiopenclaw tuinpm testpasses (1277 tests, 0 failures)Closes #1618 (partial — installer integration for multi-agent onboard)
Summary by CodeRabbit
New Features
Bug Fixes / Improvements