Skip to content

fix(host-aliases): prepend docker exec subcommand for kubectl invocation#4319

Merged
cv merged 2 commits into
mainfrom
fix/4317-hosts-list-docker-exec
May 27, 2026
Merged

fix(host-aliases): prepend docker exec subcommand for kubectl invocation#4319
cv merged 2 commits into
mainfrom
fix/4317-hosts-list-docker-exec

Conversation

@laitingsheng

@laitingsheng laitingsheng commented May 27, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

nemoclaw <sandbox> hosts-list / hosts-add / hosts-remove failed with Failed to read host aliases. unknown shorthand flag: 'n' in -n and exit 125 on real Docker engines because runKubectlInClusterRaw invoked dockerExecFileSync without the exec subcommand. Docker then parsed kubectl's -n openshell as a Docker flag and bailed before any kubectl call could run. This patch prepends "exec" to the argv so the helper actually runs docker exec <k3s-container> kubectl -n openshell ....

Related Issue

Fixes #4317.

Changes

  • src/lib/actions/sandbox/host-aliases.ts: runKubectlInClusterRaw now passes ["exec", K3S_CONTAINER, "kubectl", "-n", "openshell", ...args] to dockerExecFileSync, restoring the intended docker exec ... kubectl ... invocation.
  • test/cli.test.ts: hardened the three host-alias integration tests (hosts-add, hosts-list, hosts-remove). Each test now captures the docker stub argv and asserts log[0] === "exec", so the original PATH-shim fixture (which only printf-ed "$@" to a log) catches the missing subcommand instead of silently absorbing it.

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)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Fixed host-alias listing and removal so CLI invocations now run correctly (ensures Docker command includes the expected exec/kubectl sequence).

  • Tests

    • Strengthened tests to validate the exact command invocation format and logged docker calls for host-alias operations.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7ae13e2b-4f80-4e40-bafe-0a81395f6f96

📥 Commits

Reviewing files that changed from the base of the PR and between 54bbe5d and 5c3ca37.

📒 Files selected for processing (1)
  • test/cli.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/cli.test.ts
📝 Walkthrough

Walkthrough

Restructures the Docker exec invocation in runKubectlInClusterRaw to pass the full command as an array starting with exec, and tightens host-alias CLI tests to assert the Docker invocation begins with exec (the list test also checks for kubectl and get).

Changes

Docker kubectl invocation regression fix

Layer / File(s) Summary
Docker exec command invocation fix
src/lib/actions/sandbox/host-aliases.ts		 runKubectlInClusterRaw now constructs the Docker exec command as a single array starting with exec, the k3s container name, and kubectl, preserving namespace, stdio, and timeout options.
Host-alias regression test coverage
test/cli.test.ts		 Host-alias add/list/remove tests assert the first logged Docker invocation begins with exec. The list test additionally wires per-test docker.log and asserts the logged invocation contains kubectl and a get operation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I clipped my whiskers, hopped to test and fix,
Told Docker "exec" first — no more flag mix,
Kubectl now marches in tidy array,
Host aliases sleep sound at the end of the day. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main fix: prepending the docker exec subcommand to kubectl invocations in host-aliases functionality.
Linked Issues check ✅ Passed The PR successfully addresses issue #4317 by fixing the docker exec invocation so kubectl flags are no longer misparsed by Docker, with code changes and test assertions validating the fix.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the docker exec subcommand issue in host-aliases; no out-of-scope modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/4317-hosts-list-docker-exec

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

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

@github-actions

github-actions Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: sandbox-operations-e2e
Optional E2E: network-policy-e2e

Dispatch hint: sandbox-operations-e2e

Auto-dispatched E2E: sandbox-operations-e2e via nightly-e2e.yaml at 5c3ca37d85033eeb899ce642fec727635045bacb — nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • sandbox-operations-e2e (high): Closest existing live sandbox coverage for CLI-driven sandbox operations and gateway/k3s-backed behavior. This PR changes a runtime sandbox command path that shells into the OpenShell cluster container and patches sandbox resources, so a live sandbox operations E2E should run before merge.

Optional E2E

  • network-policy-e2e (medium): Host aliases are adjacent to sandbox networking/name-resolution behavior. This is useful extra confidence, but the changed code does not alter network policy enforcement itself.

New E2E recommendations

  • sandbox host aliases (high): No existing E2E test appears to exercise nemoclaw <sandbox> hosts-add, hosts-list, and hosts-remove against a real OpenShell k3s sandbox. The regression fixed here—missing docker exec before kubectl -n openshell—is exactly the kind of live Docker/kubectl invocation issue that unit stubs only approximate.
    • Suggested test: Add a live host-alias E2E that onboards a sandbox, runs hosts-add for a test hostname/IP, verifies hosts-list output, confirms the sandbox pod/resource contains the hostAlias or resolves via /etc/hosts, then runs hosts-remove and verifies cleanup.

Dispatch hint

  • Workflow: E2E / Nightly
  • jobs input: sandbox-operations-e2e

@github-actions

github-actions Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions

github-actions Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 0 still apply, 0 new items found

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26515328923
Target ref: 54bbe5d8b66b38dee83939c9d383d74844b59148
Workflow ref: main
Requested jobs: sandbox-operations-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
sandbox-operations-e2e ✅ success

@laitingsheng
test(host-aliases): drop issue ref from regression comment
5c3ca37 
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@laitingsheng laitingsheng added fix v0.0.53 Release target labels May 27, 2026
@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26518843183
Target ref: 5c3ca37d85033eeb899ce642fec727635045bacb
Workflow ref: main
Requested jobs: sandbox-operations-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
sandbox-operations-e2e ✅ success

@cv cv merged commit 49b2a9a into main May 27, 2026
29 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 27, 2026
Closed
12 tasks
@miyoungc miyoungc mentioned this pull request May 27, 2026
Merged
cv pushed a commit that referenced this pull request May 27, 2026
@miyoungc
docs: refresh v0.0.53 release notes (#4360)
da3bb3a 
## Summary
- Add the v0.0.53 release notes with the user-facing onboarding,
inference, policy, runtime, Hermes, and maintainer-tooling changes from
the release range.
- Refresh generated `nemoclaw-user-*` skills from the current Fern docs,
including already-merged policy, inference, troubleshooting, and
command-reference updates.
- Remove skipped experimental shield wording from generated-doc source
so the release-prep skip-term gate stays clean.

## Source summary
- #4197 -> `docs/about/release-notes.mdx`,
`docs/reference/commands.mdx`: Document pre-recreate workspace backup,
abort-on-partial-backup behavior, and
`NEMOCLAW_RECREATE_WITHOUT_BACKUP`.
- #4273 -> `docs/about/release-notes.mdx`,
`docs/reference/troubleshooting.mdx`: Document the under-provisioned
runtime prompt defaulting to abort in interactive onboarding.
- #4220 -> `docs/about/release-notes.mdx`,
`docs/network-policy/customize-network-policy.mdx`,
`docs/network-policy/integration-policy-examples.mdx`: Include the
`openclaw-pricing` preset and generated skill refresh.
- #4253 -> `docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`,
`docs/inference/switch-inference-providers.mdx`: Carry the Ollama
runtime context-window docs into generated skills.
- #4298 -> `docs/about/release-notes.mdx`,
`docs/reference/troubleshooting.mdx`: Carry WSL Docker Desktop GPU
guidance into generated skills and release notes.
- #4297, #4210, #4221, #4225, #4288, #4306, #4311, #4319, #4342, #4284,
#3327 -> `docs/about/release-notes.mdx`: Summarize release-range fixes
and maintainer tooling changes that did not need new standalone docs
pages.

## Verification
- `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix
nemoclaw-user --doc-platform fern-mdx`
- `rg "permissive mode|shields down|shields up|shields status|config
rotate-token|rotate-token" docs .agents/skills` returned no matches
outside `docs/.docs-skip`.
- `npm run docs` passes with full network access. Fern reports 0 errors
and one existing light-mode accent contrast warning.
- `FERN_VERSION=$(node -p "require('./fern/fern.config.json').version")
&& (cd fern && npx --yes "fern-api@${FERN_VERSION}" check --warnings)`
reports 0 errors and the same contrast warning.

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

## Summary by CodeRabbit

* **Documentation**
* Added v0.0.53 release notes with updates to onboarding, sandbox
recreation, and gateway handling
* Introduced `openclaw-pricing` preset for model pricing endpoint
management
* Clarified Ollama context window configuration and local model
validation behavior
* Updated sandbox recreation workflow documentation with backup/restore
details
* Enhanced interactive onboarding defaults for under-provisioned runtime
warnings
* Revised security guidance for configuration directory permissions and
immutability verification

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4360?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@wscurran wscurran added bug-fix PR fixes a bug or regression and removed fix labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cv cv cv approved these changes

Assignees

No one assigned

Labels

bug-fix PR fixes a bug or regression v0.0.53 Release target

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Ubuntu 24.04][CLI] nemoclaw hosts-list fails with Docker unknown shorthand flag

3 participants

@laitingsheng @cv @wscurran