Skip to content

refactor(openshell): remove cluster probe and legacy host aliases - #6817

Closed
ericksoa wants to merge 13 commits into
refactor/openshell-remove-coredns-patch/aefrom
refactor/openshell-remove-cluster-resource-probe/ae
Closed

ericksoa wants to merge 13 commits into
refactor/openshell-remove-coredns-patch/aefrom
refactor/openshell-remove-cluster-resource-probe/ae

Conversation

@ericksoa

@ericksoa ericksoa commented Jul 14, 2026 •

Copy link
Copy Markdown
Member

Summary

Remove the resources command's direct Docker and Kubernetes node probe and NemoClaw's legacy host-alias command surface, leaving cluster and sandbox internals owned by OpenShell. Host CPU, memory, GPU, and profile reporting remain supported, and no private host-mutation fallback is added. This is PR 21 of 21 in the dependent OpenShell gRPC migration stack and targets refactor/openshell-remove-coredns-patch/ae.

Changes

  • Delete the docker exec openshell-cluster-nemoclaw kubectl get nodes resource fallback.
  • Resolve percentage-based resource profiles consistently against host totals and remove Kubernetes allocatable fields from command output and docs.
  • Delete the hosts-add, hosts-list, and hosts-remove commands and their Kubernetes Sandbox resource patch implementation.
  • Remove the retired host-alias commands from public routing metadata, help output, docs, and package-contract tests.
  • Update the reviewed legacy-transport inventory for both removals; no ephemeral /etc/hosts or other private mutation fallback is introduced.

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: Self-review completed by Aaron Erickson; this change deletes direct host-controlled cluster inspection and Kubernetes sandbox-mutation paths and deliberately adds no private replacement.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every authored commit is cryptographically signed; GitHub verification will be checked again at the pushed exact head
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable — npm run check:diff passed against current origin/main
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 125 focused gRPC migration tests passed, plus the runtime-stdin and exit-255 regression sets
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — required CI will supply the broad repository gates at the pushed exact head
  • 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 strict docs build passed with 0 errors; Fern reported 2 configured suppressed warnings
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa self-assigned this Jul 14, 2026
@coderabbitai

coderabbitai Bot commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 557e12a6-a916-44d8-bdf0-e28972a36e91

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/openshell-remove-cluster-resource-probe/ae

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

@github-code-quality

github-code-quality Bot commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the refactor/openshell-r... branch.

TypeScript / code-coverage/cli

The overall coverage in the refactor/openshell-r... branch remains at 80%, unchanged from the refactor/openshell-r... branch.

Show a code coverage summary of the most impacted files.
File refactor/openshell-r... 07432e6 refactor/openshell-r... 595d528 +/-
src/lib/inference/local.ts 82% 69% -13%
src/lib/shields/index.ts 72% 62% -10%
src/lib/inference/vllm.ts 86% 77% -9%
src/lib/policy/index.ts 56% 64% +8%
src/lib/state/sandbox.ts 75% 85% +10%
src/lib/adapter...teway-config.ts 0% 77% +77%
src/lib/adapter...dbox-control.ts 0% 78% +78%
src/lib/state/s...tore-payload.ts 0% 83% +83%
src/lib/adapter...dbox-control.ts 0% 97% +97%
src/lib/adapter...ndbox-upload.ts 0% 99% +99%

Updated July 18, 2026 23:45 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / medium confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

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

Recommended E2E: onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox, cloud-onboard

2 optional E2E recommendations
  • snapshot-commands
  • gateway-health-honest

Workflow run details

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

ericksoa added 9 commits July 13, 2026 17:46
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
<!-- markdownlint-disable MD041 -->
## Summary

Remove NemoClaw's legacy host-alias command surface instead of
preserving its direct Docker and Kubernetes resource mutation. This is
PR 26 in the dependent OpenShell gRPC migration stack and targets
`refactor/openshell-remove-cluster-resource-probe/ae`.

## Changes

- Delete the `hosts-add`, `hosts-list`, and `hosts-remove` commands and
their Kubernetes `Sandbox` resource patch implementation.
- Remove the retired commands from public routing metadata, help output,
docs, and package-contract tests.
- Remove the final ordinary Docker-exec mutation from the reviewed
legacy-transport inventory; no ephemeral `/etc/hosts` fallback is
introduced.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Self-review completed
by Aaron Erickson; this change deletes a host-controlled Kubernetes
mutation path and deliberately adds no private replacement.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration 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: `npm
run build:cli`; 66 focused assertions across CLI adapters, integration
behavior, package contracts, and the legacy-transport inventory pass.
- [ ] 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)
- [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)

`npm run docs` passed; Fern reported the existing two hidden warnings.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa changed the title refactor(resources): remove cluster allocatable probe refactor(openshell): remove cluster probe and legacy host aliases Jul 14, 2026
@wscurran wscurran added area: architecture Architecture, design debt, major refactors, or maintainability area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery refactor PR restructures code without intended behavior change labels Jul 14, 2026
ericksoa added 2 commits July 15, 2026 01:30
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…gRPC (#6911)

<!-- markdownlint-disable MD041 -->
## Summary

Route the remaining direct read-only sandbox probes in snapshot and
process-recovery flows through NemoClaw's centralized OpenShell
gRPC-first adapter. This removes bespoke CLI marker and stream parsing
while retaining the bounded pre-dispatch read-only CLI fallback required
for gateways whose current metadata or edge-tunnel authentication cannot
establish direct gRPC. This is the final cleanup PR in the dependent
OpenShell gRPC migration stack and targets
`refactor/openshell-remove-cluster-resource-probe/ae`.

## Changes

- Route snapshot DCode quiescence checks through
`execSandboxReadOnlyWithGrpcFallback` using the sandbox's persisted
gateway identity.
- Send the multiline DCode probe over validated stdin with `sh -s`,
keeping command arguments control-character-free for both direct gRPC
and the scoped CLI fallback.
- Route process-recovery gateway status probes through the same
gRPC-first boundary.
- Preserve existing timeout and fail-closed behavior for transport
errors, signals, unknown status, malformed output, and invalid gateway
bindings.
- Remove obsolete CLI marker and stream fixture plumbing.
- Keep every remaining reviewed fallback importer explicit in the
legacy-transport inventory; mutation paths are unchanged and never
replay across transports.

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

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: This changes internal probe
transport only; command behavior and user-facing output remain
unchanged.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Self-review completed
by Aaron Erickson; probes are read-only, resolve the persisted gateway
identity, validate the exact request shape, preserve bounded timeouts,
and fail closed. Mutation paths are unchanged.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration 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: `npm
run build:cli`; 88 focused process-recovery and snapshot assertions plus
9 legacy-transport integration assertions passed. After the final stdin
request fix, all 64 focused snapshot assertions reran at the exact head
and the captured production request passed the real centralized
validator.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Exact-head required CI
will supply the broad repository gates.
- [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)
- [ ] 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: Aaron Erickson <aerickson@nvidia.com>

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa

Copy link
Copy Markdown
Member Author

Closing as superseded: NemoClaw will consume the upstream OpenShell TypeScript SDK from NVIDIA/OpenShell#2122 instead of maintaining this raw gRPC/proto transport stack.

@ericksoa ericksoa closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: architecture Architecture, design debt, major refactors, or maintainability area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants