Skip to content

fix(sandbox): reset NPM_CONFIG_OFFLINE after build-time plugin install#4900

Merged
cv merged 4 commits into
mainfrom
fix/4773-runtime-npm-offline
Jun 7, 2026
Merged

fix(sandbox): reset NPM_CONFIG_OFFLINE after build-time plugin install#4900
cv merged 4 commits into
mainfrom
fix/4773-runtime-npm-offline

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Summary

ENV NPM_CONFIG_OFFLINE=true at Dockerfile:585 is scoped to the local OpenClaw plugin-install RUN, but the directive persists into the runtime image. PID 1 in the sandbox therefore exports NPM_CONFIG_OFFLINE=true, which forces every child npm / npx into cache mode: only-if-cached and fails dashboard "Add MCP", skill installers, and ad-hoc npx -y invocations with ENOTCACHED. Reset to false immediately after the plugin install and pin the runtime value from the entrypoint as a backstop.

Related Issue

Fixes #4773.

Changes

  • Dockerfile — reset NPM_CONFIG_OFFLINE=false immediately after the openclaw plugins install RUN so the runtime image inherits false while the build-time isolation around that single RUN is preserved.
  • scripts/nemoclaw-start.sh — add npm_config_offline=false and NPM_CONFIG_OFFLINE=false to _TOOL_REDIRECTS so PID 1 and openshell sandbox connect sessions pin the value regardless of a stale base image or future build-time regression.
  • test/sandbox-provisioning.test.ts — two runtime regression guards: replays every Dockerfile ENV directive and asserts the runtime image ends with NPM_CONFIG_OFFLINE=false; replays the staged subset up to the local plugin-install RUN and asserts the offline lock is still true at that point. Pure runtime assertions — no source-text shape.
  • test/service-env.test.ts — extended emission test asserts the generated proxy-env.sh carries npm_config_offline=false and NPM_CONFIG_OFFLINE=false. Two new tests source the actual _TOOL_REDIRECTS export loop from the entrypoint and re-source the emitted proxy-env.sh in a clean env -i bash --noprofile --norc shell, asserting both spellings resolve to false (PID 1 + sandbox-connect parity).
  • test/fetch-guard-patch-regression.test.ts — narrow the existing test's end marker to # Release the offline lock so it brackets only the plugin-install RUN and ignores the follow-on ENV directive.

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

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

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Scope the build-time npm offline lock to the plugin-install step and reset it for runtime. Dockerfile adds ENV NPM_CONFIG_OFFLINE=false after plugin installation, the entrypoint pins npm online at container start, and tests verify Dockerfile staging and runtime exports.

Changes

NPM Online Runtime Access for MCP Server Installation

Layer / File(s) Summary
Dockerfile: offline lock scope and runtime reset
Dockerfile
Clarifies the offline lock applies only to the immediate OpenClaw plugin install and adds ENV NPM_CONFIG_OFFLINE=false after plugin installation to re-enable runtime npm access.
Entrypoint: pin runtime npm online
scripts/nemoclaw-start.sh
Adds npm_config_offline=false and NPM_CONFIG_OFFLINE=false to _TOOL_REDIRECTS so the container starts with npm in online mode.
Sandbox provisioning tests: Dockerfile ENV parsing and ordering
test/sandbox-provisioning.test.ts
New Vitest suite parses Dockerfile ENV directives (including line continuations), converts them to shell exports, and asserts staged (up to the RUN openclaw plugins install /opt/nemoclaw) value is true and final runtime value is false.
Service env assertions and Dockerfile marker update
test/service-env.test.ts, test/fetch-guard-patch-regression.test.ts
Add integration tests asserting the entrypoint and proxy-env.sh expose npm_config_offline=false and NPM_CONFIG_OFFLINE=false; update Dockerfile patch test boundary marker to match the restructured plugin-install region.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

Docker, Sandbox, fix, v0.0.59

Suggested reviewers

  • cv
  • prekshivyas
  • cjagwani

Poem

🐰 I nudged the lock to just one layer bright,
Then flipped it back so runtime could take flight.
Plugins install safe, then npm can roam,
The sandbox fetches packages and finds a home.
Hoppity tests confirm the path is right.

🚥 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
Title check ✅ Passed The title accurately summarizes the main change: resetting NPM_CONFIG_OFFLINE after the build-time plugin installation step.
Linked Issues check ✅ Passed The pull request implements all required changes to resolve issue #4773: resetting NPM_CONFIG_OFFLINE in the Dockerfile, adding offline mode locks to the entrypoint, and adding comprehensive tests to verify runtime behavior.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the NPM_CONFIG_OFFLINE persistence issue; no out-of-scope modifications detected.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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/4773-runtime-npm-offline

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

@laitingsheng laitingsheng added integration: openclaw OpenClaw integration behavior bug-fix PR fixes a bug or regression labels Jun 7, 2026
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: full, test-network-policy.sh, openclaw-plugin-runtime-exdev-e2e
Optional E2E: test-openclaw-skill-cli-e2e.sh, onboard-inference-smoke-e2e

Dispatch hint: openclaw-plugin-runtime-exdev-e2e,onboard-inference-smoke-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • full (Brev CPU, about 10 minutes): Runs the complete install → onboard → sandbox verification → live inference journey, building/using the Dockerfile and exercising the entrypoint, OpenClaw plugin registration, policy setup, and runtime sandbox health.
  • test-network-policy.sh (High; live Docker sandbox and network-policy suite): High-signal coverage for runtime egress and package-manager behavior. It verifies policy changes in a live sandbox and includes npm reachability checks under permissive policy, which should catch a leaked NPM_CONFIG_OFFLINE=true runtime environment.
  • openclaw-plugin-runtime-exdev-e2e (Medium-high; fresh sandbox from Dockerfile, up to 45 minutes): Focused regression around OpenClaw plugin runtime dependencies in fresh Dockerfile-built sandboxes. The Dockerfile change is adjacent to the offline-locked OpenClaw plugin install/runtime-deps layer, so this should block merge if plugin/runtime bootstrap regresses.

Optional E2E

  • test-openclaw-skill-cli-e2e.sh (Medium; live Docker sandbox with NVIDIA_API_KEY): Adjacent user-flow confidence for OpenClaw skill install/list inside a real sandbox and for runtime shell environment propagation. It does not directly validate npm registry online mode, so it is useful but not merge-blocking.
  • onboard-inference-smoke-e2e (Medium; about 15 minutes): Additional focused onboard/runtime smoke coverage that ensures a configured provider route serves a real chat completion before onboarding is considered successful. Useful because the changed Dockerfile and entrypoint affect fresh sandbox runtime state.

New E2E recommendations

  • Runtime npm online state (high): Existing E2E coverage only indirectly observes npm online behavior. Add a focused E2E that onboards a fresh sandbox, runs npm config get offline and npm ping/npx -y from both PID-1-inherited exec and openshell sandbox connect-style sourced proxy-env shells, and asserts both NPM_CONFIG_OFFLINE variants are false.
    • Suggested test: runtime-npm-online-e2e
  • Dashboard/MCP package install user flow (medium): The stated risk includes dashboard-driven MCP server installs and ad-hoc package installation. A dedicated E2E should install a tiny npm-backed MCP fixture or harmless package through the same in-sandbox path users exercise, proving the L7 proxy and npm online state work together.
    • Suggested test: mcp-package-install-e2e

Dispatch hint

  • Workflow: .github/workflows/regression-e2e.yaml
  • jobs input: openclaw-plugin-runtime-exdev-e2e,onboard-inference-smoke-e2e

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: ubuntu-repo-cloud-openclaw
Optional scenario E2E: wsl-repo-cloud-openclaw, ubuntu-repo-cloud-hermes

Dispatch required scenario E2E:

  • gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • ubuntu-repo-cloud-openclaw: Dockerfile and entrypoint changes affect the default repo-checkout Docker sandbox runtime, OpenClaw plugin/image build path, runtime proxy environment, and sandbox shell/npm environment. The default Ubuntu OpenClaw scenario is the smallest primary scenario that builds and starts the repo image, onboards OpenClaw, verifies gateway/sandbox health, and exercises the baseline runtime path.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional scenario E2E

  • wsl-repo-cloud-openclaw: Optional adjacent platform coverage for the same repo Docker OpenClaw sandbox startup/runtime environment on WSL, where entrypoint and sandbox-connect environment behavior can differ from Ubuntu.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=wsl-repo-cloud-openclaw
  • ubuntu-repo-cloud-hermes: Optional adjacent agent coverage because the entrypoint tool-cache/npm offline environment is shared across sandbox startups, though the Dockerfile plugin-install change is primarily OpenClaw-targeted.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-hermes

Relevant changed files

  • Dockerfile
  • scripts/nemoclaw-start.sh

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Add live runtime validation for gateway npm/npx paths and policy mediation (test/sandbox-provisioning.test.ts:231): The PR intentionally restores online runtime npm/npx behavior after the build-time plugin-install lock. The added tests now cover Dockerfile ENV ordering, the build-time offline lock, PID 1 exports, and emitted connect-shell env, which is useful. However, the linked issue was reported in fresh sandbox dashboard/gateway/skill child processes, and this change relies on restored online npm remaining mediated by OpenShell network policy rather than bypassing it. There is still no live validation that dashboard MCP or skill package installs inherit the corrected env without a manual gateway reload, nor a denied-policy proof for the same npm/npx fetch path.
    • Recommendation: Add or identify a targeted runtime/integration check that starts a fresh sandbox/gateway path, verifies child npm/npx processes see both NPM_CONFIG_OFFLINE=false and npm_config_offline=false, runs an allowed npx -y @modelcontextprotocol/server-filesystem /sandbox/.openclaw/workspace without ENOTCACHED, and verifies the same install is denied when npmjs is not allowed by policy.
    • Evidence: Dockerfile resets NPM_CONFIG_OFFLINE=false after the plugin install, scripts/nemoclaw-start.sh exports both npm offline spellings as false, test/sandbox-provisioning.test.ts replays Dockerfile ENV directives, and test/service-env.test.ts sources emitted proxy-env.sh. None of the changed tests starts a real OpenShell sandbox gateway, dashboard MCP install, skill installer, or npm child under a denied network policy.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — Fresh sandbox gateway child process starts with NPM_CONFIG_OFFLINE=false and npm_config_offline=false before dashboard MCP install.. The unit/snippet tests are appropriate for guarding Dockerfile and entrypoint env semantics, but the original bug and security boundary occur in real sandbox gateway/dashboard/skill child processes under OpenShell network policy.
  • **Runtime validation** — Dashboard MCP install invoking npx -y @modelcontextprotocol/server-filesystem /sandbox/.openclaw/workspace succeeds without ENOTCACHED when npmjs is allowed by network policy.. The unit/snippet tests are appropriate for guarding Dockerfile and entrypoint env semantics, but the original bug and security boundary occur in real sandbox gateway/dashboard/skill child processes under OpenShell network policy.
  • **Runtime validation** — The same npm/npx install attempt is denied when npmjs is not allowed by OpenShell policy, proving restored online mode does not bypass the policy layer.. The unit/snippet tests are appropriate for guarding Dockerfile and entrypoint env semantics, but the original bug and security boundary occur in real sandbox gateway/dashboard/skill child processes under OpenShell network policy.
  • **Runtime validation** — Fresh sandbox skill/package installer child process inherits both npm offline vars as false without requiring a manual gateway reload.. The unit/snippet tests are appropriate for guarding Dockerfile and entrypoint env semantics, but the original bug and security boundary occur in real sandbox gateway/dashboard/skill child processes under OpenShell network policy.
  • **Runtime validation** — Real openshell sandbox connect session sources /tmp/nemoclaw-proxy-env.sh and reports both npm offline env vars as false.. The unit/snippet tests are appropriate for guarding Dockerfile and entrypoint env semantics, but the original bug and security boundary occur in real sandbox gateway/dashboard/skill child processes under OpenShell network policy.
  • **Acceptance clause:** After adding new mcp using dashboard my agent doesn't see it. — add test evidence or identify existing coverage. The diff addresses the identified npm ENOTCACHED root cause by resetting Dockerfile runtime NPM_CONFIG_OFFLINE=false and exporting both npm_config_offline=false and NPM_CONFIG_OFFLINE=false for PID 1/connect env. No live dashboard Add MCP flow or agent-visible MCP assertion is included.
  • **Acceptance clause:** In logs I have an error of timeout. — add test evidence or identify existing coverage. The PR does not change timeout handling or log diagnostics directly. Later issue comments narrowed an actionable path to npm ENOTCACHED, which this PR addresses, but there is no test that the timeout log path is gone or replaced by successful MCP startup.
  • **Acceptance clause:** For example i've added filesystem mcp(@modelcontextprotocol/server-filesystem) and it's not working using openclaw. — add test evidence or identify existing coverage. Env-level tests make npm/npx online mode available, but no test invokes npx -y @modelcontextprotocol/server-filesystem /sandbox/.openclaw/workspace through the dashboard/gateway path.
Since last review details

Current findings:

  • Add live runtime validation for gateway npm/npx paths and policy mediation (test/sandbox-provisioning.test.ts:231): The PR intentionally restores online runtime npm/npx behavior after the build-time plugin-install lock. The added tests now cover Dockerfile ENV ordering, the build-time offline lock, PID 1 exports, and emitted connect-shell env, which is useful. However, the linked issue was reported in fresh sandbox dashboard/gateway/skill child processes, and this change relies on restored online npm remaining mediated by OpenShell network policy rather than bypassing it. There is still no live validation that dashboard MCP or skill package installs inherit the corrected env without a manual gateway reload, nor a denied-policy proof for the same npm/npx fetch path.
    • Recommendation: Add or identify a targeted runtime/integration check that starts a fresh sandbox/gateway path, verifies child npm/npx processes see both NPM_CONFIG_OFFLINE=false and npm_config_offline=false, runs an allowed npx -y @modelcontextprotocol/server-filesystem /sandbox/.openclaw/workspace without ENOTCACHED, and verifies the same install is denied when npmjs is not allowed by policy.
    • Evidence: Dockerfile resets NPM_CONFIG_OFFLINE=false after the plugin install, scripts/nemoclaw-start.sh exports both npm offline spellings as false, test/sandbox-provisioning.test.ts replays Dockerfile ENV directives, and test/service-env.test.ts sources emitted proxy-env.sh. None of the changed tests starts a real OpenShell sandbox gateway, dashboard MCP install, skill installer, or npm child under a denied network policy.

Workflow run details

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
Dockerfile (1)

583-618: Run the Dockerfile-focused E2E subset before merge.

Given this runtime env behavior change in the sandbox image, run the recommended nightly-e2e subset to validate container/runtime behavior beyond unit tests.

As per coding guidelines, Dockerfile changes are only fully testable with real container builds and the recommended cloud-e2e, sandbox-survival-e2e, hermes-e2e, rebuild-openclaw-e2e, and openclaw-tui-chat-correlation-e2e jobs.

🤖 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 `@Dockerfile` around lines 583 - 618, The Dockerfile change modifies sandbox
runtime behavior around NPM_CONFIG_OFFLINE and the OpenClaw plugin install (see
ENV NPM_CONFIG_OFFLINE and the RUN openclaw plugins install /opt/nemoclaw
block); before merging, run the recommended Dockerfile-focused E2E subset to
validate container/runtime behavior by executing the nightly-e2e subset
(including cloud-e2e, sandbox-survival-e2e, hermes-e2e, rebuild-openclaw-e2e,
and openclaw-tui-chat-correlation-e2e) so the image build, plugin registration,
and sandbox runtime installs are exercised end-to-end and any regressions from
the ENV/RUN changes are caught.

Source: Coding guidelines

scripts/nemoclaw-start.sh (1)

131-135: Validate with entrypoint-focused E2E jobs.

This entrypoint env redirect change affects every sandbox boot path; run the recommended sandbox E2Es before merge.

As per coding guidelines, scripts/nemoclaw-start.sh changes should be validated with sandbox-survival-e2e, sandbox-operations-e2e, cloud-e2e, and openclaw-slack-pairing-e2e.

🤖 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 `@scripts/nemoclaw-start.sh` around lines 131 - 135, The change in
scripts/nemoclaw-start.sh that pins npm runtime env vars
('npm_config_offline=false' and 'NPM_CONFIG_OFFLINE=false') affects every
sandbox entrypoint; before merging, run and attach results from the
entrypoint-focused E2E suites: sandbox-survival-e2e, sandbox-operations-e2e,
cloud-e2e, and openclaw-slack-pairing-e2e to validate sandbox boot paths and
ensure no regressions in openshell sandbox connect or PID 1 behavior; if any
test fails, revert or adjust the env export logic in nemoclaw-start.sh and
re-run the listed E2E jobs until all pass.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@Dockerfile`:
- Around line 583-618: The Dockerfile change modifies sandbox runtime behavior
around NPM_CONFIG_OFFLINE and the OpenClaw plugin install (see ENV
NPM_CONFIG_OFFLINE and the RUN openclaw plugins install /opt/nemoclaw block);
before merging, run the recommended Dockerfile-focused E2E subset to validate
container/runtime behavior by executing the nightly-e2e subset (including
cloud-e2e, sandbox-survival-e2e, hermes-e2e, rebuild-openclaw-e2e, and
openclaw-tui-chat-correlation-e2e) so the image build, plugin registration, and
sandbox runtime installs are exercised end-to-end and any regressions from the
ENV/RUN changes are caught.

In `@scripts/nemoclaw-start.sh`:
- Around line 131-135: The change in scripts/nemoclaw-start.sh that pins npm
runtime env vars ('npm_config_offline=false' and 'NPM_CONFIG_OFFLINE=false')
affects every sandbox entrypoint; before merging, run and attach results from
the entrypoint-focused E2E suites: sandbox-survival-e2e, sandbox-operations-e2e,
cloud-e2e, and openclaw-slack-pairing-e2e to validate sandbox boot paths and
ensure no regressions in openshell sandbox connect or PID 1 behavior; if any
test fails, revert or adjust the env export logic in nemoclaw-start.sh and
re-run the listed E2E jobs until all pass.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2dab1719-9c69-4b6a-a788-08521e22d91e

📥 Commits

Reviewing files that changed from the base of the PR and between d7aa5e0 and 3b6e1df.

📒 Files selected for processing (5)
  • Dockerfile
  • scripts/nemoclaw-start.sh
  • test/fetch-guard-patch-regression.test.ts
  • test/sandbox-provisioning.test.ts
  • test/service-env.test.ts

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27084018473
Target ref: 11b9885554bfa1ce55799bd8d46ddcc9a197a27a
Workflow ref: main
Requested jobs: network-policy-e2e,sandbox-operations-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
network-policy-e2e ✅ success
sandbox-operations-e2e ✅ success

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27084413711
Target ref: fb4df820b41bdb172205caad8f1f36440aa2d6f1
Workflow ref: main
Requested jobs: cloud-onboard-e2e,openclaw-skill-cli-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
openclaw-skill-cli-e2e ✅ success

@laitingsheng laitingsheng added the v0.0.61 Release target label Jun 7, 2026
@cv cv merged commit 9dd7218 into main Jun 7, 2026
37 checks passed
@cv cv deleted the fix/4773-runtime-npm-offline branch June 7, 2026 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior v0.0.61 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't add new mcp in openclaw inside the sandbox

3 participants