Skip to content

fix(sandbox): honor model override in reconcile and log guard-chain warning to file#6065

Merged
jyaunches merged 1 commit into
mainfrom
fix/e2e-model-override-and-recovery-log
Jun 30, 2026
Merged

fix(sandbox): honor model override in reconcile and log guard-chain warning to file#6065
jyaunches merged 1 commit into
mainfrom
fix/e2e-model-override-and-recovery-log

Conversation

@prekshivyas

@prekshivyas prekshivyas commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Two E2E regressions introduced by #5874:

  • runtime-overrides: reconcile_agent_model_with_provider ran after apply_model_override and overwrote agents.defaults.model.primary with an inference/-prefixed variant because qualify() unconditionally prepends inference/. The comment at the call site already documented that the explicit override should win; this adds the guard to enforce it.

  • issue-2478-crash-loop-recovery: restore_openclaw_runtime_guard_chain wrote its warning only to stderr. The new host-mediated recovery path (docker exec … nemoclaw-gateway-control recover) runs inside PID 1, not a fresh sandbox exec, so stderr never reached /tmp/gateway.log. Mirrors the warning to the gateway log file using the same _NEMOCLAW_GATEWAY_LOG seam the watchdog already uses.

Test plan

  • Trigger the runtime-overrides E2E target — expect primaryModel(modelOverride) to return "anthropic/claude-sonnet-4-6" (not "inference/anthropic/…")
  • Trigger the issue-2478-crash-loop-recovery E2E target — expect [gateway-recovery] WARNING: .*restoring library guards from packaged preloads to appear in /tmp/gateway.log
  • Full main E2E run to confirm no regressions

Signed-off-by: Preksha Vijayvargiya prekshiv@nvidia.com

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Preserved explicit model overrides so they are not replaced by later reconciliation.
    • Added startup guard-chain warnings to the gateway log as well as stderr for easier troubleshooting.

…arning to file

Two regressions from #5874:

1. reconcile_agent_model_with_provider ran after apply_model_override and
   overwrote agents.defaults.model.primary with an inference/-prefixed variant
   because qualify() unconditionally adds the prefix. Guard with an early return
   when NEMOCLAW_MODEL_OVERRIDE is set — the comment at the call site already
   documented this intent but the implementation didn't enforce it.

2. restore_openclaw_runtime_guard_chain wrote its warning only to stderr. The
   new host-mediated recovery path (docker exec nemoclaw-gateway-control recover)
   runs inside PID 1, not inside a fresh sandbox exec, so stderr never reached
   /tmp/gateway.log. Mirror the warning to the gateway log file using the same
   pattern as the watchdog (_NEMOCLAW_GATEWAY_LOG seam for tests).

Signed-off-by: Preksha Vijayvargiya <prekshiv@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Modifies scripts/nemoclaw-start.sh with two changes: an early-return guard preventing agent/provider-model reconciliation when NEMOCLAW_MODEL_OVERRIDE is set, and duplication of the runtime guard chain warning message into the gateway log file in addition to stderr output.

Changes

Startup Script Fixes

Layer / File(s) Summary
Skip reconciliation on model override
scripts/nemoclaw-start.sh
reconcile_agent_model_with_provider now returns early as a no-op when NEMOCLAW_MODEL_OVERRIDE is set, preventing the explicit override from being overwritten by inference-qualified reconciliation.
Log guard chain warning to gateway log
scripts/nemoclaw-start.sh
restore_openclaw_runtime_guard_chain stores the "guard chain missing or unsafe" message in a local variable and writes it to the gateway log file in addition to printing it to stderr.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#1633: Introduces the apply_model_override mechanism that this PR's reconciliation skip is designed to protect.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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 title clearly summarizes both main fixes: preserving model overrides during reconciliation and logging the guard-chain warning to a file.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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/e2e-model-override-and-recovery-log

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

@github-code-quality

github-code-quality Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the branch is 96%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File 76e5414 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the branch is 68%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File 76e5414 +/-
src/lib/shields...nsition-lock.ts 86%
src/lib/actions...dbox/rebuild.ts 80%
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 80%
src/lib/state/sandbox.ts 72%
src/lib/onboard/preflight.ts 69%
src/lib/shields/index.ts 67%
src/lib/onboard...er-gpu-patch.ts 59%
src/lib/actions...licy-channel.ts 58%
src/lib/onboard.ts 20%

Updated June 30, 2026 20:04 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: runtime-overrides, openclaw-inference-switch, gateway-guard-recovery, issue-2478-crash-loop-recovery
Optional E2E: test-e2e-gateway-isolation, model-router-provider-routed-inference

Dispatch hint: runtime-overrides,openclaw-inference-switch,gateway-guard-recovery,issue-2478-crash-loop-recovery

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • runtime-overrides (medium): Directly validates live/runtime handling of NEMOCLAW_MODEL_OVERRIDE, related override knobs, and config hash integrity in the packaged sandbox startup path touched by this PR.
  • openclaw-inference-switch (high): Exercises the real user route-switch flow and verifies OpenShell route state, OpenClaw config/hash state, inference.local, and an agent turn after provider/model reconciliation logic has run.
  • gateway-guard-recovery (medium): Covers the /tmp guard-chain wipe/recovery contract and asserts the gateway recovery warning and restored preload chain behavior affected by the logging change.
  • issue-2478-crash-loop-recovery (medium): Validates crash-loop recovery preserves/restores the guard chain and observes the [gateway-recovery] WARNING marker in the gateway recovery path changed by this PR.

Optional E2E

  • test-e2e-gateway-isolation (medium): Useful adjacent coverage for the production image entrypoint, gateway isolation, and existing shell assertions around NEMOCLAW_MODEL_OVERRIDE patching of openclaw.json.
  • model-router-provider-routed-inference (high): Optional confidence for provider-routed inference after startup reconciliation, especially because the changed code reasons about gateway/provider model state.

New E2E recommendations

  • model override versus gateway reconciliation (medium): Existing runtime override coverage validates the override itself, and inference-switch coverage validates route reconciliation, but there does not appear to be a focused live regression that starts with NEMOCLAW_MODEL_OVERRIDE while openshell inference get --json reports a divergent or inference/-qualified model and asserts the override is not overwritten.
    • Suggested test: Add a focused live E2E that boots a sandbox with NEMOCLAW_MODEL_OVERRIDE, forces or stubs a divergent gateway inference route, restarts/reconciles, and verifies openclaw.json primary/model entries plus hash still reflect the explicit override.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: runtime-overrides,openclaw-inference-switch,gateway-guard-recovery,issue-2478-crash-loop-recovery

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-openclaw
Optional E2E targets: ubuntu-repo-docker-post-reboot-recovery

Dispatch required E2E targets:

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

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • ubuntu-repo-cloud-openclaw: The PR changes the NemoClaw startup script paths for model override/provider reconciliation and gateway runtime guard restoration. The primary live-supported OpenClaw target exercises startup, gateway health, inference routing, and credential-backed cloud onboarding on Ubuntu Docker.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Optional E2E targets

  • ubuntu-repo-docker-post-reboot-recovery: Optional adjacent coverage for the changed gateway guard restoration/recovery logging path, which is closest to the post-reboot recovery lifecycle surface but not the primary model-override regression path.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-docker-post-reboot-recovery

Relevant changed files

  • scripts/nemoclaw-start.sh

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Validate the gateway log target before root append.
Open items: 0 required · 2 warnings · 0 suggestions · 8 test follow-ups
Top item: Validate the gateway log target before root append

Action checklist

  • PRA-1 Resolve or justify: Validate the gateway log target before root append in scripts/nemoclaw-start.sh:4474
  • PRA-2 Resolve or justify: Add a focused regression for explicit model override winning reconciliation in scripts/nemoclaw-start.sh:1039
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Add a focused regression for explicit model override winning reconciliation
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify security scripts/nemoclaw-start.sh:4474 Either route this through a small safe append helper that refuses symlinks/non-regular files and enforces the expected ownership/location, or add code/test evidence that production always precreates the gateway log with safe permissions before this root recovery function can run and that the sandbox user cannot substitute it.
PRA-2 Resolve/justify tests scripts/nemoclaw-start.sh:1039 Add a unit-style case in `test/nemoclaw-start-reconcile.test.ts` with `NEMOCLAW_MODEL_OVERRIDE=anthropic/claude-sonnet-4-6` and an `openshell inference get --json` stub returning a different model, then assert reconciliation does not rewrite the already-overridden config or recompute the hash.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Validate the gateway log target before root append

  • Location: scripts/nemoclaw-start.sh:4474
  • Category: security
  • Problem: `restore_openclaw_runtime_guard_chain()` can run in PID 1/root recovery paths and now appends directly to `${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}`. The warning text is constant, so this is not log injection or secret disclosure, but the target path is an env/test-seam-controlled fallback in `/tmp` and this change extends an existing direct-redirection pattern into guard-chain recovery.
  • Impact: If the sandbox user can replace `/tmp/gateway.log` or influence `_NEMOCLAW_GATEWAY_LOG` with a symlink or non-regular file before recovery, PID 1 could append a root-written line to an unintended path. Even though the payload is constant, this weakens the file-boundary assumptions around a sandbox recovery path.
  • Recommended action: Either route this through a small safe append helper that refuses symlinks/non-regular files and enforces the expected ownership/location, or add code/test evidence that production always precreates the gateway log with safe permissions before this root recovery function can run and that the sandbox user cannot substitute it.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read the initialization and ownership flow for `_NEMOCLAW_GATEWAY_LOG` and `/tmp/gateway.log`, then check whether a sandbox-owned `/tmp/gateway.log` symlink can exist before `restore_openclaw_runtime_guard_chain()` is called from `prepare_openclaw_gateway_restart()` or `prepare_openclaw_automatic_respawn()`.
  • Missing regression test: Add a shell-harness test around `restore_openclaw_runtime_guard_chain()` that sets `_NEMOCLAW_GATEWAY_LOG` to a symlink or non-regular temp path and proves the code refuses or ignores it without writing through the link; if the design intentionally permits only a precreated regular file, test that ownership/no-symlink invariant.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the initialization and ownership flow for `_NEMOCLAW_GATEWAY_LOG` and `/tmp/gateway.log`, then check whether a sandbox-owned `/tmp/gateway.log` symlink can exist before `restore_openclaw_runtime_guard_chain()` is called from `prepare_openclaw_gateway_restart()` or `prepare_openclaw_automatic_respawn()`.
  • Evidence: The diff adds `echo "$_guard_warn" >>"${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}" 2>/dev/null || true` inside `restore_openclaw_runtime_guard_chain()`, which is reachable from gateway restart/recovery code paths.

PRA-2 Resolve/justify — Add a focused regression for explicit model override winning reconciliation

  • Location: scripts/nemoclaw-start.sh:1039
  • Category: tests
  • Problem: The new early return is the entire correctness fix for the runtime-overrides regression, but the changed source file has no accompanying focused test. Existing live Docker coverage checks `primaryModel(modelOverride)` through the full entrypoint, while `test/nemoclaw-start-reconcile.test.ts` has a lightweight harness for exactly this function and currently lacks the conflicting-gateway-model case.
  • Impact: A future refactor of `reconcile_agent_model_with_provider()` could reintroduce the `inference/` prefix overwrite while still leaving broader live coverage gated or slow, making this high-risk sandbox config behavior easier to regress.
  • Recommended action: Add a unit-style case in `test/nemoclaw-start-reconcile.test.ts` with `NEMOCLAW_MODEL_OVERRIDE=anthropic/claude-sonnet-4-6` and an `openshell inference get --json` stub returning a different model, then assert reconciliation does not rewrite the already-overridden config or recompute the hash.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `test/nemoclaw-start-reconcile.test.ts` and confirm there is no case where `options.env` includes `NEMOCLAW_MODEL_OVERRIDE` while the gateway stub returns a conflicting model.
  • Missing regression test: Concrete test: `reconcile is a no-op when NEMOCLAW_MODEL_OVERRIDE is set even if the live gateway reports another model`; assert primary remains `anthropic/claude-sonnet-4-6`, provider model entries are unchanged from the post-override fixture, and `.config-hash` is unchanged by reconciliation.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `test/nemoclaw-start-reconcile.test.ts` and confirm there is no case where `options.env` includes `NEMOCLAW_MODEL_OVERRIDE` while the gateway stub returns a conflicting model.
  • Evidence: The diff adds `[ -z "${NEMOCLAW_MODEL_OVERRIDE:-}" ] || return 0`; existing `runtime-overrides.test.ts` has live Docker coverage, but no changed test file or nearby function-level case was added.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add `reconcile is a no-op when NEMOCLAW_MODEL_OVERRIDE is set even if the live gateway reports another model` to `test/nemoclaw-start-reconcile.test.ts`.. The only changed file is the sandbox entrypoint/recovery shell. Existing live tests cover related acceptance paths, but this PR changes high-risk runtime behavior and should have focused harness coverage for the exact branches plus runtime validation through the existing live targets.
  • PRA-T2 Runtime validation — Add `guard-chain recovery appends the restoration warning to _NEMOCLAW_GATEWAY_LOG when the transient guard chain is incomplete` to `test/nemoclaw-start-guard-recovery.test.ts`.. The only changed file is the sandbox entrypoint/recovery shell. Existing live tests cover related acceptance paths, but this PR changes high-risk runtime behavior and should have focused harness coverage for the exact branches plus runtime validation through the existing live targets.
  • PRA-T3 Runtime validation — Add `guard-chain recovery does not write through a symlink or non-regular gateway log target` if direct root-time log appends remain.. The only changed file is the sandbox entrypoint/recovery shell. Existing live tests cover related acceptance paths, but this PR changes high-risk runtime behavior and should have focused harness coverage for the exact branches plus runtime validation through the existing live targets.
  • PRA-T4 Runtime validation — Run or identify runtime validation for `runtime-overrides` and `issue-2478-crash-loop-recovery`; do not rely on external status in this review surface.. The only changed file is the sandbox entrypoint/recovery shell. Existing live tests cover related acceptance paths, but this PR changes high-risk runtime behavior and should have focused harness coverage for the exact branches plus runtime validation through the existing live targets.
  • PRA-T5 Add a focused regression for explicit model override winning reconciliation — Add a unit-style case in `test/nemoclaw-start-reconcile.test.ts` with `NEMOCLAW_MODEL_OVERRIDE=anthropic/claude-sonnet-4-6` and an `openshell inference get --json` stub returning a different model, then assert reconciliation does not rewrite the already-overridden config or recompute the hash.
  • PRA-T6 Acceptance clause — Onboard finishes successfully. `nemoclaw <name> status` initially shows everything green. But the gateway never actually serves anything. Messages to the Telegram bot get no reply. — add test evidence or identify existing coverage. This PR does not change onboarding/status behavior directly. Existing live `issue-2478-crash-loop-recovery.test.ts` exercises a recovered OpenClaw sandbox and checks gateway/inference availability; this diff only improves the recovery warning visibility in `/tmp/gateway.log`.
  • PRA-T7 Acceptance clause — 1. Wrap the ciao NetworkManager calls in try/catch inside OpenClaw, fall back to no mDNS if `os.networkInterfaces()` throws. Probably the smallest diff. — add test evidence or identify existing coverage. This is an OpenClaw-side possible fix from the issue, not the approach taken in this NemoClaw shell change. No OpenClaw ciao code is changed in this PR.
  • PRA-T8 Acceptance clause — 2. Add an `OPENCLAW_DISABLE_MDNS=1` env var (or a config flag) that skips loading ciao entirely. Most explicit user-facing fix. — add test evidence or identify existing coverage. This PR does not add an OpenClaw env var or config flag. It relies on the existing NemoClaw runtime guard preload recovery path.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 1068-1071: Add test coverage for the no-override reconciliation
path in `scripts/nemoclaw-start.sh`, specifically around the
`NEMOCLAW_MODEL_OVERRIDE` guard and the gateway/legacy model reconciliation
logic. Extend the existing tests that verify `agents.defaults.model.primary`
preservation so they also assert that when `NEMOCLAW_MODEL_OVERRIDE` is unset,
reconciliation can still replace the model with the gateway-derived value. Make
sure the new case targets the same flow controlled by the override check so
regressions in `apply_model_override`/reconciliation are caught.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 504fff40-39aa-4268-b8b1-00468674de30

📥 Commits

Reviewing files that changed from the base of the PR and between 5d115f8 and 76e5414.

📒 Files selected for processing (1)
  • scripts/nemoclaw-start.sh

Comment thread scripts/nemoclaw-start.sh
Comment on lines +1068 to +1071
# apply_model_override already won; reconciling against the gateway would
# overwrite the user's explicit choice with an inference/-prefixed variant.
[ -z "${NEMOCLAW_MODEL_OVERRIDE:-}" ] || return 0

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for a test asserting reconciliation still runs when override is unset.
rg -n -B2 -A15 'reconcile_agent_model_with_provider|provider-model reconciliation' test/nemoclaw-start.test.ts

Repository: NVIDIA/NemoClaw

Length of output: 2303


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='test/nemoclaw-start.test.ts'

echo '--- search: override + primary ---'
rg -n 'NEMOCLAW_MODEL_OVERRIDE|agents\.defaults\.model\.primary|reconcile_agent_model_with_provider|gateway|legacy-derived|provider-model' "$file" || true

echo
echo '--- around first reconcile stub ---'
sed -n '430,520p' "$file"

echo
echo '--- around second reconcile stub ---'
sed -n '3600,3685p' "$file"

Repository: NVIDIA/NemoClaw

Length of output: 19828


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1280,1385p' test/nemoclaw-start.test.ts

Repository: NVIDIA/NemoClaw

Length of output: 3748


Add coverage for the unset-override reconciliation path. The tests cover NEMOCLAW_MODEL_OVERRIDE preserving agents.defaults.model.primary, but not the no-override case where gateway/legacy reconciliation should still be able to replace it. Without that, a future change could disable reconciliation entirely without a failing test.

🤖 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 1068 - 1071, Add test coverage for
the no-override reconciliation path in `scripts/nemoclaw-start.sh`, specifically
around the `NEMOCLAW_MODEL_OVERRIDE` guard and the gateway/legacy model
reconciliation logic. Extend the existing tests that verify
`agents.defaults.model.primary` preservation so they also assert that when
`NEMOCLAW_MODEL_OVERRIDE` is unset, reconciliation can still replace the model
with the gateway-derived value. Make sure the new case targets the same flow
controlled by the override check so regressions in
`apply_model_override`/reconciliation are caught.

Source: Path instructions

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Missing test for NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider.
Open items: 0 required · 3 warnings · 1 suggestion · 5 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Missing test for NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider in test/nemoclaw-start-reconcile.test.ts
  • PRA-2 Resolve or justify: Missing test for guard-chain warning written to _NEMOCLAW_GATEWAY_LOG in test/nemoclaw-start-guard-recovery.test.ts
  • PRA-4 Resolve or justify: NEMOCLAW_MODEL_OVERRIDE early-return is a localized workaround for dual-write architecture in scripts/nemoclaw-start.sh:1071
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Missing test for NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider
  • PRA-T4 Add or justify test follow-up: Missing test for guard-chain warning written to _NEMOCLAW_GATEWAY_LOG
  • PRA-T5 Add or justify test follow-up: NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider
  • PRA-3 In-scope improvement: Silent log-write failure suppression in restore_openclaw_runtime_guard_chain in scripts/nemoclaw-start.sh:4415

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify tests test/nemoclaw-start-reconcile.test.ts Add a test in nemoclaw-start-reconcile.test.ts that sets NEMOCLAW_MODEL_OVERRIDE and verifies the reconcile function returns early without modifying openclaw.json, without recomputing the config hash, and without calling openshell inference get.
PRA-2 Resolve/justify tests test/nemoclaw-start-guard-recovery.test.ts Extend the guard-recovery test to set _NEMOCLAW_GATEWAY_LOG to a test-controlled file and assert the warning message is appended to that file when the guard chain is incomplete.
PRA-3 Improvement security scripts/nemoclaw-start.sh:4415 Consider logging the write failure to stderr at debug level, or document that _NEMOCLAW_GATEWAY_LOG must be a writable path controlled by the operator. This is a defense-in-depth improvement, not a vulnerability.
PRA-4 Resolve/justify correctness scripts/nemoclaw-start.sh:1071 Track as technical debt. The minimal fix (F1 test) provides regression protection. A future PR should consolidate model override and reconciliation into a single write path or pass override state explicitly.
Review findings by urgency: 0 required fixes, 3 items to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Missing test for NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider

  • Location: test/nemoclaw-start-reconcile.test.ts
  • Category: tests
  • Problem: The new early-return guard at line ~1071-1074 in scripts/nemoclaw-start.sh returns 0 immediately when NEMOCLAW_MODEL_OVERRIDE is set, but the test suite lacks a case exercising this path. Without a test, a future refactor could silently remove the guard and reintroduce the [Inference] openshell inference set changes gateway model but sandbox agent still reports old model #3175 regression where gateway probe overwrites user's explicit model choice with an inference/-prefixed variant.
  • Impact: Regression risk: user-specified model override could be overwritten by gateway reconciliation, breaking the trust boundary that explicit user intent should win over gateway state.
  • Recommended action: Add a test in nemoclaw-start-reconcile.test.ts that sets NEMOCLAW_MODEL_OVERRIDE and verifies the reconcile function returns early without modifying openclaw.json, without recomputing the config hash, and without calling openshell inference get.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run the reconcile test suite (vitest run test/nemoclaw-start-reconcile.test.ts) and confirm a new test case for NEMOCLAW_MODEL_OVERRIDE passes.
  • Missing regression test: Test that NEMOCLAW_MODEL_OVERRIDE=explicit-model causes reconcile_agent_model_with_provider to return early without modifying openclaw.json or calling openshell inference get.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the reconcile test suite (vitest run test/nemoclaw-start-reconcile.test.ts) and confirm a new test case for NEMOCLAW_MODEL_OVERRIDE passes.
  • Evidence: reconcile_agent_model_with_provider() at line ~1071-1074: `[ -z "${NEMOCLAW_MODEL_OVERRIDE:-}" ] || return 0` with comment explaining apply_model_override already won. No test in nemoclaw-start-reconcile.test.ts exercises this branch.

PRA-2 Resolve/justify — Missing test for guard-chain warning written to _NEMOCLAW_GATEWAY_LOG

  • Location: test/nemoclaw-start-guard-recovery.test.ts
  • Category: tests
  • Problem: The new log-file write in restore_openclaw_runtime_guard_chain() (line ~4415) appends the warning to ${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}, but the test harness only captures stderr and does not assert the warning also appears in the gateway log file. This is the specific fix for the host-mediated recovery path (docker exec ... nemoclaw-gateway-control recover) where stderr never reaches the gateway log.
  • Impact: Regression risk: host-mediated gateway recovery could silently lose guard-chain restoration observability, making post-mortem diagnosis of crash-loop recovery ([DGX Spark] Gateway crash loop on startup: @homebridge/ciao networkInterfaces() returns EPERM in OpenShell sandbox #2478/[DGX Spark] Host reboot bricks sandbox until 5-min rebuild --yes: connect recovery path warns about missing /tmp guards but launches gateway naked → @homebridge/ciao crash loop #2701) harder.
  • Recommended action: Extend the guard-recovery test to set _NEMOCLAW_GATEWAY_LOG to a test-controlled file and assert the warning message is appended to that file when the guard chain is incomplete.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run the guard-recovery test (vitest run test/nemoclaw-start-guard-recovery.test.ts) and verify the new log-file assertion passes.
  • Missing regression test: Test that restore_openclaw_runtime_guard_chain writes the guard-chain warning to _NEMOCLAW_GATEWAY_LOG (default /tmp/gateway.log) when guard files are missing.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the guard-recovery test (vitest run test/nemoclaw-start-guard-recovery.test.ts) and verify the new log-file assertion passes.
  • Evidence: restore_openclaw_runtime_guard_chain() at line ~4415: `echo "$_guard_warn" >>"${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}" 2>/dev/null || true`. Test harness in nemoclaw-start-guard-recovery.test.ts captures stderr but does not check the gateway log file.

PRA-4 Resolve/justify — NEMOCLAW_MODEL_OVERRIDE early-return is a localized workaround for dual-write architecture

  • Location: scripts/nemoclaw-start.sh:1071
  • Category: correctness
  • Problem: The early return in reconcile_agent_model_with_provider() is a localized workaround for the fact that apply_model_override() and reconcile_agent_model_with_provider() both write to the same config file (openclaw.json). The root cause is two separate phases mutating the same state. A future fix could make reconciliation aware of override state without early return, or consolidate the write.
  • Impact: Technical debt: the workaround masks an architectural issue where two sequential phases mutate the same config file. If a future change adds a third phase or reorders calls, the early return may not be sufficient.
  • Recommended action: Track as technical debt. The minimal fix (F1 test) provides regression protection. A future PR should consolidate model override and reconciliation into a single write path or pass override state explicitly.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Verify the early return guard exists at line ~1071-1074 and that apply_model_override() runs before reconcile_agent_model_with_provider() in the startup sequence.
  • Missing regression test: Test that NEMOCLAW_MODEL_OVERRIDE=explicit-model causes reconcile_agent_model_with_provider to return early without modifying openclaw.json or calling openshell inference get (same as F1).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Verify the early return guard exists at line ~1071-1074 and that apply_model_override() runs before reconcile_agent_model_with_provider() in the startup sequence.
  • Evidence: scripts/nemoclaw-start.sh:1071-1074 — early return with comment: 'apply_model_override already won; reconciling against the gateway would overwrite the user's explicit choice with an inference/-prefixed variant.'

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-3 Improvement — Silent log-write failure suppression in restore_openclaw_runtime_guard_chain

  • Location: scripts/nemoclaw-start.sh:4415
  • Category: security
  • Problem: The log-file write uses `2>/dev/null || true` which silently ignores all failures (permission denied, disk full, invalid path, path traversal via _NEMOCLAW_GATEWAY_LOG). While justified for non-critical observability, it could mask operational issues. The variable _NEMOCLAW_GATEWAY_LOG is used elsewhere in the codebase for gateway logging and is considered trusted.
  • Impact: Operational visibility: if the gateway log becomes unwritable (disk full, permission change, path removed), the warning disappears silently. No sensitive data is logged.
  • Suggested action: Consider logging the write failure to stderr at debug level, or document that _NEMOCLAW_GATEWAY_LOG must be a writable path controlled by the operator. This is a defense-in-depth improvement, not a vulnerability.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect line 4415 in scripts/nemoclaw-start.sh and confirm _NEMOCLAW_GATEWAY_LOG is only set by trusted startup code (grep for _NEMOCLAW_GATEWAY_LOG in scripts/).
  • Missing regression test: None needed — this is a defense-in-depth observability improvement.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line ~4415: `echo "$_guard_warn" >>"${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}" 2>/dev/null || true`. Variable _NEMOCLAW_GATEWAY_LOG used in gateway watchdog and launch wiring (test/nemoclaw-start-gateway-health.test.ts lines 138, 353).
Simplification opportunities: 2 possible cuts

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-1 yagni (test/nemoclaw-start-reconcile.test.ts): Early return guard for NEMOCLAW_MODEL_OVERRIDE
    • Replacement: Pass override state through to reconciliation logic instead of early return
    • Net: 0 lines
    • Safety boundary: Must preserve that explicit user model override wins over gateway reconciliation
  • PRA-2 native (test/nemoclaw-start-guard-recovery.test.ts): Log-file append via shell redirection
    • Replacement: Native shell >> operator (already used)
    • Net: 0 lines
    • Safety boundary: Must not introduce custom logging abstraction; shell redirection is sufficient
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — reconcile_agent_model_with_provider() honors NEMOCLAW_MODEL_OVERRIDE early-return. Runtime/sandbox/infrastructure paths need behavioral runtime validation: scripts/nemoclaw-start.sh.
  • PRA-T2 Runtime validation — restore_openclaw_runtime_guard_chain() writes warning to _NEMOCLAW_GATEWAY_LOG. Runtime/sandbox/infrastructure paths need behavioral runtime validation: scripts/nemoclaw-start.sh.
  • PRA-T3 Missing test for NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider — Add a test in nemoclaw-start-reconcile.test.ts that sets NEMOCLAW_MODEL_OVERRIDE and verifies the reconcile function returns early without modifying openclaw.json, without recomputing the config hash, and without calling openshell inference get.
  • PRA-T4 Missing test for guard-chain warning written to _NEMOCLAW_GATEWAY_LOG — Extend the guard-recovery test to set _NEMOCLAW_GATEWAY_LOG to a test-controlled file and assert the warning message is appended to that file when the guard chain is incomplete.
  • PRA-T5 NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider — Test that NEMOCLAW_MODEL_OVERRIDE=explicit-model causes early return without modifying config or calling openshell inference get. scripts/nemoclaw-start.sh:1071-1074
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Missing test for NEMOCLAW_MODEL_OVERRIDE early-return in reconcile_agent_model_with_provider

  • Location: test/nemoclaw-start-reconcile.test.ts
  • Category: tests
  • Problem: The new early-return guard at line ~1071-1074 in scripts/nemoclaw-start.sh returns 0 immediately when NEMOCLAW_MODEL_OVERRIDE is set, but the test suite lacks a case exercising this path. Without a test, a future refactor could silently remove the guard and reintroduce the [Inference] openshell inference set changes gateway model but sandbox agent still reports old model #3175 regression where gateway probe overwrites user's explicit model choice with an inference/-prefixed variant.
  • Impact: Regression risk: user-specified model override could be overwritten by gateway reconciliation, breaking the trust boundary that explicit user intent should win over gateway state.
  • Recommended action: Add a test in nemoclaw-start-reconcile.test.ts that sets NEMOCLAW_MODEL_OVERRIDE and verifies the reconcile function returns early without modifying openclaw.json, without recomputing the config hash, and without calling openshell inference get.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run the reconcile test suite (vitest run test/nemoclaw-start-reconcile.test.ts) and confirm a new test case for NEMOCLAW_MODEL_OVERRIDE passes.
  • Missing regression test: Test that NEMOCLAW_MODEL_OVERRIDE=explicit-model causes reconcile_agent_model_with_provider to return early without modifying openclaw.json or calling openshell inference get.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the reconcile test suite (vitest run test/nemoclaw-start-reconcile.test.ts) and confirm a new test case for NEMOCLAW_MODEL_OVERRIDE passes.
  • Evidence: reconcile_agent_model_with_provider() at line ~1071-1074: `[ -z "${NEMOCLAW_MODEL_OVERRIDE:-}" ] || return 0` with comment explaining apply_model_override already won. No test in nemoclaw-start-reconcile.test.ts exercises this branch.

PRA-2 Resolve/justify — Missing test for guard-chain warning written to _NEMOCLAW_GATEWAY_LOG

  • Location: test/nemoclaw-start-guard-recovery.test.ts
  • Category: tests
  • Problem: The new log-file write in restore_openclaw_runtime_guard_chain() (line ~4415) appends the warning to ${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}, but the test harness only captures stderr and does not assert the warning also appears in the gateway log file. This is the specific fix for the host-mediated recovery path (docker exec ... nemoclaw-gateway-control recover) where stderr never reaches the gateway log.
  • Impact: Regression risk: host-mediated gateway recovery could silently lose guard-chain restoration observability, making post-mortem diagnosis of crash-loop recovery ([DGX Spark] Gateway crash loop on startup: @homebridge/ciao networkInterfaces() returns EPERM in OpenShell sandbox #2478/[DGX Spark] Host reboot bricks sandbox until 5-min rebuild --yes: connect recovery path warns about missing /tmp guards but launches gateway naked → @homebridge/ciao crash loop #2701) harder.
  • Recommended action: Extend the guard-recovery test to set _NEMOCLAW_GATEWAY_LOG to a test-controlled file and assert the warning message is appended to that file when the guard chain is incomplete.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run the guard-recovery test (vitest run test/nemoclaw-start-guard-recovery.test.ts) and verify the new log-file assertion passes.
  • Missing regression test: Test that restore_openclaw_runtime_guard_chain writes the guard-chain warning to _NEMOCLAW_GATEWAY_LOG (default /tmp/gateway.log) when guard files are missing.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the guard-recovery test (vitest run test/nemoclaw-start-guard-recovery.test.ts) and verify the new log-file assertion passes.
  • Evidence: restore_openclaw_runtime_guard_chain() at line ~4415: `echo "$_guard_warn" >>"${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}" 2>/dev/null || true`. Test harness in nemoclaw-start-guard-recovery.test.ts captures stderr but does not check the gateway log file.

PRA-3 Improvement — Silent log-write failure suppression in restore_openclaw_runtime_guard_chain

  • Location: scripts/nemoclaw-start.sh:4415
  • Category: security
  • Problem: The log-file write uses `2>/dev/null || true` which silently ignores all failures (permission denied, disk full, invalid path, path traversal via _NEMOCLAW_GATEWAY_LOG). While justified for non-critical observability, it could mask operational issues. The variable _NEMOCLAW_GATEWAY_LOG is used elsewhere in the codebase for gateway logging and is considered trusted.
  • Impact: Operational visibility: if the gateway log becomes unwritable (disk full, permission change, path removed), the warning disappears silently. No sensitive data is logged.
  • Suggested action: Consider logging the write failure to stderr at debug level, or document that _NEMOCLAW_GATEWAY_LOG must be a writable path controlled by the operator. This is a defense-in-depth improvement, not a vulnerability.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect line 4415 in scripts/nemoclaw-start.sh and confirm _NEMOCLAW_GATEWAY_LOG is only set by trusted startup code (grep for _NEMOCLAW_GATEWAY_LOG in scripts/).
  • Missing regression test: None needed — this is a defense-in-depth observability improvement.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line ~4415: `echo "$_guard_warn" >>"${_NEMOCLAW_GATEWAY_LOG:-/tmp/gateway.log}" 2>/dev/null || true`. Variable _NEMOCLAW_GATEWAY_LOG used in gateway watchdog and launch wiring (test/nemoclaw-start-gateway-health.test.ts lines 138, 353).

PRA-4 Resolve/justify — NEMOCLAW_MODEL_OVERRIDE early-return is a localized workaround for dual-write architecture

  • Location: scripts/nemoclaw-start.sh:1071
  • Category: correctness
  • Problem: The early return in reconcile_agent_model_with_provider() is a localized workaround for the fact that apply_model_override() and reconcile_agent_model_with_provider() both write to the same config file (openclaw.json). The root cause is two separate phases mutating the same state. A future fix could make reconciliation aware of override state without early return, or consolidate the write.
  • Impact: Technical debt: the workaround masks an architectural issue where two sequential phases mutate the same config file. If a future change adds a third phase or reorders calls, the early return may not be sufficient.
  • Recommended action: Track as technical debt. The minimal fix (F1 test) provides regression protection. A future PR should consolidate model override and reconciliation into a single write path or pass override state explicitly.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Verify the early return guard exists at line ~1071-1074 and that apply_model_override() runs before reconcile_agent_model_with_provider() in the startup sequence.
  • Missing regression test: Test that NEMOCLAW_MODEL_OVERRIDE=explicit-model causes reconcile_agent_model_with_provider to return early without modifying openclaw.json or calling openshell inference get (same as F1).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Verify the early return guard exists at line ~1071-1074 and that apply_model_override() runs before reconcile_agent_model_with_provider() in the startup sequence.
  • Evidence: scripts/nemoclaw-start.sh:1071-1074 — early return with comment: 'apply_model_override already won; reconciling against the gateway would overwrite the user's explicit choice with an inference/-prefixed variant.'

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Target Results — ✅ All requested jobs passed

Run: 28472585714
Workflow ref: fix/e2e-model-override-and-recovery-log
Requested targets: (default — all supported)
Requested jobs: runtime-overrides,openclaw-inference-switch,gateway-guard-recovery,issue-2478-crash-loop-recovery
Summary: 4 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
gateway-guard-recovery ✅ success
issue-2478-crash-loop-recovery ✅ success
openclaw-inference-switch ✅ success
runtime-overrides ✅ success

@jyaunches jyaunches merged commit 3f358d9 into main Jun 30, 2026
125 checks passed
@jyaunches jyaunches deleted the fix/e2e-model-override-and-recovery-log branch June 30, 2026 20:38
jyaunches added a commit that referenced this pull request Jun 30, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Refreshes the public documentation for NemoClaw v0.0.71 after scanning
commits since v0.0.70. Adds release notes and fills the remaining doc
gaps for Windows bootstrap diagnostics, OpenClaw agent auto-relock
warnings, auto-pair cadence tuning, and plugin-install recovery hints.

## Changes
- `docs/about/release-notes.mdx`: adds the v0.0.71 release-note section,
grouped by gateway recovery, OpenShell auth, policy provenance, day-two
maintenance, messaging/inference, and Windows setup.
- `docs/get-started/windows-preparation.mdx`: documents sanitized WSL
install output and reboot gating in the Windows bootstrap.
- `docs/reference/commands.mdx`: documents the host `agent` wrapper's
shields auto-relock warning and OpenClaw auto-pair watcher tuning
variables.
- `docs/reference/troubleshooting.mdx`: adds plugin-install network
failure recovery guidance and updates Windows WSL troubleshooting for
sanitized install logs and reboot-required handling.

Source summary:
- #6065 -> `docs/about/release-notes.mdx`: Notes explicit model override
preservation and gateway-log guard-chain recovery diagnostics.
- #5874 -> `docs/about/release-notes.mdx`: Summarizes host-mediated
`recover` and `gateway restart`, linking to lifecycle, command,
troubleshooting, and trusted-boundary docs already added by the source
PR.
- #5596 -> `docs/about/release-notes.mdx`: Summarizes OpenShell 0.0.71
gateway auth, loopback binding, and compatibility-container docs already
added by the source PR.
- #5797 and #5798 -> `docs/about/release-notes.mdx`: Summarizes
`policy-list` provenance, Restricted tier suppression, and Balanced tier
weather behavior already reflected in policy docs.
- #5784 -> `docs/about/release-notes.mdx`: Summarizes
`--destroy-user-data` and the safe `--yes` uninstall behavior already
documented in lifecycle and command docs.
- #6034 -> `docs/about/release-notes.mdx`: Summarizes custom Dockerfile
warm-build cache behavior already documented in the command reference.
- #5951 -> `docs/reference/commands.mdx`: Documents the stderr-only host
`agent` wrapper warning after recent shields auto-relock.
- #5387 -> `docs/reference/commands.mdx`: Documents OpenClaw auto-pair
watcher cadence and fast-reentry tuning variables.
- #5835 -> `docs/reference/troubleshooting.mdx`: Adds recovery guidance
for OpenClaw plugin-install network failures.
- #5995 and #5956 -> `docs/about/release-notes.mdx`: Summarizes
Microsoft Teams final-message delivery and runtime mention hints already
covered by messaging docs.
- #5716 -> `docs/about/release-notes.mdx`: Summarizes non-interactive
Ollama loopback safety already covered by local inference docs.
- #5505, #5527, and #5528 -> `docs/about/release-notes.mdx`: Summarizes
compatible local endpoint, model task-fit, and model capability audit
docs.
- #6009 -> `docs/get-started/windows-preparation.mdx`,
`docs/reference/troubleshooting.mdx`: Documents sanitized Windows
bootstrap WSL output and reboot-required gating.
- #6055 -> no additional source doc page change needed beyond the
already-merged quickstart update; release notes did not duplicate
routine quickstart cleanup.

No matching v0.0.71 GitHub announcement discussion was found in the
latest 20 discussions, so this refresh is based on the commit scan and
existing source PR docs.

## Type of Change

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

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: docs-only refresh with no
runtime behavior changes.
- [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:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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) — ran
`npm run docs`; Fern reported 0 errors and 2 existing warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>


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

## Summary by CodeRabbit

* **Documentation**
* Added a new release-notes entry covering gateway recovery,
authentication, network policy/provenance output, uninstall safety,
Windows bootstrap diagnostics, messaging defaults, and inference setup
guidance.
* Clarified Windows preparation steps around reboot behavior and
redacting troubleshooting transcripts.
* Expanded command reference details for OpenClaw wrapper behavior and
new auto-pair tuning options.
* Improved troubleshooting guidance for plugin installation issues, WSL
repair/reboot cases, and install timing problems.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
prekshivyas added a commit that referenced this pull request Jul 1, 2026
…st it

An audit of the live E2E suite (which does not run on PR CI) found
behavior-critical assertions that were only guarded by live targets, the
same gap that let the #6065 regressions ship. Backfill the high-priority,
security/recovery-class ones as fast mocked units that run on every PR:

- ollama-auth-proxy: Bearer enforcement, no /api/tags bypass (#3338),
  header stripping, non-ASCII auth no-crash (#4820), backend 502.
- config get: credential redaction + gateway-key omission (the nvapi- class).
- device approval policy: scope-upgrade allowlist gate, gateway-env
  stripping, and recover-failed rejection paths (#4462).
- shields audit JSONL: credentials redacted before persistence.
- hermes env secret boundary: value-shape (not key-name) discriminator
  accepts openshell:resolve refs and rejects raw secrets without echoing
  (added to the dedicated hardening suite).
- dashboard bind: NEMOCLAW_DASHBOARD_BIND opt-in incl. negative cases (#3259).
- whatsapp compact QR: package shape-detection + terminal-only small (#4522).

Guard the recurrence: scripts/checks/no-unit-blocks-in-live-e2e.ts bans the
vitest it(...) primitive inside test/e2e/live/** (those blocks never run on
PR CI). Relocate the two existing offenders — the skill-agent and
messaging-compatible-endpoint classifier blocks — into importable
test/e2e/support modules with PR-collected unit tests; the live tests import
them unchanged.

whatsapp-qr-compact.ts: minimal behavior-preserving refactor to export the
pure shape-detection/patch helpers (the preload still auto-installs on
require); tsconfig.runtime-preloads.json excludes the new co-located test
from the shipped preload build.

SKIP=test-cli: the full cli+integration vitest hook trips on pre-existing
macOS bash 3.2 failures in untouched shell-harness suites (select/set -u);
CI runs bash 5.x green. Every new/changed file here was verified green
individually, and the checks registry + budget + gitleaks + typecheck pass.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@github-actions github-actions Bot added the v0.0.71 Release target label Jul 5, 2026
jyaunches pushed a commit that referenced this pull request Jul 6, 2026
…st it (#6086)

## Summary

Two regressions from #5874 (fixed in #6065) only surfaced in **live E2E
targets that don't run on PR CI**. This PR closes that class of gap: it
audits the live suite for behavior-critical assertions that are cheaply
mockable, backfills them as fast units that run on **every** PR, and
adds a guard so pure-unit blocks can't hide in live files again.

## What's here

**1. The two direct #6065 regression fences** (mocked shell-units)
- `reconcile`: an explicit `NEMOCLAW_MODEL_OVERRIDE` survives a
divergent gateway model and the stale in-file fallback; normal
drift-correction still runs when unset.
- `guard recovery`: the restore warning is mirrored into
`_NEMOCLAW_GATEWAY_LOG` (the marker the crash-loop E2E polls), and stays
silent when the chain is healthy.

**2. High-priority mockable backfill** (security/recovery class)
- ollama-auth-proxy: Bearer enforcement, no `/api/tags` bypass (#3338),
header stripping, non-ASCII auth no-crash (#4820), backend 502.
- `config get`: credential redaction + `gateway`-key omission (the
`nvapi-` regression class).
- device approval policy: scope-upgrade allowlist gate, gateway-env
stripping, recover-failed rejection paths (#4462).
- shields audit JSONL: credentials redacted before persistence.
- hermes env secret boundary: value-shape (not key-name) discriminator;
raw secrets rejected without echoing.
- dashboard bind: `NEMOCLAW_DASHBOARD_BIND` opt-in incl. negative cases
(#3259).
- whatsapp compact QR: package shape-detection + terminal-only `small`
(#4522).

**3. Medium/low backfill**
- ollama token-file lifecycle (0600 / persisted / divergent-repair);
extra-placeholder-keys canonical placeholder + accepted-keys breadcrumb;
hermes `remove_stale_gateway_file` symlink-safety; token-rotation
selective-rebuild naming; `_validate_port` fail-closed; snapshot `help`
branch.

**4. Regression guard**
- `scripts/checks/no-unit-blocks-in-live-e2e.ts` bans the vitest
`it(...)` primitive inside `test/e2e/live/**` (that glob is uncollected
on PR CI, so such blocks never run). Wired into the checks registry with
its own unit test.
- Relocated the existing offenders (skill-agent +
messaging-compatible-endpoint classifier blocks, plus the bare-`test(`
unit cases in common-egress + openclaw-inference-switch) into importable
`test/e2e/support` modules with PR-collected tests; the live tests
import them unchanged.

## Notes

- Minimal behavior-preserving refactor to `whatsapp-qr-compact.ts` to
export its pure helpers (the preload still auto-installs on require);
`tsconfig.runtime-preloads.json` excludes the new co-located test from
the shipped preload build.
- `nemoclaw-start.sh`: made two possibly-empty-array iterations
bash-3.2-safe via the existing `"${arr[@]+...}"` idiom so the shell-unit
harnesses run on stock macOS bash.
- **Deferred (2 low-value items):** the install.sh "Resolved install
ref:" log assertion and the OpenClaw anthropic plain-baseUrl assertion
both land in legacy-budget-capped test files where the growth guardrail
forbids bumping the budget; the anthropic contract is already covered on
the Hermes side and enforced live on the OpenClaw side.

## Verification

- Every new/changed test verified green individually across the `cli`,
`integration`, and `e2e-support` projects.
- `npm run checks` (incl. the new live-unit-block guard), test-file-size
budget, gitleaks, and CLI typecheck all pass.
- The full `test-cli` pre-commit hook was skipped locally only because
it trips on **pre-existing** macOS bash 3.2 failures in untouched
shell-harness suites (`select`/`set -u`); CI runs bash 5.x, where they
are green.

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>


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

* **Bug Fixes**
* Improved startup robustness for environment parsing and background
launch behavior.
* WhatsApp compact-QR rendering more consistently uses the compact
“terminal” style.
* Dashboard remote bind activates only when explicitly opted in via
`NEMOCLAW_DASHBOARD_BIND=0.0.0.0`.
* Tightened audit/config redaction to prevent secret leakage and omit
gateway details.
* **Tests**
* Expanded coverage for guard-chain recovery warnings, model override
precedence, Hermes env-boundary hardening, and proxy/policy correctness.
* **Chores**
* Added a CI safeguard to prevent unit-test primitives from being
included in live E2E tests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
ericksoa pushed a commit that referenced this pull request Jul 8, 2026
## Summary

Follow-up to merged PR #6065 to address the unaddressed PR Review
Advisor warnings:

- Adds safe gateway-log append helper for PID 1/root recovery warnings;
refuses symlink/non-regular/replaced log targets instead of appending
through them.
- Adds guard-chain recovery tests proving regular log append still works
and unsafe targets are refused.
- Adds focused reconciliation coverage proving `NEMOCLAW_MODEL_OVERRIDE`
wins over conflicting gateway state, plus no-override reconciliation
still runs.

## Test plan

- [x] `npx vitest run --project integration
test/nemoclaw-start-reconcile.test.ts
test/nemoclaw-start-guard-recovery.test.ts`

Refs #6065

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

* **Bug Fixes**
* Hardened gateway log writing to refuse unsafe targets (non-regular,
swapped, invalid), while sanitizing and newline-terminating log lines in
UTF-8.
* Recovery now records restoration warnings in the gateway log when
safe, without blocking startup if logging fails.
* Reconcile behavior improved: explicit model overrides are preserved;
otherwise configuration is reconciled from the live gateway model.
* **Tests**
* Expanded gateway recovery coverage with
regular/symlink/directory/missing log scenarios, validating behavior via
gateway-log contents.
  * Added reconcile tests for overridden vs non-overridden behavior.
* Updated gateway health watchdog tests to direct critical logging to
controlled per-test log files.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

## Advisor disposition / E2E evidence

PR Review Advisor `PRA-1` / `PRA-2` runtime-validation warnings are
resolved by the combination of in-PR helper hardening coverage plus live
E2E evidence:

- Source boundary: `append_openclaw_gateway_log_line` is the permanent
PID 1 append boundary and hardcodes canonical `/tmp/gateway.log`; it
intentionally refuses symlink, non-regular, replaced, or missing log
targets rather than honoring inherited path seams.
- Source-fix constraint: startup remains responsible for pre-creating
canonical `/tmp/gateway.log`; recovery/watchdog append paths must not
create a missing log because creation at recovery time would re-open the
unsafe target/ownership ambiguity this PR removes.
- Regression/unit coverage: `test/nemoclaw-start-guard-recovery.test.ts`
covers regular append, symlink refusal, non-regular refusal, missing-log
no-create, replaced-target refusal, inherited env ignored, and newline
sanitization. `test/nemoclaw-start-gateway-health.test.ts` covers
watchdog CRITICAL breadcrumb append through the same safe helper.
- Runtime validation: current-head E2E revalidation passed on
`9cda35bdaa6d426d160d262ac35c99c7449c1560`: run
https://github.com/NVIDIA/NemoClaw/actions/runs/28874860688 passed
required jobs `gateway-guard-recovery` and
`issue-2478-crash-loop-recovery`, and run
https://github.com/NVIDIA/NemoClaw/actions/runs/28874896411 passed
required target `ubuntu-repo-cloud-openclaw`. The earlier same-PR
artifact for `issue-2478-crash-loop-recovery` showed the real sandbox
respawn path emitted `[gateway-recovery] WARNING ... (#2478/#2701)` and
mirrored the same line as `[gateway-log:] [gateway-recovery] WARNING
...`, proving the real PID 1 startup/recovery lifecycle pre-created and
preserved the canonical gateway log before append.
- Removal condition: keep the helper-level contract tests until gateway
recovery/watchdog logging is no longer routed through `/tmp/gateway.log`
or the startup ownership contract is replaced by a different diagnostics
channel with equivalent live E2E coverage.

---------

Signed-off-by: cjagwani <cjagwani@nvidia.com>
Co-authored-by: cjagwani <cjagwani@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.71 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants