Skip to content

fix(uninstall): delete the scoped sandbox before removing its unit (#8220) - #8259

Merged
jyaunches merged 26 commits into
NVIDIA:mainfrom
harjothkhara:oss-find/nemoclaw-2026-08-04
Aug 6, 2026
Merged

fix(uninstall): delete the scoped sandbox before removing its unit (#8220)#8259
jyaunches merged 26 commits into
NVIDIA:mainfrom
harjothkhara:oss-find/nemoclaw-2026-08-04

Conversation

@harjothkhara

@harjothkhara harjothkhara commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

A gateway-scoped nemoclaw uninstall removed the selected gateway's marked Linux unit before deleting its sandbox through that gateway.
If sandbox deletion failed, later cleanup stopped and the sandbox container remained.

This change deletes the selected gateway's OpenShell resources first.
It removes the marked Linux unit only after OpenShell cleanup succeeds.
Full uninstall behavior is unchanged.

Related Issue

Fixes #8220

Changes

  • Move marked Linux unit removal for a gateway-scoped uninstall from Stopping services to OpenShell resources, after selected-sandbox cleanup succeeds.
  • Keep the marked Linux unit and gateway process running when selected-sandbox cleanup fails so the user can inspect the remaining state and retry.
  • Centralize the default-port, --keep-openshell, and external-supervision checks shared by full and gateway-scoped cleanup.
  • Add regressions for successful cleanup ordering and failed selected-sandbox cleanup.
  • Document the ordering, partial-completion risk, preserved state, gateway inspection, and gateway-port-preserving retry.

Type of Change

  • 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)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • 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: Nine-category security review passed with no findings for PR commit c8544155016243c47ed24fc289f54e94da90dadd against base SHA db31c286129e878c3356eed49f76ab259561e47e.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: No waiver. GitHub Actions is running the required checks for PR commit c8544155016243c47ed24fc289f54e94da90dadd.

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-updated
  • Evidence: Reviewed all four effective changed files at PR commit c8544155016243c47ed24fc289f54e94da90dadd against base SHA db31c286129e878c3356eed49f76ab259561e47e. The two changed documentation pages accurately describe cleanup order, failure preservation, possible partial completion, gateway inspection, and retry with the failed gateway port for the shared OpenClaw, Hermes, and LangChain Deep Agents Code uninstall path. The implementation and regression tests match that contract. The normal commit hook passed. Local tests were not run; GitHub Actions is authoritative after the PR commit is pushed.
  • Agent: Codex Desktop independent documentation writer; Writing Style Guide path

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh does not change.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub — all 25 commits appear as Verified.
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable.
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — GitHub Actions is running for PR commit c8544155016243c47ed24fc289f54e94da90dadd.
  • Applicable broad gate passed — GitHub Actions is running for PR commit c8544155016243c47ed24fc289f54e94da90dadd.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — the authoritative Fern Preview check is running.
  • Doc pages follow the style guide (doc changes only) — independent documentation writer review passed.
  • New doc pages include SPDX header and frontmatter (new pages only) — not applicable; no new page.

Security Review

Verdict

PASS with no findings.
Reviewed PR commit c8544155016243c47ed24fc289f54e94da90dadd against base SHA db31c286129e878c3356eed49f76ab259561e47e.
The change corrects sequencing in the existing supported uninstall lifecycle.
It does not create a product surface or require a product, business, or architecture decision.
This security verdict does not waive repository merge gates.

Findings Table

No findings.

Detailed Analysis

  1. Secrets and Credentials — PASS. No secret, credential, sensitive value, or sensitive logging behavior changes.
  2. Input Validation and Data Sanitization — PASS. Existing gateway selection and argv-based OpenShell command construction remain unchanged. No shell evaluation, parser, or path surface is added.
  3. Authentication and Authorization — PASS. Cleanup remains constrained by gateway selection, default-port scope, managed-unit markers, no-follow file handling, --keep-openshell, and lifecycle authority.
  4. Dependencies and Third-Party Libraries — PASS. No dependency, lockfile, container image, download, registry, or provenance change is present.
  5. Error Handling and Logging — PASS. Sandbox-deletion and gateway-registration failures return nonzero before unit or process removal. Diagnostics disclose no secret material and document possible partial completion.
  6. Cryptography and Data Protection — PASS / not applicable. No cryptographic behavior changes. Preserved retry state prevents the selected sandbox from being abandoned after gateway shutdown.
  7. Configuration and Security Headers — PASS. Configuration ownership checks remain in force. Upstream, foreign, symlinked, and externally supervised resources remain preserved. Full-uninstall conditions are unchanged.
  8. Security Testing — PASS. Tests cover cleanup ordering, successful container cleanup, sandbox-deletion failure, and gateway-registration failure. GitHub Actions is running these tests for the PR commit.
  9. System Security — PASS. The selected OpenShell gateway remains available until its OpenShell resources are removed. Failure preserves an inspectable retry path and sibling gateway isolation.

Files Reviewed

  • docs/manage-sandboxes/uninstall-nemoclaw.mdx
  • docs/reference/commands.mdx
  • src/lib/actions/uninstall/run-plan-gateway-service.test.ts
  • src/lib/actions/uninstall/run-plan.ts

GitHub Actions

GitHub Actions is authoritative for PR commit c8544155016243c47ed24fc289f54e94da90dadd against base SHA db31c286129e878c3356eed49f76ab259561e47e.
The new checks are running.
No required check has a maintainer waiver.


Signed-off-by: harjoth harjoth.khara@gmail.com

Summary by CodeRabbit

  • Bug Fixes

    • Selected sandbox resources are now removed before stopping the gateway service.
    • Failed cleanup exits with an error while preserving the gateway service for retry.
    • Improved cleanup behavior for default gateway services during full and scoped uninstalls.
    • Cleanup now reports when partial removal may have occurred.
  • Documentation

    • Added guidance on cleanup ordering, failure handling, partial cleanup, inspecting gateways, and retrying uninstall operations.

@copy-pr-bot

copy-pr-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Gateway-scoped uninstall now deletes OpenShell resources before removing the managed Linux service. Tests cover cleanup ordering, Docker cleanup, service removal, and failure handling. Documentation describes partial cleanup and retry guidance.

Changes

Gateway-scoped uninstall

Layer / File(s) Summary
Defer managed service removal
src/lib/actions/uninstall/run-plan.ts
Service-removal conditions are centralized. Scoped cleanup keeps the service active during OpenShell deletion and removes it afterward.
Validate and document cleanup behavior
src/lib/actions/uninstall/run-plan-gateway-service.test.ts, docs/manage-sandboxes/uninstall-nemoclaw.mdx, docs/reference/commands.mdx
Tests verify selected sandbox deletion order, Docker cleanup, service removal, and failure preservation. Documentation describes nonzero exits, partial cleanup, and retry guidance.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Uninstall as Uninstall run plan
  participant OpenShell
  participant Docker
  participant Systemd as Managed Linux service
  Uninstall->>OpenShell: Delete selected sandbox resources
  OpenShell->>Docker: Remove sandbox container
  OpenShell-->>Uninstall: Report cleanup result
  Uninstall->>Systemd: Remove managed service
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #8220 by deleting the selected sandbox before service removal and preserving state when cleanup fails.
Out of Scope Changes check ✅ Passed The code, tests, and documentation changes directly support the scoped-uninstall fix and its failure handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main scoped-uninstall change: deleting the sandbox before removing its service unit.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported 1 fewer blocker, the same number of warnings, the same number of suggestions.
1 additional E2E selection from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • sandbox-survival: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

3 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • established — gateway-scoped cleanup at docs/manage-sandboxes/uninstall-nemoclaw.mdx:73: Keep the established term.
  • established — marked Linux unit at docs/manage-sandboxes/uninstall-nemoclaw.mdx:73: Keep the established term.
  • established — OpenShell resources at docs/manage-sandboxes/uninstall-nemoclaw.mdx:73: Keep the established term.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: None

2 optional E2E recommendations
  • concurrent-gateway-ports
  • sandbox-operations

Blockers

PRA-1 Blocker — Preserve the gateway process when scoped unit removal fails

  • Location: src/lib/actions/uninstall/run-plan.ts:1891
  • Category: correctness
  • Problem: Scoped cleanup continues to stop the host gateway process after disabling the marked user unit fails. This breaks the documented retry path because the running gateway can be terminated even though its unit remains for recovery.
  • Impact: A failed systemd unit disable can leave the selected gateway unavailable, so a retry cannot use that gateway to complete scoped cleanup.
  • Fix: Return from the scoped OpenShell-resources step when removeManagedDefaultGatewayUserService fails, before stopping host gateway processes or executing later cleanup steps.
  • Verification: Inspect the scoped branch in executePlan and simulate a nonzero systemctl disable result; confirm no host-process stop call occurs.
  • Test coverage: Add a scoped-uninstall test where systemctl disable --now fails and assert exit code 1, the service file remains, and no host gateway process stop is invoked.
  • Evidence: src/lib/actions/uninstall/run-plan.ts:1889-1901 continues from a false unit-removal result into stopHostGatewayProcessesForUninstall. docs/manage-sandboxes/uninstall-nemoclaw.mdx:75 and docs/reference/commands.mdx:3659 state that this failure preserves the marked Linux unit and gateway process.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@harjothkhara
harjothkhara marked this pull request as ready for review August 4, 2026 20:46
…ure (NVIDIA#8220)

Signed-off-by: harjoth <harjoth.khara@gmail.com>
@wscurran wscurran added area: install Install, setup, prerequisites, or uninstall flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression labels Aug 4, 2026
@wscurran

wscurran commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@apurvvkumaria apurvvkumaria self-assigned this Aug 5, 2026
@apurvvkumaria

Copy link
Copy Markdown
Collaborator

The current CI failures are classified:

  • static-checks and CLI shard 7 fail only because three architecture budgets are looser than the measured values.
  • The aggregate cli-tests and checks failures follow from that shard.
  • The protected E2E failure is the separate shared artifact-boundary defect tracked by fix(e2e): restore shared CLI artifact boundaries #8333.

A signed, locally validated CI-only fix is prepared to reduce these limits in ci/source-architecture-budget.json:

  • src/lib/state/registry.ts fan-in: 99 to 98
  • src/lib/onboard.ts fan-out: 222 to 219
  • src/lib/onboard root files: 308 to 307

GitHub authorization currently prevents publishing that commit to the contributor branch. The branch owner or a maintainer with working contributor-fork authorization must apply those three mechanical reductions. I will resume CI monitoring once the branch is updated.

@senthilr-nv senthilr-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refreshed implementation and focused suite pass on f5f3a33, but one actionable test gap remains from the repository PR Advisor. The new regression covers sandbox-delete failure only. Please add a scoped-uninstall case where openshell sandbox delete succeeds, openshell gateway delete fails, and assert exit 1, the marked Linux unit remains, and no systemctl disable call occurs. That pins the same retry-preservation invariant at the second failure boundary before service removal. Product scope, nine-category security review, exact-head docs review, type-check, repository checks, docs build, and current focused behavior otherwise pass.

…A#8220)

The scoped-uninstall regression covered only sandbox-delete failure. Add the
second cleanup boundary: sandbox delete succeeds, gateway registration removal
fails, and uninstall still returns before it removes the marked Linux unit.
@harjothkhara

Copy link
Copy Markdown
Contributor Author

Added in 4c4452096: scoped uninstall where openshell sandbox delete succeeds and openshell gateway remove fails. Asserts exit 1, the marked Linux unit remains, and no systemctl disable call. Note the command is gateway remove, not gateway delete — that is the second boundary in removeOpenShellResources.

Checked it fails for the right reason: dropping the registration-failure return makes only the new test fail, while the existing sandbox-delete test still passes. So the old coverage did not pin this boundary, as you said.

npx vitest run --project cli src/lib/actions/uninstall/run-plan-gateway-service.test.ts — 13 passed.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

The prior branch-push blocker is resolved. The contributor branch now includes the requested second cleanup-boundary regression at revision 4c4452096, and the three architecture-budget reductions identified earlier are already present in the current base, so no additional CI-only commit is needed.

I vetted the current fork changes as limited to the uninstall implementation, focused tests, and matching documentation, then approved the held ordinary workflows through GitHub's normal maintainer approval path. I am monitoring the fresh DCO, documentation, security, and CI results. The requested-change review is addressed in code but still awaits reviewer clearance.

A release-tag merge freeze is active, so this PR will not be merged until the freeze is explicitly lifted and all repository gates have passed.

@senthilr-nv senthilr-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Product scope PASS: this fixes ordering and retry preservation in the existing supported uninstall lifecycle. Exact head fa8c048 has the unchanged reviewed four-file patch; CLI build, 64 focused scoped-uninstall tests, CLI type-check, repository checks, documentation build and variant review, nine-category security review, cross-issue sweep, and automated review report no actionable findings. Approving per explicit maintainer direction; no auto-merge is being enabled.

@senthilr-nv
senthilr-nv dismissed their stale review August 5, 2026 17:28

Superseded by the exact-head maintainer approval after the requested regression was added and validated.

@cjagwani cjagwani added v0.0.104 and removed v0.0.103 labels Aug 5, 2026

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review — exact head 829a1125ab9383c1fa7ad8b8fd442e3fb95c0758 against base c176af5f08595ed748b4eeac8988c554e180183e: PASS with no findings.

  1. Secrets and credentials — PASS. No secret, credential, environment, or sensitive logging behavior changes.
  2. Input validation and injection resistance — PASS. The existing validated gateway selection and exact OpenShell command construction remain unchanged; no new caller-controlled shell or path interpolation is introduced.
  3. Authentication and authorization — PASS. Destructive work remains scoped to the selected gateway and existing owned-resource guards. The change does not grant a new caller or component deletion authority.
  4. Dependencies and supply chain — PASS. No dependency, lockfile, image, download, or external source change.
  5. Error handling and information exposure — PASS. Failed selected-sandbox or gateway-registration cleanup returns nonzero before marked unit/process removal, preserving inspectable retry state. Diagnostics reveal no secret material.
  6. Cryptography and data protection — PASS / not applicable. No cryptographic or protected-data behavior changes. The reordered operation reduces orphaned sandbox-container risk.
  7. Configuration and infrastructure — PASS. Default-port, --keep-openshell, external-supervision, and managed-unit checks are centralized without broadening their conditions. Full uninstall behavior is unchanged.
  8. Security testing — PASS. The exact changed-behavior suite passes 13/13 for successful cleanup ordering and both sandbox-deletion and gateway-registration failure preservation. npm run build:cli, npm run check:diff, and diff hygiene pass; fresh GitHub Actions remains authoritative for the complete head.
  9. System security — PASS. OpenShell resources are removed while the selected gateway service is still available, and privileged service removal occurs only after cleanup succeeds. Retry state remains intact on partial failure.

The signed merge was conflict-free and preserves the effective four-file patch byte-for-byte (raw SHA-256 e2a86769135b14957645b9549be1117f7d56f8547fc6a3e414f860a232ea2ba2; stable patch ID afc2c9ec9528aae2845e00aeee08617a46919f37). Base PR #8371 extracts vLLM model acquisition behind its preserved API and has no interaction with uninstall ordering, gateway selection, unit ownership, or cleanup retry state. Product scope is established by accepted bug #8220. Fresh CI, protected E2E, documentation receipt, and independent post-push approval remain mandatory.

@cjagwani
cjagwani requested review from apurvvkumaria and cv August 5, 2026 18:27
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

Security review — exact head cfc6ce2cda02b9eb7e0f635e92bd8ace3ff92d7f against base 3b208d79e5d3bda4183704145ee5c28d79876ae1: PASS with no findings. The effective patch is safe from the reviewed security perspective, subject to every repository merge gate.

Findings Table

No findings.

Detailed Analysis

  1. Secrets and Credentials — PASS. No secret, credential, environment, or sensitive logging behavior changes.

  2. Input Validation and Data Sanitization — PASS. The existing validated gateway selection and exact OpenShell command construction remain unchanged; no new caller-controlled shell or path interpolation is introduced.

  3. Authentication and Authorization — PASS. Destructive work remains scoped to the selected gateway and existing owned-resource guards. The change does not grant a new caller or component deletion authority.

  4. Dependencies and Third-Party Libraries — PASS. No dependency, lockfile, image, download, or external source change.

  5. Error Handling and Logging — PASS. Failed selected-sandbox or gateway-registration cleanup returns nonzero before marked unit/process removal, preserving inspectable retry state. Diagnostics reveal no secret material.

  6. Cryptography and Data Protection — PASS / not applicable. No cryptographic or protected-data behavior changes. The reordered operation reduces orphaned sandbox-container risk.

  7. Configuration and Security Headers — PASS. Default-port, --keep-openshell, external-supervision, and managed-unit checks are centralized without broadening their conditions. Full uninstall behavior is unchanged.

  8. Security Testing — PASS. The exact changed-behavior suite passes 13/13 for successful cleanup ordering and both sandbox-deletion and gateway-registration failure preservation. npm run build:cli, npm run check:diff, and diff hygiene pass; fresh GitHub Actions remains authoritative for the complete head.

  9. System Security — PASS. OpenShell resources are removed while the selected gateway service is still available, and privileged service removal occurs only after cleanup succeeds. Retry state remains intact on partial failure.

Files Reviewed

  • docs/manage-sandboxes/uninstall-nemoclaw.mdx
  • docs/reference/commands.mdx
  • src/lib/actions/uninstall/run-plan-gateway-service.test.ts
  • src/lib/actions/uninstall/run-plan.ts

Provenance and Required Gates

The signed refresh merge and subsequent signed empty synchronization commit preserve the effective four-file patch byte-for-byte (raw SHA-256 e2a86769135b14957645b9549be1117f7d56f8547fc6a3e414f860a232ea2ba2; stable patch ID afc2c9ec9528aae2845e00aeee08617a46919f37). Base PR #8372 changes the canonical security rubric and PR Review Advisor machinery only; it does not alter any effective PR file or runtime trust boundary. This review independently applies the new rubric.

Fresh CI, protected E2E, documentation receipt, unresolved-review resolution, and independent exact-head approval remain mandatory. This security review waives none of those gates.

cv and others added 3 commits August 5, 2026 12:01
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

Security review — exact head 6697d74454de88c1b7152ff792908ee565004862 against base c6713c9dd17a9682654713ccb6829b8714f1b307: PASS with no findings. The effective patch is safe from the reviewed security perspective, subject to every repository merge gate.

Findings Table

No findings.

Detailed Analysis

  1. Secrets and Credentials — PASS. No secret, credential, environment, or sensitive logging behavior changes.

  2. Input Validation and Data Sanitization — PASS. The existing validated gateway selection and exact OpenShell command construction remain unchanged; no new caller-controlled shell or path interpolation is introduced.

  3. Authentication and Authorization — PASS. Destructive work remains scoped to the selected gateway and existing owned-resource guards. The change does not grant a new caller or component deletion authority.

  4. Dependencies and Third-Party Libraries — PASS. No dependency, lockfile, image, download, or external source change.

  5. Error Handling and Logging — PASS. Failed selected-sandbox or gateway-registration cleanup returns nonzero before marked unit/process removal, preserving inspectable retry state. Diagnostics reveal no secret material.

  6. Cryptography and Data Protection — PASS / not applicable. No cryptographic or protected-data behavior changes. The reordered operation reduces orphaned sandbox-container risk.

  7. Configuration and Security Headers — PASS. Default-port, --keep-openshell, external-supervision, and managed-unit checks are centralized without broadening their conditions. Full uninstall behavior is unchanged. The retry documentation now explicitly preserves the selected gateway port without changing runtime behavior.

  8. Security Testing — PASS. The exact changed-behavior suite passes 13/13 for successful cleanup ordering and both sandbox-deletion and gateway-registration failure preservation. npm run build:cli, npm run check:diff, fresh strict docs validation, and diff hygiene pass; fresh GitHub Actions remains authoritative for the complete head.

  9. System Security — PASS. OpenShell resources are removed while the selected gateway service is still available, and privileged service removal occurs only after cleanup succeeds. Retry state remains intact on partial failure.

Files Reviewed

  • docs/manage-sandboxes/uninstall-nemoclaw.mdx
  • docs/reference/commands.mdx
  • src/lib/actions/uninstall/run-plan-gateway-service.test.ts
  • src/lib/actions/uninstall/run-plan.ts

Provenance and Required Gates

The signed base refresh and attribution-preserving merge include the signed retry-port clarification authored by Carlos Villela. The resulting effective four-file patch (raw SHA-256 f573725c95cdb589edaedce5267df4d6bb7f5b86265510601f9722f1ea92f178; stable patch ID bdf744aee5b3f69d973e7f308608f10fe801f555). Base PR #8383 adds only the v0.0.103 release-note page and does not alter any effective PR file or reviewed runtime trust boundary. Base PR #8372 changes the canonical security rubric and PR Review Advisor machinery only; it does not alter any effective PR file or runtime trust boundary. This review independently applies the new rubric.

Fresh CI, protected E2E, documentation receipt, unresolved-review resolution, and independent exact-head approval remain mandatory. This security review waives none of those gates.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Current advisor retry disposition: both advisor jobs stopped after the primary partial review preserved 0 blockers, 0 warnings, and 0 suggestions. Its three terminology decisions confirmed the established wording and did not identify a semantic problem. The single bounded retry for this branch revision is consumed; no further retry will be requested. All other current checks, approval, documentation, DCO, verification, and security review are complete. The merge freeze remains active.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

Security revalidation — exact head 9830a2b118355afdffc054b962c428581b540e7a against current base 1608281462923ff8282151c7be1a109ee262b4e9: PASS with no findings, subject to every repository merge gate.

Findings Table

No findings.

Detailed Analysis

  1. Secrets and Credentials — PASS. The effective PR patch is unchanged from the prior exact review. The base refresh adds no overlapping secret, credential, environment, or sensitive-logging path.

  2. Input Validation and Data Sanitization — PASS. The reviewed validation and command/path construction are unchanged. The new base delta does not alter an effective PR file.

  3. Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary changed in the PR patch or its integration with the refreshed base.

  4. Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PR #8387 adds verified llama.cpp GGUF acquisition in eight disjoint files and does not change this PR's dependency surface.

  5. Error Handling and Logging — PASS. The reviewed fail-closed behavior and diagnostics are unchanged; the base refresh introduces no overlapping error or logging path.

  6. Cryptography and Data Protection — PASS. The PR's reviewed cryptographic and protected-data behavior is unchanged. No new cross-patch data flow exists.

  7. Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries.

  8. Security Testing — PASS. Raw patch SHA-256 remains f573725c95cdb589edaedce5267df4d6bb7f5b86265510601f9722f1ea92f178 and stable patch ID remains bdf744aee5b3f69d973e7f308608f10fe801f555. Documentation writer revalidation passed for the identical exact patch against the current base; the existing PR documentation remains accurate. Prior targeted test evidence remains applicable; fresh exact-head GitHub Actions is authoritative.

  9. System Security — PASS. The signed refresh is conflict-free, preserves the reviewed patch byte-for-byte, and introduces no new runtime trust-boundary interaction with the disjoint base delta.

Provenance and Required Gates

This exact-head revalidation supplements the full prior nine-category review at #8259 (review). It applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa.

Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

Security review — exact head 3431a02b041ba8bcd7fabd38374af3b9438ae98b against current base b148bc13ee53b7a1f612d07e42275ec732e71877: PASS with no findings, subject to every repository merge gate.

Findings Table

No findings.

Detailed Analysis

  1. Secrets and Credentials — PASS. The effective PR patch is unchanged and adds no secret, credential source, environment exposure, or sensitive logging. The refreshed base introduces no overlapping credential path.

  2. Input Validation and Data Sanitization — PASS. Reviewed validation, command construction, and path handling remain unchanged. The portable-profile base delta has no exact file overlap with this PR.

  3. Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary is widened by the PR patch or its integration with the refreshed base.

  4. Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PRs #8333 and #8376 add disjoint E2E artifact restoration and portable experimental onboarding behavior.

  5. Error Handling and Logging — PASS. Reviewed fail-closed behavior and diagnostics remain unchanged; the signed integrations add no overlapping error or logging path.

  6. Cryptography and Data Protection — PASS. Reviewed cryptographic and protected-data behavior remains unchanged, with no new cross-patch data flow.

  7. Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries. The hidden portable profile does not bypass this PR's authority checks.

  8. Security Testing — PASS. Raw patch SHA-256 remains f573725c95cdb589edaedce5267df4d6bb7f5b86265510601f9722f1ea92f178 and stable patch ID remains bdf744aee5b3f69d973e7f308608f10fe801f555. The prior exact uninstall ordering and partial-failure evidence remains applicable. Exact-head documentation writer revalidation passed; fresh GitHub Actions is authoritative.

  9. System Security — PASS. Both signed refreshes are conflict-free, preserve the effective patch byte-for-byte, and introduce no new runtime trust-boundary interaction. For #7853 specifically, the portable profile supplies onboarding defaults but neither stages messaging credentials nor bypasses registry-driven credential-drift validation.

Provenance and Required Gates

The exact head preserves contributor history through signed merge commits and applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa. This supplements the full prior nine-category review at #8259 (review).

Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Maintainer revalidation is complete for revision c85441550162 on current main (db31c286129e).

  • The refresh is a signed, GitHub-Verified merge of main. The effective PR diff remains the same four reviewed files with unchanged file contents, so contributor attribution and scope are preserved.
  • The PR body includes the contributor DCO declaration, all 25 commits are GitHub Verified, the approval remains valid, and there are no review threads.
  • The existing documentation-writer and nine-category security reviews remain applicable because the two documentation pages, implementation, regression test, and repository instructions are unchanged by the refresh. Documentation, CodeQL, and security checks pass.
  • Ordinary CI passes, including all eight CLI shards, build/type checks, static checks, package audit, installer integration, and the final rollups.
  • Both advisor lanes completed with high confidence, zero findings, and merge_as_is: advisor run.
  • The trusted E2E controller evaluated this revision after ordinary CI passed. It selected no credentialed live target; both E2E gate checks pass.

No code, documentation, compliance, security, CI, or review follow-up is required for this revision. Merge remains deferred under the active release-tag freeze; no merge or auto-merge action was taken.

@jyaunches
jyaunches merged commit 6bb4920 into NVIDIA:main Aug 6, 2026
43 of 45 checks passed
@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Post-merge follow-up: #8416 addresses the advisor finding identified after this PR merged. If scoped Linux unit disablement fails, uninstall now returns immediately so the managed service and gateway process remain available for inspection and retry. The 43 focused tests and npm run validate:pr pass locally; #8416 will still require every normal repository check and review gate before merge.

@cjagwani cjagwani mentioned this pull request Aug 7, 2026
23 tasks
cjagwani added a commit that referenced this pull request Aug 7, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical dated changelog entry required before cutting
`v0.0.104`.
The entry reconciles user-facing changes merged from `v0.0.103` through
`8d2b86aaf44968b4f7bc3b714222a73bd28e0403` while excluding hidden and
experimental product surfaces.

## Changes

- Added `docs/changelog/2026-08-06.mdx` with the exact `## v0.0.104`
heading and release themes for local inference, private endpoints,
network policy, state authority, lifecycle recovery, uninstall, Hermes,
MCP diagnostics, credential safety, and installation guidance.
- Source summary links:
- [#8399](#8399) ->
`docs/changelog/2026-08-06.mdx`: fixed DGX Spark local serving profiles.
- [#8418](#8418) ->
`docs/changelog/2026-08-06.mdx`: durable llama.cpp lifecycle management.
- [#8422](#8422) ->
`docs/changelog/2026-08-06.mdx`: recoverable llama.cpp receipt
publication.
- [#8402](#8402) ->
`docs/changelog/2026-08-06.mdx`: remediable DGX Spark storage admission.
- [#8391](#8391) ->
`docs/changelog/2026-08-06.mdx`: host-local serving recipe contracts.
- [#8401](#8401) ->
`docs/changelog/2026-08-06.mdx`: serving profile lifecycle provenance.
- [#8322](#8322) ->
`docs/changelog/2026-08-06.mdx`: guarded llama.cpp route compatibility.
- [#8272](#8272) ->
`docs/changelog/2026-08-06.mdx`: explicitly trusted private endpoints
with stable policy pins and CA trust.
- [#8431](#8431) ->
`docs/changelog/2026-08-06.mdx`: Personal onboarding policy tier and its
trust boundary.
- [#8143](#8143) ->
`docs/changelog/2026-08-06.mdx`: manifest-derived state authority.
- [#7859](#7859) ->
`docs/changelog/2026-08-06.mdx`: side-effect-free lifecycle lock
timeouts.
- [#8262](#8262) ->
`docs/changelog/2026-08-06.mdx`: managed gateway lease waiting.
- [#8339](#8339) ->
`docs/changelog/2026-08-06.mdx`: continued journaled rebuild recreation.
- [#8373](#8373) ->
`docs/changelog/2026-08-06.mdx`: restore readiness after compatibility
decisions.
- [#8443](#8443) ->
`docs/changelog/2026-08-06.mdx`: fail-closed malformed registry
handling.
- [#8419](#8419) ->
`docs/changelog/2026-08-06.mdx`: bounded recovery for a gateway that
never served.
- [#8486](#8486) ->
`docs/changelog/2026-08-06.mdx`: target-scoped registry recovery.
- [#8259](#8259) ->
`docs/changelog/2026-08-06.mdx`: scoped uninstall ordering and retry
safety.
- [#8457](#8457) ->
`docs/changelog/2026-08-06.mdx`: desktop metadata exclusion during
uninstall.
- [#8026](#8026) ->
`docs/changelog/2026-08-06.mdx`: typed Hermes configuration policy.
- [#8242](#8242) ->
`docs/changelog/2026-08-06.mdx`: Hermes WhatsApp session diagnostics.
- [#8344](#8344) ->
`docs/changelog/2026-08-06.mdx`: patched Hermes image and dependency
checks.
- [#8491](#8491) ->
`docs/changelog/2026-08-06.mdx`: bounded MCP discovery timeout.
- [#8490](#8490) ->
`docs/changelog/2026-08-06.mdx`: MCP shadow diagnostics.
- [#7619](#7619) ->
`docs/changelog/2026-08-06.mdx`: web-search credential isolation.
- [#8476](#8476) ->
`docs/changelog/2026-08-06.mdx`: stable preflight advisory identifiers.
- [#8452](#8452) ->
`docs/changelog/2026-08-06.mdx`: user-local CLI resolution.
- [#8481](#8481) ->
`docs/changelog/2026-08-06.mdx`: remote network-policy terminal
guidance.
- Product-scope exclusions:
[#8429](#8429) remains
experimental; [#8261](#8261)
remains feature-gated; and portable-profile changes
[#8408](#8408),
[#8415](#8415),
[#8446](#8446),
[#8458](#8458),
[#8462](#8462), and
[#8506](#8506) are not promoted
as supported product surfaces.

## 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: `npx vitest
run test/changelog-docs.test.ts` passed 6/6 and validates dated
changelog structure 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-08-06.mdx`; release-range scope,
writing rules, documentation style, skip terms, exact names,
threat-boundary wording, and published routes reviewed; changelog tests
and docs build passed.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 02b51ae -->
<!-- docs-review-agents-blob-sha: c69aad4 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; no DGX Station host preparation script
changed.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## 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 validate:pr` passed after refreshing `origin/main` 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.
- [ ] 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 a single
changelog entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [x] `npm run docs` builds without warnings (doc changes only)
- [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)

The new dated changelog file includes the required parser-safe SPDX
header and intentionally has no frontmatter, matching the changelog
contract and existing entries.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.104.
* Documented fixes for local model runtimes, private endpoints, network
policies, state recovery, uninstall behavior, safety updates, MCP
diagnostics, credential isolation, and installation guidance.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: install Install, setup, prerequisites, or uninstall flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression release-target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 26.04][Install] scoped uninstall strands the sandbox container and exits 1 when a sibling gateway is present

7 participants