fix(onboard): clean CLI error on invalid gateway contract (#7627) - #7630
Conversation
An invalid NEMOCLAW_GATEWAY_MANAGEMENT declaration (bad version, unknown field, non-loopback / DNS endpoint, embedded credentials, query string, path, ...) made `nemoclaw onboard` throw an uncaught Node.js exception and print a full stack trace instead of a clean CLI error. The contract loader's consumers threw a bare `Error`, and `runOnboardCommand`'s catch only recognized prompt-cancellation errors, so everything else was re-thrown and escaped to Node's default uncaught-exception printer. Introduce a recognized `GatewayManagementDeclarationError` and a single factory that both consumers (gateway-host-runtime and gateway-teardown-authority) throw, then have the onboard command boundary present it as a clean single-line error and exit nonzero via `fail`. Every rejection reason funnels through one loader result, so one error class covers the whole class of contract-validation failures. `uninstall` already rendered the reason cleanly from `error.message`; keeping the same message means no regression there. Fixes #7627 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
📝 WalkthroughWalkthroughInvalid gateway-management declarations now use a dedicated error type and factory. Gateway runtime and teardown paths throw it, while ChangesGateway declaration error handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GatewayManagement
participant GatewayHostRuntime
participant RunOnboardCommand
participant CLI
GatewayHostRuntime->>GatewayManagement: load gateway-management declaration
GatewayManagement-->>GatewayHostRuntime: invalid declaration reason
GatewayHostRuntime->>RunOnboardCommand: throw GatewayManagementDeclarationError
RunOnboardCommand->>CLI: fail(error.message) with exit code 1
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 11f4976 in the TypeScript / code-coverage/cliThe overall coverage in commit 11f4976 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
prekshivyas
left a comment
There was a problem hiding this comment.
Approved exact head 11f4976 against base a47ddd8. Invalid gateway-management declarations now fail closed with a typed, single-line CLI error; C0/C1 controls, Unicode line separators, and bidirectional controls are escaped with command-boundary regression coverage. The exact-head documentation receipt is current, both selected live E2E jobs passed, required CI and CodeQL are green, both advisors are clean, and no blocking correctness or security findings remain.
<!-- markdownlint-disable MD041 --> ## Summary `docs/changelog/2026-07-25.mdx` now includes the user-facing fixes that merged after #7607 and before the v0.0.96 tag. The follow-up covers safer bulk backup and clone restore behavior, policy and inference repairs, cleaner onboarding diagnostics, and OpenClaw base-image validation while leaving test-only and maintainer-internal merges out of the release entry. ## Changes - Document the Shields-safe `backup-all` flow from #7557 and the clone-specific restore pairing publication from #7608. - Record the Claude Code resolved-launcher policy repair from #7581, Hermes namespaced-model handling from #7604, and persisted Ollama proxy-token reuse from #7620. - Record OpenClaw immutable base-inventory validation from #7606, hidden route-only reservations from #7621, and clean invalid gateway-management errors from #7630. - Link the gateway lifecycle and snapshot authorities, retain #7622's already-merged Docker Engine wording, and exclude internal or test-only merges from the release entry. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This PR changes release-entry prose only. The changelog contract test and Fern validation cover the dated entry, published routes, and rendering requirements. - [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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: At exact PR head `29316da26`, a Codex Desktop documentation writer reviewed `docs/changelog/2026-07-25.mdx` against `AGENTS.md`, `WRITING.md`, and `docs/CONTRIBUTING.md`. The review confirmed that the full entry accurately reflects the merged user-visible behavior, retains #7622's existing wording, appropriately excludes internal and test-only PRs, and uses conforming terminology, structure, links, and release classification. It also confirmed that the review follow-ups use active third-person release-entry voice, name the actor and recovery requirement directly, and accurately preserve the trusted-backup, cached-release refresh, and local-build fallback constraints. The changelog test passed 6/6, and the docs build completed with 0 errors and 2 pre-existing hidden warnings. - Agent: Codex Desktop <!-- docs-review-head-sha: 29316da --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6/6 tests after the final review fix. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this prose-only changelog change. - [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) — exited 0 with 0 errors and 2 pre-existing hidden warnings after the final review fix. - [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) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded the changelog to clarify persistent `policy exclude`/`policy restore` behavior across rebuilds and snapshot restores, including reporting on removed endpoints and exclusion consistency. * Updated `claude-code` preset guidance to allow the npm-installed OpenShell launcher path while maintaining endpoint/HTTP method scope. * Documented hardened handling for invalid gateway-management declarations, improved gateway/agent-version diagnostics scope, and clarified onboarding/restore credential and reasoning precedence. * Tightened bulk backup/restore guidance (safety windows, approval limits, and failure recovery) and refined OpenClaw base selection to avoid incompatible cached releases and `:latest` fallback. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
An invalid
NEMOCLAW_GATEWAY_MANAGEMENTdeclaration madenemoclaw onboardthrow an uncaught Node.js exception and print a full stack trace instead of a clean CLI error. This PR makes the contract-validation rejection a recognized error the onboard command presents as a clean single-line message with a nonzero exit.Closes #7627.
Reproduction
On our Ubuntu 24.04 x86_64 test host (no GPU), built from
main:Environment
mainHEADa47ddd896Observed on
main(before fix) — exit 1 but a raw Node.js stack trace:Observed on
fix/...(after fix) — clean single line, exit 1, no stack trace (verified for the version case and the embedded-credentials case):Analysis
resolveCurrentGatewayOwner(src/lib/onboard/gateway-host-runtime.ts) loads the declaration and, on a validation failure, threw a bareError("Invalid gateway management declaration: ${loaded.reason}"). During onboarding this is called deep in the FSM.runOnboardCommand's catch (src/lib/onboard/command.ts) only recognized prompt-cancellation errors (SIGINT / EOF) and re-threw everything else — so the contract error escaped every handler and reached Node's default uncaught-exception printer (raw stack trace). Every rejection reason (bad version, unknown fields, DNS endpoint, embedded credentials, query string, path) funnels through the singleloaded.reason, so all of them produced the same uncaught throw.Fix
src/lib/onboard/gateway-management.ts: add a recognizedGatewayManagementDeclarationErrorand aninvalidGatewayManagementDeclarationError(reason)factory — the single source of the error type and message.src/lib/onboard/gateway-host-runtime.tsandsrc/lib/onboard/gateway-teardown-authority.ts: throw the recognized error via the factory instead of a bareError.src/lib/onboard/command.ts:runOnboardCommandnow recognizesGatewayManagementDeclarationErrorand prints the reason as a clean single-line CLI error, exiting nonzero via the existingfailhelper — before the generic re-throw, so unrelated errors still surface as genuine failures.Whole-class notes. Every rejection reason routes through one loader result → one error class → covered by the single onboard catch (verified live for the version and embedded-credentials reasons). Other consumers of the same contract:
uninstallalready caught the error and renderederror.messagecleanly (src/lib/actions/uninstall/run-plan.ts) — keeping the same message means no regression; the sandboxdestroygateway-cleanup path now throws the same recognized class and renders via itserror.messagehandling. No new failure/rollback path is introduced (presentation-only). No user-facing docs describe the previous stack-trace behavior, so none need updating.Tests lock: the onboard command prints a clean error (no stack frames) and exits nonzero for a contract error; a non-cancellation, non-gateway error still re-throws (regression lock against over-catching); and the runtime boundary throws the recognized
GatewayManagementDeclarationError.Changes
src/lib/onboard/gateway-management.ts: recognized error class + factory.src/lib/onboard/gateway-host-runtime.ts,src/lib/onboard/gateway-teardown-authority.ts: throw the recognized error.src/lib/onboard/command.ts: present the contract error cleanly at the onboard boundary.src/lib/onboard/command.test.ts,src/lib/onboard/gateway-host-runtime.test.ts: coverage.Type of Change
Documentation Writer Review
no-docs-neededdocs/deployment/gateway-lifecycle-authority.mdxalready documents declaration validation, rejected fields, and recovery. This PR changes error presentation and C0/C1, line-separator, and bidirectional-control escaping without changing configuration or user actions. Comments and test titles identify the command boundary, single-line output, control escaping, and rethrow behavior; no blockingWRITING.mdfindings remain.Verification
npx prek runpasses on the changed filesnpm testpasses (touched files at minimum)AI Disclosure
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
Summary by CodeRabbit