fix(status): show live enforced policy instead of stale baseline (#1132) - #1896
Conversation
`nemoclaw status` displays the policy detail from `openshell sandbox get`, which returns the immutable baseline policy from sandbox creation time. Network policies added later via `nemoclaw policy-add` (which calls `openshell policy set`) are not reflected in this baseline — by design, OpenShell preserves it for static field validation (filesystem_policy, landlock, process fields are kernel-level and irreversible). This causes the Policy section in `nemoclaw status` to be missing policies that were successfully applied and are actively enforced. Fix: In `getSandboxGatewayState`, after a successful `sandbox get`, fetch the live policy via `openshell policy get --full` and replace the Policy section in the output. The Sandbox info and colored "Policy:" header are preserved from the original output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Use regex with multiline flag to match "---" separator regardless of line endings (\n, \r\n) or trailing whitespace. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
…overage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdded async recoverNamedGatewayRuntime() to attempt recovering a named gateway (select, recheck, optionally start) and enhanced getSandboxGatewayState() to fetch live policy via openshell, extract YAML from the first Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Nemoclaw
participant Openshell
participant GatewayRuntime
Caller->>Nemoclaw: recoverNamedGatewayRuntime(name)
Nemoclaw->>GatewayRuntime: selectNamedGateway(name)
Nemoclaw->>GatewayRuntime: getLifecycleState()
alt lifecycle != "healthy_named"
Nemoclaw->>Openshell: sandbox get (capture output)
Openshell-->>Nemoclaw: sandbox output (ANSI colored)
Nemoclaw->>Openshell: policy get --full sandboxName (ignore errors)
Openshell-->>Nemoclaw: full policy output (may contain '---' YAML)
Nemoclaw->>Nemoclaw: extract YAML from first '---', validate, re-indent
Nemoclaw->>Nemoclaw: replace Policy: section in sandbox output
alt allowed by lifecycle guards
Nemoclaw->>GatewayRuntime: startGatewayForRecovery()
GatewayRuntime-->>Nemoclaw: new lifecycle state
end
Nemoclaw->>GatewayRuntime: re-select and re-check lifecycle
Nemoclaw->>Nemoclaw: set process.env.OPENSHELL_GATEWAY on success
end
Nemoclaw-->>Caller: recovery result {attempted, via, state, output}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nemoclaw.ts`:
- Around line 562-578: The code replaces the "Policy:" section whenever
livePolicy.output is non-empty; instead, only replace when the extracted
yamlPart actually looks like a policy YAML. After computing delimIdx and
yamlPart, validate yamlPart with a regex for expected top-level policy keys (for
example /(^|\n)\s*(name|rules|version)\s*:/m) or reuse the existing guard logic
from policies.ts; if the check passes proceed to indent and replace (using
yamlPart), otherwise leave output unchanged so warnings/status text isn't
mistaken for policy YAML (references: livePolicy, policyLineIdx, delimIdx,
yamlPart).
🪄 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: Pro Plus
Run ID: c0e8000e-8cd3-4ec3-b1a3-9047250aff18
📒 Files selected for processing (1)
src/nemoclaw.ts
Only replace the Policy section when the extracted content looks like valid policy YAML (starts with a YAML key). Prevents replacing with warnings or status text from unexpected openshell output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
…verage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nemoclaw.ts (1)
576-584: Consider adding error/warning prefix check for robustness.The YAML-key regex guard at line 580 doesn't prevent error messages from being treated as valid policy content. Messages like
error: connection refusedorwarning: timeoutmatch the pattern/^[a-z_][a-z0-9_]*\s*:/msinceerror:andwarning:look like YAML keys.The existing
parseCurrentPolicy()insrc/lib/policies.ts(lines 87-89) guards against this with an explicit prefix check before the YAML-key test.🛡️ Proposed fix to add error prefix guard
// Guard: only replace if the extracted content looks like policy YAML // (starts with a YAML key like "version:" or "network_policies:"). // Avoids replacing with warnings or status text from unexpected output. const trimmedYaml = yamlPart.trim(); - if (trimmedYaml && /^[a-z_][a-z0-9_]*\s*:/m.test(trimmedYaml)) { + const looksLikeError = /^(error|failed|invalid|warning|status)\b/i.test(trimmedYaml); + if (trimmedYaml && !looksLikeError && /^[a-z_][a-z0-9_]*\s*:/m.test(trimmedYaml)) { // Add 2-space indent to match the original sandbox get output format. const indented = trimmedYaml.split("\n").map((l) => (l ? " " + l : l)).join("\n"); output = before + "\n\n" + indented + "\n"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nemoclaw.ts` around lines 576 - 584, The YAML guard in nemoclaw.ts currently treats lines like "error: ..." or "warning: ..." as valid YAML keys; update the validation around trimmedYaml (the block that computes indented and assigns output) to first reject strings that begin with known non-policy prefixes (e.g., /^ *(error|warning|status):/i) before applying the /^[a-z_][a-z0-9_]*\s*:/m test, mirroring the prefix check used in parseCurrentPolicy() in src/lib/policies.ts; ensure you reference trimmedYaml and keep the existing indentation and output assignment logic only for content that passes the new prefix filter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/nemoclaw.ts`:
- Around line 576-584: The YAML guard in nemoclaw.ts currently treats lines like
"error: ..." or "warning: ..." as valid YAML keys; update the validation around
trimmedYaml (the block that computes indented and assigns output) to first
reject strings that begin with known non-policy prefixes (e.g., /^
*(error|warning|status):/i) before applying the /^[a-z_][a-z0-9_]*\s*:/m test,
mirroring the prefix check used in parseCurrentPolicy() in src/lib/policies.ts;
ensure you reference trimmedYaml and keep the existing indentation and output
assignment logic only for content that passes the new prefix filter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 26c7bd0f-4516-464e-b450-8af8c89c94b4
📒 Files selected for processing (1)
src/nemoclaw.ts
Add explicit prefix check for error/warning/status text before the YAML-key regex, mirroring parseCurrentPolicy() in policies.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nemoclaw.ts (1)
559-585: Add regression coverage for the policy-output parser.This now depends on delimiter detection plus heuristic YAML guards before rewriting the
Policy:section. Please add focused tests for at least: validpolicy get --fullYAML, exit-0 warning/status text, and output with noPolicy:header so future CLI output changes don't silently reintroduce stale or malformed status output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nemoclaw.ts` around lines 559 - 585, Add regression tests for the policy-output parser that exercise the new delimiter detection and YAML-guard heuristics: write focused tests that simulate captureOpenshell returning (1) a valid `policy get --full` output whose YAML begins at the `---` delimiter and should be preserved and indented (exercise the delimIdx, yamlPart, trimmedYaml and indented logic), (2) an exit-0 output that contains only warning/status text (e.g. lines starting with "warning", "status", "failed", which should trigger looksLikeError and NOT replace the Policy: section), and (3) output that contains no "Policy:" header (policyLineIdx === -1) to ensure no rewrite occurs; implement these by stubbing/mocking the captureOpenshell result (livePolicy.output and livePolicy.status) and asserting the final output value produced by the parser code in nemoclaw.ts (the block referencing livePolicy, policyLineIdx, delimIdx, yamlPart, trimmedYaml and looksLikeError).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/nemoclaw.ts`:
- Around line 559-585: Add regression tests for the policy-output parser that
exercise the new delimiter detection and YAML-guard heuristics: write focused
tests that simulate captureOpenshell returning (1) a valid `policy get --full`
output whose YAML begins at the `---` delimiter and should be preserved and
indented (exercise the delimIdx, yamlPart, trimmedYaml and indented logic), (2)
an exit-0 output that contains only warning/status text (e.g. lines starting
with "warning", "status", "failed", which should trigger looksLikeError and NOT
replace the Policy: section), and (3) output that contains no "Policy:" header
(policyLineIdx === -1) to ensure no rewrite occurs; implement these by
stubbing/mocking the captureOpenshell result (livePolicy.output and
livePolicy.status) and asserting the final output value produced by the parser
code in nemoclaw.ts (the block referencing livePolicy, policyLineIdx, delimIdx,
yamlPart, trimmedYaml and looksLikeError).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4bbb6d51-bd72-4168-9708-a5b6c264bdcc
📒 Files selected for processing (1)
src/nemoclaw.ts
## Summary - Document `nemoclaw <name> snapshot create/list/restore` commands (from #1892) - Document `nemoclaw <name> policy-remove` command (from #1822) - Document `nemoclaw <name> rebuild` command with version staleness detection (from #1870) - Document `nemoclaw backup-all` command (from #1870) - Update `status` to mention live enforced policy display (from #1896) - Update `connect` and `status` to mention version staleness warnings (from #1870) - Update `destroy` to reference snapshots and rebuild as alternatives - Add snapshot commands section to backup-restore page - Add `policy-remove` to customize-network-policy page - Add "Sandbox is running an outdated agent version" troubleshooting entry - Bump doc version switcher through 0.0.16 - Regenerate agent skills from updated docs ## Test plan - [x] `make docs` builds without warnings - [x] All pre-commit hooks pass - [ ] Verify rendered pages in docs build output 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added documentation for policy preset removal command with selection flow * Documented new rebuild command for sandbox upgrades while preserving workspace state * Introduced snapshot commands for creating, listing, and restoring workspace backups * Added bulk backup capability for multiple sandboxes * Enhanced agent version warnings in connect and status commands with remediation guidance * Expanded troubleshooting guide with outdated agent version resolution steps <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Fixes #1132
nemoclaw statusdisplays policy details fromopenshell sandbox get, which returns the immutable baseline policy from sandbox creation time. Network policies added later vianemoclaw policy-add/openshell policy setare not reflected in this baseline, causing the Policy section to be missing actively enforcedpolicies.
Root Cause
OpenShell maintains two separate storage locations for policy:
sandbox.spec.policy(baseline) — Written once at sandbox creation. Used byvalidate_static_fields_unchanged()to ensure static fields (filesystem_policy,landlock,process) cannot be modified after creation, since these are kernel-level restrictions (Landlock, UID/GID) that are irreversible. Returnedby
openshell sandbox get.sandbox_policiestable (runtime revisions) — Updated on everyopenshell policy set. The sandbox proxy reads the latest revision to enforce network access rules. Returned byopenshell policy get --full.When
openshell policy setis called, it validates the new policy against the baseline, writes a new revision to the runtime table, but intentionally does NOT updatesandbox.spec.policy— preserving the immutable security baseline. This meansopenshell sandbox getnever reflects post-creation policy changes.Fix
In
getSandboxGatewayState(), after a successfulopenshell sandbox get, fetch the live policy viaopenshell policy get --fulland replace the Policy section in the output. The Sandbox info and colored "Policy:" header are preserved from the original output.Reproduction & Verification
Before fix —
nemoclaw statusPolicy section missing slack afterpolicy-add:nemoclaw lyy policy-listnemoclaw lyy statusheaderopenshell policy get --full lyyapi.slack.comreturns{"ok":true}nemoclaw lyy statuspolicy detailopenshell sandbox get lyynemoclaw statusPolicy section missing slack afterpolicy-add:After fix — Policy detail section now shows all enforced policies including slack, pypi, npm_yarn (all policies added post-creation are visible).
Policy section shows all enforced policies including slack:
Test plan
nemoclaw policy-addslack →nemoclaw statusnow shows slack in Policy detailpolicy get --fullfails, original output shown unchangedSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Signed-off-by: Yanyun Liao yanyunl@nvidia.com