fix(install): propagate onboard failure exit codes (#5029) - #5044
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughrun_onboard now captures and returns the onboarding CLI exit code via a single status variable across all invocation modes; main treats onboarding failures as fatal and non-interactive preflight failures now hard-error. A Vitest suite verifies exit-code propagation for non-interactive and main flows. ChangesInstaller onboarding exit-code propagation
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 unit tests (beta)
Comment |
|
✨ Thanks for submitting this detailed PR about propagating onboard failure exit codes. This proposes a way to fix the regression in the installer's exit code handling. Related open issues: |
|
CI is green on the latest run (cli-test-shards, installer-integration, static-checks, wsl-e2e, macos-e2e). Ready to merge when you have a moment — completes the installer half of #5029. |
Selective E2E Results — ✅ All requested jobs passedRun: 27432250974
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27432419944
|
|
Ready for merge when the v0.0.66 train allows it.
Thanks for the review — happy to address anything else if needed before merge. |
Return nemoclaw onboard status from run_onboard() on every path and fail the installer when onboarding does not complete. Non-interactive installs also exit non-zero when host preflight blocks onboarding. The connect gated-model half of NVIDIA#5029 is already fixed on main; this completes the installer half only. Signed-off-by: Thabhelo <50872400+Thabhelo@users.noreply.github.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
e451d2e to
d34db08
Compare
|
Rebased onto current |
<!-- markdownlint-disable MD041 --> ## Summary Refreshes the v0.0.68 docs with release notes, Microsoft Teams messaging guidance, and generated support-matrix coverage. Also corrects the Hermes generated command reference so `sessions export` examples match Hermes-only behavior while keeping the shared command heading compatible with CLI parity checks. ## Changes - #5585 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/platform-support.mdx`: Documents experimental Microsoft Teams channel setup, Bot Framework credentials, webhook forwarding, local `MSTEAMS_PORT` conflicts, and the generated integration support row. - #5526 -> `docs/reference/commands.mdx`, `docs/reference/commands-nemohermes.mdx`: Keeps Hermes `sessions export` examples on the supported single-JSONL export path while preserving the canonical shared CLI heading. - #5044 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for installer onboarding failure propagation. - #5641 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for fresh recovery after pre-sandbox installer interruption. - #5673 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for explicit deny-all messaging manifests. - #5743 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for unsupported-agent channel-add rejection. - #5252 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for Hermes `gateway-token` dashboard guidance. - #5659 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for local OpenClaw `agent` wrapper help. - #5661 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for default extra-agent paths. - #5669 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for Deep Agents Code branding. - #5672 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for hosted-compatible default model ID preservation. - #5725 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for Deep Agents Code sandbox liveness. - #5644 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for DGX Spark managed-vLLM express install defaults. - #5712 -> `docs/about/release-notes.mdx`, `docs/reference/platform-support.mdx`: Adds v0.0.68 release-note coverage for the canonical support matrix and updates the matrix source with Teams. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: docs-only release refresh; no runtime code changed. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — `npm run docs` passed with 0 errors; Fern reported the existing light-mode accent contrast warning. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional verification: - `python3 scripts/generate-platform-docs.py --check` passed. - `npm run docs:sync-agent-variants` passed. - `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-cli` passed. - `npm run docs` passed with 0 errors and one Fern theme warning: light-mode accent contrast ratio is 2.41:1 and should be at least 3:1. - `npm run build:cli` refreshed local untracked `dist/` artifacts after rebase; no tracked files changed. - `npm run typecheck:cli` passed. - Normal commit and push hooks passed after the local CLI rebuild. --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added **Microsoft Teams** as an experimental messaging channel, including manifest-first onboarding, local port routing/conflict handling, sandbox delivery controls, and policy preset support. * **Documentation** * Expanded messaging-channel setup for Teams (prerequisites, credential/webhook setup, wizard flow, and add/remove commands). * Updated reference docs for **agent-specific** session export examples (OpenClaw vs Hermes). * Refreshed platform support guidance and added the latest release-notes entry. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…DIA#5044) ## Summary Completes the installer half of NVIDIA#5029. The `nemoclaw connect` gated-model guard already exits 1 on current main (verified by `test/cli/dispatch-basics.test.ts`); this PR fixes installer exit propagation only. ## Related Issue Fixes NVIDIA#5029 (installer half) ## Changes - `run_onboard()`: capture CLI exit status on non-interactive, TTY, and piped-stdin paths; `return` it (same `|| status=$?` pattern used elsewhere in `install.sh`) - `main()`: `run_onboard || error` before setting `ONBOARD_RAN=true` - Non-interactive installs exit non-zero when host preflight blocks onboarding (was warn + exit 0) - Add `test/install-onboard-exit.test.ts` for propagation and main gate behavior ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [x] `npm test` passes (targeted: `test/install-onboard-exit.test.ts`, `test/install-onboard-yes.test.ts`, `test/cli/dispatch-basics.test.ts` gated-model case) - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Thabhelo <50872400+Thabhelo@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Installer now correctly propagates onboarding exit codes in non-interactive runs. * Onboarding failures are now treated as fatal in the main installer flow; interactive mode still warns and skips when host prerequisites are not met. * In non-interactive mode, failing host prerequisites now hard-stops installation with an explicit error. * **Tests** * Added automated coverage to verify onboarding exit-code behavior across success/failure and installer modes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Thabhelo <50872400+Thabhelo@users.noreply.github.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Refreshes the v0.0.68 docs with release notes, Microsoft Teams messaging guidance, and generated support-matrix coverage. Also corrects the Hermes generated command reference so `sessions export` examples match Hermes-only behavior while keeping the shared command heading compatible with CLI parity checks. ## Changes - NVIDIA#5585 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/platform-support.mdx`: Documents experimental Microsoft Teams channel setup, Bot Framework credentials, webhook forwarding, local `MSTEAMS_PORT` conflicts, and the generated integration support row. - NVIDIA#5526 -> `docs/reference/commands.mdx`, `docs/reference/commands-nemohermes.mdx`: Keeps Hermes `sessions export` examples on the supported single-JSONL export path while preserving the canonical shared CLI heading. - NVIDIA#5044 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for installer onboarding failure propagation. - NVIDIA#5641 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for fresh recovery after pre-sandbox installer interruption. - NVIDIA#5673 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for explicit deny-all messaging manifests. - NVIDIA#5743 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for unsupported-agent channel-add rejection. - NVIDIA#5252 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for Hermes `gateway-token` dashboard guidance. - NVIDIA#5659 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for local OpenClaw `agent` wrapper help. - NVIDIA#5661 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for default extra-agent paths. - NVIDIA#5669 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for Deep Agents Code branding. - NVIDIA#5672 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for hosted-compatible default model ID preservation. - NVIDIA#5725 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for Deep Agents Code sandbox liveness. - NVIDIA#5644 -> `docs/about/release-notes.mdx`: Adds v0.0.68 release-note coverage for DGX Spark managed-vLLM express install defaults. - NVIDIA#5712 -> `docs/about/release-notes.mdx`, `docs/reference/platform-support.mdx`: Adds v0.0.68 release-note coverage for the canonical support matrix and updates the matrix source with Teams. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: docs-only release refresh; no runtime code changed. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — `npm run docs` passed with 0 errors; Fern reported the existing light-mode accent contrast warning. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional verification: - `python3 scripts/generate-platform-docs.py --check` passed. - `npm run docs:sync-agent-variants` passed. - `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-cli` passed. - `npm run docs` passed with 0 errors and one Fern theme warning: light-mode accent contrast ratio is 2.41:1 and should be at least 3:1. - `npm run build:cli` refreshed local untracked `dist/` artifacts after rebase; no tracked files changed. - `npm run typecheck:cli` passed. - Normal commit and push hooks passed after the local CLI rebuild. --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added **Microsoft Teams** as an experimental messaging channel, including manifest-first onboarding, local port routing/conflict handling, sandbox delivery controls, and policy preset support. * **Documentation** * Expanded messaging-channel setup for Teams (prerequisites, credential/webhook setup, wizard flow, and add/remove commands). * Updated reference docs for **agent-specific** session export examples (OpenClaw vs Hermes). * Refreshed platform support guidance and added the latest release-notes entry. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Completes the installer half of #5029. The
nemoclaw connectgated-model guard already exits 1 on current main (verified bytest/cli/dispatch-basics.test.ts); this PR fixes installer exit propagation only.Related Issue
Fixes #5029 (installer half)
Changes
run_onboard(): capture CLI exit status on non-interactive, TTY, and piped-stdin paths;returnit (same|| status=$?pattern used elsewhere ininstall.sh)main():run_onboard || errorbefore settingONBOARD_RAN=truetest/install-onboard-exit.test.tsfor propagation and main gate behaviorType of Change
Verification
npx prek run --all-filespassesnpm testpasses (targeted:test/install-onboard-exit.test.ts,test/install-onboard-yes.test.ts,test/cli/dispatch-basics.test.tsgated-model case)npm run docsbuilds without warnings (doc changes only)Signed-off-by: Thabhelo 50872400+Thabhelo@users.noreply.github.com
Summary by CodeRabbit
Bug Fixes
Tests