feat(onboard): bind external gateway lifecycle authority#7246
Conversation
Add a versioned gateway lifecycle declaration and durable resume binding. Register the exact endpoint and fail closed across preflight and recovery. Co-authored-by: Souvik Ghosh <138186578+souvikDevloper@users.noreply.github.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
added
platform: brev
|
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:
📝 WalkthroughWalkthroughThis change adds a versioned gateway lifecycle authority contract, validates externally supervised gateways, persists authority across onboarding checkpoints, prevents destructive lifecycle actions, and integrates attachment-only behavior into onboarding, recovery, runtime, and diagnostic output. ChangesGateway lifecycle authority
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Onboarding
participant GatewayManagement
participant GatewayHostRuntime
participant Supervisor
participant OpenShell
Onboarding->>GatewayManagement: Load and validate declaration
GatewayManagement-->>Onboarding: Resolve lifecycle owner
Onboarding->>GatewayHostRuntime: Probe attachment
GatewayHostRuntime->>Supervisor: Check unit, cgroup, and listener identity
Supervisor-->>GatewayHostRuntime: Return runtime evidence
GatewayHostRuntime->>OpenShell: Register and select exact endpoint
OpenShell-->>Onboarding: Return attached gateway
Onboarding->>Onboarding: Checkpoint authority and continue onboarding
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-7246.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit bcbbef7 in the TypeScript / code-coverage/cliThe overall coverage in commit bcbbef7 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/onboard/gateway-ownership.ts (1)
237-238: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc comment overstates what
listenerPidscontains. The externally-supervised probe path feeds this field fromgetGatewayPortListenerRawScan(unfiltered enumeration), not identity-verified gateways — seeprobeGatewayAttachmentingateway-host-runtime.ts, which setslistenerPids: scan.pidsfrom the raw scan. That is intentional and load-bearing: themultiple_ownerscheck at Line 336 must count every listener (e.g. SO_REUSEPORT) to fail closed. The current wording could lead a future maintainer to add identity filtering upstream and silently weaken that guarantee. Consider clarifying the comment to "All processes holding the port; identity is established separately below."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard/gateway-ownership.ts` around lines 237 - 238, Update the doc comment for listenerPids in the gateway ownership model to state that it contains all processes holding the port, including unfiltered listeners, and that identity is established separately. Do not change the raw scan flow or the multiple_owners counting behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/onboard/gateway-authority-checkpoint.test.ts`:
- Around line 61-69: Update the round-trip test “accepts the same authority
after a process resume round-trip (`#6576`)” so it distinguishes a persisted
checkpoint from a reset one, rather than only comparing the returned owner.
Assert an observable outcome specific to the already-recorded branch of
bindGatewayAuthorityToCheckpoint, while retaining the existing same-owner resume
scenario.
In `@src/lib/onboard/gateway-host-runtime.ts`:
- Around line 186-195: The listener identity check in probeGatewayAttachment
must close the PID-reuse gap between reading /proc/<pid>/exe and
/proc/<pid>/cgroup. Revalidate that the PID still represents the originally
scanned listener before accepting its supervisor match, or combine the
executable and cgroup identity reads into one consistent probe; update
readListenerSupervisorMatch or the surrounding probe flow while preserving the
existing attach-gate behavior.
In `@src/lib/state/onboard-checkpoint.ts`:
- Around line 4-9: Move SUPPORTED_GATEWAY_CAPABILITIES out of the onboard layer
into a shared core/domain module such as ports.ts or a dedicated
gateway-capabilities module, then update onboard-checkpoint.ts and
gateway-management.ts to import it from there. Re-export it from
gateway-management.ts to preserve existing onboard callers, while ensuring the
shared module becomes the single source of truth and the state layer no longer
imports ../onboard/gateway-management.
---
Nitpick comments:
In `@src/lib/onboard/gateway-ownership.ts`:
- Around line 237-238: Update the doc comment for listenerPids in the gateway
ownership model to state that it contains all processes holding the port,
including unfiltered listeners, and that identity is established separately. Do
not change the raw scan flow or the multiple_owners counting behavior.
🪄 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: Enterprise
Run ID: 2b375967-eb1f-4218-93a0-a307c5443b57
📒 Files selected for processing (45)
docs/deployment/gateway-lifecycle-authority.mdxdocs/index.ymlsrc/lib/onboard.tssrc/lib/onboard/checkpoint-replay.test.tssrc/lib/onboard/checkpoint-resume-guard.test.tssrc/lib/onboard/dashboard-preflight-ports.test.tssrc/lib/onboard/docker-driver-gateway-port-listener.test.tssrc/lib/onboard/docker-driver-gateway-port-listener.tssrc/lib/onboard/docker-driver-gateway-runtime.tssrc/lib/onboard/gateway-authority-checkpoint.test.tssrc/lib/onboard/gateway-authority-checkpoint.tssrc/lib/onboard/gateway-host-runtime-https.test.tssrc/lib/onboard/gateway-host-runtime.test.tssrc/lib/onboard/gateway-host-runtime.tssrc/lib/onboard/gateway-management.test.tssrc/lib/onboard/gateway-management.tssrc/lib/onboard/gateway-ownership.test.tssrc/lib/onboard/gateway-ownership.tssrc/lib/onboard/gateway-port-conflict.test.tssrc/lib/onboard/gateway-port-conflict.tssrc/lib/onboard/gateway-recovery.test.tssrc/lib/onboard/gateway-recovery.tssrc/lib/onboard/gateway-stale-port-reuse.test.tssrc/lib/onboard/gateway-stale-port-reuse.tssrc/lib/onboard/machine/handlers/gateway.test.tssrc/lib/onboard/machine/handlers/gateway.tssrc/lib/onboard/machine/handlers/sandbox-checkpoint-crash-recovery.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.test.tssrc/lib/onboard/machine/handlers/sandbox.test.tssrc/lib/onboard/machine/initial-flow-phases.test.tssrc/lib/onboard/preflight-gateway-cleanup-decision.test.tssrc/lib/onboard/preflight-gateway-cleanup-decision.tssrc/lib/onboard/preflight-gateway-reuse.test.tssrc/lib/onboard/preflight-gateway-reuse.tssrc/lib/onboard/preflight-gateway-sequence.test.tssrc/lib/onboard/preflight-gateway-sequence.tssrc/lib/onboard/preflight-orphan-gateway-cleanup.test.tssrc/lib/onboard/preflight-orphan-gateway-cleanup.tssrc/lib/onboard/preflight-ports.tssrc/lib/onboard/session-bootstrap.test.tssrc/lib/state/onboard-checkpoint-migrate.test.tssrc/lib/state/onboard-checkpoint-migrate.tssrc/lib/state/onboard-checkpoint-types.tssrc/lib/state/onboard-checkpoint.test.tssrc/lib/state/onboard-checkpoint.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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 (1)
src/lib/onboard/gateway-host-runtime.ts (1)
353-358: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winBind the owner before mutating OpenShell.
attachGatewaytrusts itsownerparameter without comparing it togetGatewayOwner(). A stale resumed owner can therefore register/select its endpoint after the declaration resolves to a different authority. CallbindGatewayOwner(owner)before the early return and before any OpenShell command; add a test proving mismatches fail without registration.Proposed fix
async function attachGateway( owner: GatewayOwner, expectedProbe: GatewayAttachmentProbe, ): Promise<void> { + bindGatewayOwner(owner); if (!isExternallySupervised(owner) || !owner.endpoint) return;As per path instructions, in-scope callers must use one authoritative path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard/gateway-host-runtime.ts` around lines 353 - 358, Update attachGateway to call bindGatewayOwner(owner) as its first operation, before the supervision/endpoint early return and any OpenShell mutation. Ensure mismatched owners fail immediately without registering or selecting an endpoint, and add a test covering this rejection and absence of registration.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/onboard/gateway-host-runtime.ts`:
- Around line 353-358: Update attachGateway to call bindGatewayOwner(owner) as
its first operation, before the supervision/endpoint early return and any
OpenShell mutation. Ensure mismatched owners fail immediately without
registering or selecting an endpoint, and add a test covering this rejection and
absence of registration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: edc6f847-f11e-4d6e-a07e-bb281e13a7ca
📒 Files selected for processing (8)
src/lib/onboard/gateway-host-runtime.test.tssrc/lib/onboard/gateway-host-runtime.tssrc/lib/onboard/gateway-management.test.tssrc/lib/onboard/gateway-management.tssrc/lib/onboard/gateway-ownership.test.tssrc/lib/onboard/gateway-ownership.tssrc/lib/onboard/machine/handlers/gateway.test.tssrc/lib/onboard/machine/initial-flow-phases.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/lib/onboard/gateway-management.test.ts
- src/lib/onboard/machine/initial-flow-phases.test.ts
- src/lib/onboard/gateway-host-runtime.test.ts
- src/lib/onboard/gateway-management.ts
- src/lib/onboard/gateway-ownership.test.ts
- src/lib/onboard/machine/handlers/gateway.test.ts
- src/lib/onboard/gateway-ownership.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
Security re-review: PASS at exact head The delta from the prior nine-category PASS ( |
Advisor finding disposition at
|
|
Exact-head sensitive-path re-review for I rechecked the complete PR diff after the clean |
|
@apurvvkumaria, the requested fixes are now complete on exact head |
|
@apurvvkumaria i also think that your proposed changes are in place check it and i think its mergeable now. Also eagerly waiting for your response regarding joining these NVIDIA team |
|
Exact-head sensitive-path re-review for This review carries forward the complete 68-file security review and re-read the exact six-file delta in this SHA.
No findings. Delta files reviewed: Local exact-head evidence: focused tests 11/11 passed; |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Stale after the blocking teardown-authority gap was fixed in 57ebaf7 and covered by zero-effect tests for stop, final-sandbox cleanup, scoped uninstall, and full uninstall for both systemd supervisor kinds. Issue #6576 now records the accepted design. Current head bcbbef7 is current with main and has green required CI, selected live E2E, CodeRabbit, and both advisor lanes. Apurv remains requested for current-head review.
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical pre-tag `## v0.0.95` release entry to `docs/changelog/2026-07-24.mdx`, before the existing v0.0.94 entry. The entry summarizes approved user-visible changes merged since v0.0.94 and excludes internal-only prerequisites. ## Changes - Adds the v0.0.95 summary and detailed bullets for gateway lifecycle, recovery, state transfer, inference compatibility, sandbox security, Discord policy, and E2E evidence. - Links each user-facing theme to the most specific published documentation. - Records the release entry in the shared native changelog used by the OpenClaw, Hermes, and Deep Agents guides. Source summary: - [#7246](#7246), [#7228](#7228), [#7267](#7267), [#7489](#7489), [#7509](#7509), [#7351](#7351), and [#7290](#7290) -> `docs/changelog/2026-07-24.mdx`: Gateway authority, forward teardown and retry, managed recovery, Hermes restart recovery, scoped uninstall, and orphan-aware backup behavior. - [#7344](#7344) and [#7416](#7416) -> `docs/changelog/2026-07-24.mdx`: Atomic SQLite restore and host download verification. - [#7476](#7476), [#7347](#7347), [#7281](#7281), [#7485](#7485), [#7491](#7491), and [#7422](#7422) -> `docs/changelog/2026-07-24.mdx`: Windows Ollama reuse, CDI fallback, bounded OpenRouter connection setup, Nemotron-3 request compatibility, and managed Deep Agents retry and provider-error behavior. - [#6884](#6884), [#7481](#7481), [#6878](#6878), [#7467](#7467), [#7502](#7502), [#7503](#7503), [#7504](#7504), and [#7486](#7486) -> `docs/changelog/2026-07-24.mdx`: Trusted base-image overrides, local rebuild images, runtime validation, config preservation, reviewed package updates, and fewer final-image payload layers. - [#7303](#7303) -> `docs/changelog/2026-07-24.mdx`: Scoped Discord application-command management. - [#7488](#7488), [#7465](#7465), [#7497](#7497), [#7464](#7464), [#7501](#7501), [#7494](#7494), and [#7493](#7493) -> `docs/changelog/2026-07-24.mdx`: Selected-test risk signals, retry cleanup, full root-image validation, direct-main Hermes setup, executed PR-gate evidence, nightly history, and runner wait reporting. - [#7447](#7447) is an internal pinned-runtime prerequisite and is intentionally excluded from canonical supported-integration documentation. - [#7370](#7370) adds maintainer-only advisory reconciliation tooling and does not change supported user behavior. - [#7495](#7495) updates existing documentation and does not add a new v0.0.95 behavior claim. ## 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 - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog structure, heading uniqueness, and published links. - [ ] Tests not applicable — justification: - [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: `docs/changelog/2026-07-24.mdx`; writing rules, documentation style, factual release meaning, and published links reviewed at exact head `58b02f2bf`. - Agent: Codex documentation writer reviewer <!-- docs-review-head-sha: 58b02f2 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## 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 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) — the build passed with 0 errors and 2 Fern warnings. - [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: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new v0.0.95 changelog entry above v0.0.94. * Documented improved externally supervised gateway lifecycle ownership. * Improved snapshot restore reliability and SQLite state handling. * Tightened CLI `backup-all` behavior and host artifact verification. * Updated Windows onboarding guidance (including Ollama service reuse and CDI directory fallback). * Noted inference compatibility fixes, deeper agent failure classification, stricter base-image validation, updated Discord bot command permissions, and refined E2E release automation evidence handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
6 participants
Summary
External systemd supervisors can now remain the sole OpenShell gateway lifecycle authority while NemoClaw validates and attaches to their exact endpoint. The authority is persisted across onboarding resume, and every preflight, start, recovery, attachment, stop, final-sandbox cleanup, and uninstall path fails closed instead of creating or tearing down the wrong gateway.
Thank you to @souvikDevloper for the original contribution in #6842, and to @apurvvkumaria for the follow-up work. The underlying onboarding architecture shifted to per-gateway bindings and versioned checkpoints while that PR was open, so this needed a fresh implementation. Its central ideas and structure remain here: an explicit authority contract, pure ownership evaluation, host attestation, and composed lifecycle guards.
Related Issue
Refs #6576
Supersedes #6842
Changes
nemoclaw status,status --json, andnemoclaw debug, without the external state directory or credential values.gateway destroyfor them.Type of Change
Quality Gates
Documentation Writer Review
docs-updatedbcbbef70cadds only an empty signed CI-retry commit. Its tree and effective PR documentation diff are identical to reviewed head1bd90ae67.git diff --checkpassed.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run check:diff, and normal commit/pre-push hooks passed.bcbbef70con current base46ecb8677; final coordination passed.npm run docsbuilds without warnings (doc changes only) — passed with 0 errors and 2 pre-existing Fern warningsSigned-off-by: Julie Yaunches jyaunches@nvidia.com
Signed-off-by: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit
status/debugto display redacted gateway authority details.