fix(sandbox): harden PR 6065 advisor follow-ups#6068
Merged
Conversation
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe 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.
TypeScript / code-coverage/cliThe overall coverage in the branch is 76%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
Contributor
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Findings index
Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 2 in-scope improvements
|
Contributor
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 in-scope improvements
|
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
21 tasks
wscurran
added
area: sandbox
Contributor
This was referenced Jul 6, 2026
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a fail-closed gateway log append helper, routes startup logging through it, expands gateway-log recovery and watchdog test coverage, and adds reconcile tests for model override precedence. ChangesGateway log appending safety
Reconcile model override tests
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/nemoclaw-start-reconcile.test.ts (1)
226-246: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSolid override-precedence test; minor clarity nitpick.
Using
NEMOCLAW_MODEL_OVERRIDE: "anthropic/claude-sonnet-4-6"— identical to the initial config's existing primary — still proves the override wins (via the unchanged-hash assertion confirming no write occurred), but a distinct override value would make the intent more immediately obvious to future readers without relying on the hash check alone.♻️ Optional: differentiate override from initial config's model
const initial = { - agents: { defaults: { model: { primary: "anthropic/claude-sonnet-4-6" } } }, + agents: { defaults: { model: { primary: "anthropic/claude-sonnet-4-5" } } }, models: { providers: { inference: { api: "openai-completions", - models: [{ id: "anthropic/claude-sonnet-4-6", name: "anthropic/claude-sonnet-4-6" }], + models: [{ id: "anthropic/claude-sonnet-4-5", name: "anthropic/claude-sonnet-4-5" }], }, }, }, }; const { result, config, hash } = runReconcile(initial, { env: { NEMOCLAW_MODEL_OVERRIDE: "anthropic/claude-sonnet-4-6" }, gatewayModel: "nvidia/nemotron-3-super-120b-a12b", });🤖 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 `@test/nemoclaw-start-reconcile.test.ts` around lines 226 - 246, The override-precedence test in runReconcile is valid, but the chosen NEMOCLAW_MODEL_OVERRIDE value matches the initial agents.defaults.model.primary and makes the intent less obvious. Update the test to use a distinct override model string while keeping the same assertions on result, config, and hash, so it is clear that the explicit override in runReconcile wins over the live gateway’s conflicting model.
🤖 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 `@test/nemoclaw-start-reconcile.test.ts`:
- Around line 226-246: The override-precedence test in runReconcile is valid,
but the chosen NEMOCLAW_MODEL_OVERRIDE value matches the initial
agents.defaults.model.primary and makes the intent less obvious. Update the test
to use a distinct override model string while keeping the same assertions on
result, config, and hash, so it is clear that the explicit override in
runReconcile wins over the live gateway’s conflicting model.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 947e118e-1735-492f-83d1-bf21a9ecccac
📒 Files selected for processing (3)
scripts/nemoclaw-start.shtest/nemoclaw-start-guard-recovery.test.tstest/nemoclaw-start-reconcile.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/nemoclaw-start-guard-recovery.test.ts (2)
22-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the
log_filerewrite actually matched.
String.prototype.replacewith a string pattern silently no-ops if the production line drifts (e.g. indentation/quoting change). If that happens,functionSourcekeepslocal log_file="/tmp/gateway.log"and every harness invocation wouldlstat/openthe real/tmp/gateway.log— producing confusing failures and, in the "regular" case, a stray write to the host log. A single positive assertion pins the seam.♻️ Guard against a silent no-op rewrite
const functionSource = extractShellFunction(source, "append_openclaw_gateway_log_line").replace( ' local log_file="/tmp/gateway.log"', ` local log_file=${JSON.stringify(gatewayLog)}`, ); + expect(functionSource, "gateway log path rewrite did not match").toContain( + `local log_file=${JSON.stringify(gatewayLog)}`, + );🤖 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 `@test/nemoclaw-start-guard-recovery.test.ts` around lines 22 - 43, The rewrite in extractGatewayLogAppendFunction is currently a silent no-op if the expected log_file line changes, so add an explicit assertion that the replacement of append_openclaw_gateway_log_line succeeded before returning functionSource. Use the existing extractShellFunction and the local log_file rewrite logic as the seam, and verify the source no longer contains the default /tmp/gateway.log binding after substitution so tests fail fast instead of accidentally hitting the real host log.
317-322: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSource-text assertion; keep as a regression guard but consider anchoring on behavior.
Per path instructions, prefer observable outcomes through the public boundary over source-text assertions. This test only greps
nemoclaw-start.sh, so it can't catch a routing regression that still matches the string but behaves incorrectly. It does satisfy the migration requirement of proving the superseded>> "$LOG"path is removed, so it's acceptable as a complementary guard alongside the behavioral symlink/directory/missing tests — just be aware it locks source text rather than behavior.🤖 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 `@test/nemoclaw-start-guard-recovery.test.ts` around lines 317 - 322, The watchdog gateway-log test is still a source-text assertion, so keep it only as a complementary regression guard and add/lean on a behavioral check instead. In nemoclaw-start-guard-recovery.test.ts, anchor the assertion around the observable output of the start path exercised by the public boundary rather than only grepping nemoclaw-start.sh, using the existing symlink/directory/missing recovery cases and the append_openclaw_gateway_log_line flow to verify routing. Keep the removal check for the superseded redirection path, but make the primary validation behavior-based so a future routing regression won’t slip through while the source string still matches.Source: Path instructions
🤖 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 `@test/nemoclaw-start-guard-recovery.test.ts`:
- Around line 22-43: The rewrite in extractGatewayLogAppendFunction is currently
a silent no-op if the expected log_file line changes, so add an explicit
assertion that the replacement of append_openclaw_gateway_log_line succeeded
before returning functionSource. Use the existing extractShellFunction and the
local log_file rewrite logic as the seam, and verify the source no longer
contains the default /tmp/gateway.log binding after substitution so tests fail
fast instead of accidentally hitting the real host log.
- Around line 317-322: The watchdog gateway-log test is still a source-text
assertion, so keep it only as a complementary regression guard and add/lean on a
behavioral check instead. In nemoclaw-start-guard-recovery.test.ts, anchor the
assertion around the observable output of the start path exercised by the public
boundary rather than only grepping nemoclaw-start.sh, using the existing
symlink/directory/missing recovery cases and the
append_openclaw_gateway_log_line flow to verify routing. Keep the removal check
for the superseded redirection path, but make the primary validation
behavior-based so a future routing regression won’t slip through while the
source string still matches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b0f7979f-2635-4ce3-a9e9-44634df76313
📒 Files selected for processing (2)
scripts/nemoclaw-start.shtest/nemoclaw-start-guard-recovery.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/nemoclaw-start-gateway-health.test.ts (1)
50-56: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an assertion that the
log_filesubstitution actually matched.
extractGatewayLogAppendFunctionusesString.replaceto rewrite the hardcoded/tmp/gateway.logpath, but unlike the siblingextractShellFunction(which callsexpect(...).not.toBe(-1)), there's no check that the replacement text was found. If the production literal ever changes (e.g., a formatting/quoting tweak inscripts/nemoclaw-start.sh), this silently no-ops: the generated helper would still target the real/tmp/gateway.log, and since the helper never creates a missing file, the test would silently fail to observe the log write (or, worse, write into a shared path) instead of failing loudly with a clear diagnostic.🛠️ Proposed fix
function extractGatewayLogAppendFunction(src: string, gatewayLog: string): string { - return extractShellFunction(src, "append_openclaw_gateway_log_line").replace( - ' local log_file="/tmp/gateway.log"', - ` local log_file=${JSON.stringify(gatewayLog)}`, - ); + const fn = extractShellFunction(src, "append_openclaw_gateway_log_line"); + const marker = ' local log_file="/tmp/gateway.log"'; + expect(fn.includes(marker), "Expected log_file assignment in append_openclaw_gateway_log_line").toBe(true); + return fn.replace(marker, ` local log_file=${JSON.stringify(gatewayLog)}`); }🤖 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 `@test/nemoclaw-start-gateway-health.test.ts` around lines 50 - 56, `extractGatewayLogAppendFunction` currently rewrites the `append_openclaw_gateway_log_line` shell helper with a blind String.replace, so add a check that the hardcoded `log_file` literal is actually present before replacing it, similar to `extractShellFunction`. If the expected `/tmp/gateway.log` text is missing, fail the test immediately with a clear assertion so changes in `scripts/nemoclaw-start.sh` don’t silently leave the helper pointing at the real path.
🤖 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 `@test/nemoclaw-start-gateway-health.test.ts`:
- Around line 50-56: `extractGatewayLogAppendFunction` currently rewrites the
`append_openclaw_gateway_log_line` shell helper with a blind String.replace, so
add a check that the hardcoded `log_file` literal is actually present before
replacing it, similar to `extractShellFunction`. If the expected
`/tmp/gateway.log` text is missing, fail the test immediately with a clear
assertion so changes in `scripts/nemoclaw-start.sh` don’t silently leave the
helper pointing at the real path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 36643c17-7d4b-4831-ac36-b511227c04e0
📒 Files selected for processing (2)
test/nemoclaw-start-gateway-health.test.tstest/nemoclaw-start-guard-recovery.test.ts
💤 Files with no reviewable changes (1)
- test/nemoclaw-start-guard-recovery.test.ts
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28871463038
|
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28874896411
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28874860688
|
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Contributor
Security review — exact head
|
| # | Category | Severity | File | Result |
|---|---|---|---|---|
| 1 | TOCTOU / availability | Medium, resolved | scripts/nemoclaw-start.sh |
Added O_NONBLOCK so a regular file swapped to a FIFO after lstat cannot block root before fstat; added a behavioral FIFO-swap regression. |
| 2 | Test isolation | Low, resolved | gateway-health + guard-recovery tests | Assert the canonical log-path rewrite matched before running extracted shell, preventing accidental writes to the host /tmp/gateway.log. |
Nine-category review:
- Secrets and credentials — PASS. No credential material added or logged; messages are fixed operational diagnostics.
- Input validation and sanitization — PASS. CR/LF are flattened, the canonical path is not inherited, and argv carries the message without shell evaluation.
- Authentication and authorization — PASS. No auth surface changed; the PID 1 boundary does not grant callers path control.
- Dependencies — PASS. No dependency changes; Python runs isolated with
-I. - Error handling and logging — PASS. Missing/unsafe/replaced targets do not get created or followed; logging failure remains non-fatal to recovery.
- Cryptography and data protection — PASS. Not applicable; no cryptographic behavior changes.
- Configuration and secure defaults — PASS. The helper hardcodes
/tmp/gateway.log, uses append-only/no-follow/close-on-exec/nonblocking flags, and preserves startup-owned creation. - Security testing — PASS. Covers regular, symlink, directory, missing, regular replacement, FIFO replacement, inherited-env, and newline cases; exact-head live E2E evidence remains linked in the PR.
- Holistic posture — PASS. The
lstat/open/fstatidentity check plus nonblocking open closes the discovered swap-to-FIFO denial-of-service window; no sandbox escape, policy bypass, or secret exposure found.
Local verification on Node 22: 61/61 affected integration tests passed, bash -n passed, Biome format/lint passed, and the test-size budget passed.
Signed-off-by: cjagwani <cjagwani@nvidia.com>
cjagwani
approved these changes
Jul 8, 2026
cjagwani
left a comment
cjagwani
left a comment
Contributor
There was a problem hiding this comment.
Approved exact head 15634efa after the nine-category security review. The canonical log path is fixed and not environment-controlled; no-follow + append + close-on-exec + nonblocking open is followed by regular-file and inode identity verification; unsafe/missing/replaced targets do not get created or written; logging failures remain non-fatal to recovery. The added FIFO-swap regression closes the discovered blocking TOCTOU case, and all exact-head required checks are green.
21 tasks
cv
pushed a commit
that referenced
this pull request
Jul 9, 2026
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user documentation for NemoClaw v0.0.78 by replacing the unreleased section with release highlights and synchronizing the affected inference, lifecycle, messaging, and CLI reference pages with merged behavior. ## Changes - Publish the v0.0.78 release-notes section with links to the most specific user guides for each shipped behavior. - Document authoritative Deep Agents route health, Nemotron Ultra profile behavior, and Hermes compatible-endpoint context metadata. - Document forced rebuild recovery after total backup failure and the ownership-safe tunnel/full-stop behavior. - Keep command examples and shared agent variants aligned with the current OpenClaw, Hermes, and Deep Agents interfaces. Source mapping: - [#3787](#3787) -> `docs/about/release-notes.mdx`: Record reliable workspace template seeding during sandbox startup. - [#4960](#4960) -> `docs/about/release-notes.mdx`: Record safer detection of rewritten OpenClaw gateway processes. - [#5676](#5676) -> `docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON handling. - [#5857](#5857) -> `docs/about/release-notes.mdx`: Record synchronization of explicit OpenClaw main-agent model state. - [#5929](#5929) -> `docs/about/release-notes.mdx`: Record copyable SSH port-forward guidance for remote dashboards. - [#6068](#6068) -> `docs/about/release-notes.mdx`: Record custom-image plugin provenance reconciliation. - [#6116](#6116) -> `docs/about/release-notes.mdx`: Record live-loopback dashboard-forward recovery. - [#6122](#6122) -> `docs/about/release-notes.mdx`: Announce validated, round-trippable policy YAML output. - [#6211](#6211) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild --force` recovery boundary. - [#6283](#6283) -> `docs/about/release-notes.mdx`: Record Hermes WebUI port alignment. - [#6293](#6293) -> `docs/inference/switch-inference-providers.mdx`, `docs/about/release-notes.mdx`: Document compatible-endpoint context-length probing for Hermes. - [#6320](#6320) -> `docs/about/release-notes.mdx`: Record bounded gateway-recovery waits. - [#6377](#6377) -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain rebuild diagnostics and prepared MCP-destroy recovery. - [#6412](#6412) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document authoritative agent-visible inference route health. - [#6421](#6421) -> `docs/about/release-notes.mdx`: Record the longer quiet-pull window for managed vLLM images. - [#6431](#6431) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document the version-pinned Nemotron Ultra profile plugin. - [#6439](#6439) -> `docs/about/release-notes.mdx`: Summarize the authenticated, pinned credential-capture helper boundary. - [#6450](#6450) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document host-forward cleanup and ownership-safe gateway-port release. - [#6474](#6474) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/about/release-notes.mdx`: Record composable OpenClaw messaging runtime loaders. - [#6475](#6475) -> `docs/about/release-notes.mdx`: Record removal of the unavailable Kimi K2.6 production endpoint option. - [#6480](#6480) -> `docs/about/release-notes.mdx`: Record stderr routing for the plugin registration banner. - [#6481](#6481) -> `docs/about/release-notes.mdx`: Record post-pull Ollama model discovery checks. - [#6482](#6482) -> `docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon restart. - [#6486](#6486) -> `docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep Agents auto-approval boundary. - [#6490](#6490) -> `docs/about/release-notes.mdx`: Record diagnostics for custom images missing the managed runtime. - [#6494](#6494) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document nonempty tool-call content preservation and placeholder rejection. - [#6497](#6497) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document isolated Deep Agents route-probe output. - [#6506](#6506) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document observability-preserving managed route probes. - [#6508](#6508) -> `docs/about/release-notes.mdx`: Link the new extension taxonomy and SDK-readiness reference from the release summary. Release-source verification: GitHub reports all 29 cited source PRs as merged with base `main`, and every merge commit is an ancestor of `origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No source-mapping mismatches were found. ## 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 exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Documentation-only release-prep changes; `npm run docs` validates variants, routes, and Fern content. - [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 applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests are not applicable to this documentation-only change set. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — exited 0 with zero errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast 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: Charan Jagwani <cjagwani@nvidia.com> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Follow-up to merged PR NVIDIA#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 NVIDIA#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 ... (NVIDIA#2478/NVIDIA#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>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user documentation for NemoClaw v0.0.78 by replacing the unreleased section with release highlights and synchronizing the affected inference, lifecycle, messaging, and CLI reference pages with merged behavior. ## Changes - Publish the v0.0.78 release-notes section with links to the most specific user guides for each shipped behavior. - Document authoritative Deep Agents route health, Nemotron Ultra profile behavior, and Hermes compatible-endpoint context metadata. - Document forced rebuild recovery after total backup failure and the ownership-safe tunnel/full-stop behavior. - Keep command examples and shared agent variants aligned with the current OpenClaw, Hermes, and Deep Agents interfaces. Source mapping: - [NVIDIA#3787](NVIDIA#3787) -> `docs/about/release-notes.mdx`: Record reliable workspace template seeding during sandbox startup. - [NVIDIA#4960](NVIDIA#4960) -> `docs/about/release-notes.mdx`: Record safer detection of rewritten OpenClaw gateway processes. - [NVIDIA#5676](NVIDIA#5676) -> `docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON handling. - [NVIDIA#5857](NVIDIA#5857) -> `docs/about/release-notes.mdx`: Record synchronization of explicit OpenClaw main-agent model state. - [NVIDIA#5929](NVIDIA#5929) -> `docs/about/release-notes.mdx`: Record copyable SSH port-forward guidance for remote dashboards. - [NVIDIA#6068](NVIDIA#6068) -> `docs/about/release-notes.mdx`: Record custom-image plugin provenance reconciliation. - [NVIDIA#6116](NVIDIA#6116) -> `docs/about/release-notes.mdx`: Record live-loopback dashboard-forward recovery. - [NVIDIA#6122](NVIDIA#6122) -> `docs/about/release-notes.mdx`: Announce validated, round-trippable policy YAML output. - [NVIDIA#6211](NVIDIA#6211) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild --force` recovery boundary. - [NVIDIA#6283](NVIDIA#6283) -> `docs/about/release-notes.mdx`: Record Hermes WebUI port alignment. - [NVIDIA#6293](NVIDIA#6293) -> `docs/inference/switch-inference-providers.mdx`, `docs/about/release-notes.mdx`: Document compatible-endpoint context-length probing for Hermes. - [NVIDIA#6320](NVIDIA#6320) -> `docs/about/release-notes.mdx`: Record bounded gateway-recovery waits. - [NVIDIA#6377](NVIDIA#6377) -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain rebuild diagnostics and prepared MCP-destroy recovery. - [NVIDIA#6412](NVIDIA#6412) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document authoritative agent-visible inference route health. - [NVIDIA#6421](NVIDIA#6421) -> `docs/about/release-notes.mdx`: Record the longer quiet-pull window for managed vLLM images. - [NVIDIA#6431](NVIDIA#6431) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document the version-pinned Nemotron Ultra profile plugin. - [NVIDIA#6439](NVIDIA#6439) -> `docs/about/release-notes.mdx`: Summarize the authenticated, pinned credential-capture helper boundary. - [NVIDIA#6450](NVIDIA#6450) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document host-forward cleanup and ownership-safe gateway-port release. - [NVIDIA#6474](NVIDIA#6474) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/about/release-notes.mdx`: Record composable OpenClaw messaging runtime loaders. - [NVIDIA#6475](NVIDIA#6475) -> `docs/about/release-notes.mdx`: Record removal of the unavailable Kimi K2.6 production endpoint option. - [NVIDIA#6480](NVIDIA#6480) -> `docs/about/release-notes.mdx`: Record stderr routing for the plugin registration banner. - [NVIDIA#6481](NVIDIA#6481) -> `docs/about/release-notes.mdx`: Record post-pull Ollama model discovery checks. - [NVIDIA#6482](NVIDIA#6482) -> `docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon restart. - [NVIDIA#6486](NVIDIA#6486) -> `docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep Agents auto-approval boundary. - [NVIDIA#6490](NVIDIA#6490) -> `docs/about/release-notes.mdx`: Record diagnostics for custom images missing the managed runtime. - [NVIDIA#6494](NVIDIA#6494) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document nonempty tool-call content preservation and placeholder rejection. - [NVIDIA#6497](NVIDIA#6497) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document isolated Deep Agents route-probe output. - [NVIDIA#6506](NVIDIA#6506) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document observability-preserving managed route probes. - [NVIDIA#6508](NVIDIA#6508) -> `docs/about/release-notes.mdx`: Link the new extension taxonomy and SDK-readiness reference from the release summary. Release-source verification: GitHub reports all 29 cited source PRs as merged with base `main`, and every merge commit is an ancestor of `origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No source-mapping mismatches were found. ## 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 exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Documentation-only release-prep changes; `npm run docs` validates variants, routes, and Fern content. - [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 applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests are not applicable to this documentation-only change set. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — exited 0 with zero errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast 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: Charan Jagwani <cjagwani@nvidia.com> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to merged PR #6065 to address the unaddressed PR Review Advisor warnings:
NEMOCLAW_MODEL_OVERRIDEwins over conflicting gateway state, plus no-override reconciliation still runs.Test plan
npx vitest run --project integration test/nemoclaw-start-reconcile.test.ts test/nemoclaw-start-guard-recovery.test.tsRefs #6065
Summary by CodeRabbit
Advisor disposition / E2E evidence
PR Review Advisor
PRA-1/PRA-2runtime-validation warnings are resolved by the combination of in-PR helper hardening coverage plus live E2E evidence:append_openclaw_gateway_log_lineis 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./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.test/nemoclaw-start-guard-recovery.test.tscovers 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.tscovers watchdog CRITICAL breadcrumb append through the same safe helper.9cda35bdaa6d426d160d262ac35c99c7449c1560: run https://github.com/NVIDIA/NemoClaw/actions/runs/28874860688 passed required jobsgateway-guard-recoveryandissue-2478-crash-loop-recovery, and run https://github.com/NVIDIA/NemoClaw/actions/runs/28874896411 passed required targetubuntu-repo-cloud-openclaw. The earlier same-PR artifact forissue-2478-crash-loop-recoveryshowed 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./tmp/gateway.logor the startup ownership contract is replaced by a different diagnostics channel with equivalent live E2E coverage.