fix(status): show gateway active policy version, not the schema constant#3185
Merged
Conversation
`nemoclaw <sandbox> status` rendered the merged policy YAML's schema `version: 1` even after onboarding had advanced the gateway active version to 6, leaving users uncertain whether their preset selections took effect. Capture the `Active: N` line emitted by `openshell policy get --full` and rewrite the leading `version:` in the displayed YAML to match. Closes #1961 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughmergeLivePolicyIntoSandboxOutput now ANSI-strips live policy output, splits metadata from the YAML document, parses an Active: N value from metadata, and replaces sandbox YAML's version: M with that Active number when present; tests exercising normal and ANSI-wrapped cases were added. ChangesPolicy Version Fix
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 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/gateway-state.ts (1)
58-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winANSI-sensitive delimiter parsing can bypass the version rewrite path.
On Line 58, delimiter detection uses raw
livePolicyOutput, but Line 70 parses metadata from ANSI-stripped text. If---is colorized,delimIdxbecomes-1, metadata is treated as YAML, andversionwon’t be rewritten.Suggested fix
export function mergeLivePolicyIntoSandboxOutput( output: string, livePolicyOutput: string, ): string { @@ - const delimIdx = livePolicyOutput.search(/^---\s*$/m); - const metadataPart = delimIdx !== -1 ? livePolicyOutput.slice(0, delimIdx) : ""; + const cleanLivePolicy = stripAnsi(String(livePolicyOutput)); + const delimIdx = cleanLivePolicy.search(/^---\s*$/m); + const metadataPart = delimIdx !== -1 ? cleanLivePolicy.slice(0, delimIdx) : ""; const yamlPart = delimIdx !== -1 - ? livePolicyOutput.slice(delimIdx).replace(/^---\s*[\r\n]+/, "") - : livePolicyOutput; + ? cleanLivePolicy.slice(delimIdx).replace(/^---\s*[\r\n]+/, "") + : cleanLivePolicy; @@ - const activeMatch = stripAnsi(metadataPart).match(/^Active:\s*(\d+)\s*$/m); + const activeMatch = metadataPart.match(/^Active:\s*(\d+)\s*$/m);🤖 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 `@src/lib/actions/sandbox/gateway-state.ts` around lines 58 - 74, Determine the delimiter using an ANSI-stripped version of livePolicyOutput so color codes don't hide the `---` marker: call stripAnsi(livePolicyOutput) first, search for the delimiter on that stripped string to compute delimIdx, then derive metadataPart and yamlPart from the stripped text (instead of raw livePolicyOutput) before running the existing checks and version rewrite logic (references: livePolicyOutput, stripAnsi, delimIdx, metadataPart, yamlPart, activeMatch, rewrittenYaml).
🧹 Nitpick comments (1)
test/gateway-state.test.ts (1)
250-283: ⚡ Quick winAdd one ANSI-wrapped metadata test to lock this parser path.
These tests cover core behavior well, but they currently miss
Active/---lines with ANSI escapes. Adding that case would prevent regressions in colored CLI output.Suggested test addition
describe("mergeLivePolicyIntoSandboxOutput (`#1961`)", () => { @@ it("returns the original output when livePolicy is an error string", () => { const merged = mergeLivePolicyIntoSandboxOutput(sandboxOutput, "Error: not found"); expect(merged).toBe(sandboxOutput); }); + + it("rewrites version when metadata and separator are ANSI-wrapped", () => { + const livePolicy = [ + "\x1b[1mVersion:\x1b[0m 6", + "\x1b[1mActive:\x1b[0m 6", + "\x1b[36m---\x1b[0m", + "version: 1", + "filesystem_policy:", + " include_workdir: false", + ].join("\n"); + + const merged = mergeLivePolicyIntoSandboxOutput(sandboxOutput, livePolicy); + expect(merged).toContain(" version: 6"); + expect(merged).not.toContain(" version: 1"); + }); });🤖 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/gateway-state.test.ts` around lines 250 - 283, Add a test in test/gateway-state.test.ts for mergeLivePolicyIntoSandboxOutput that covers ANSI-escaped metadata lines: create a livePolicy string where the "Active:" metadata and the YAML document separator ("---") are wrapped with ANSI escape sequences (e.g., CSI codes) and assert that the merged output still rewrites the YAML "version" to the active value (e.g., expects " version: 6" and not " version: 1"); also include a negative case (e.g., when only ANSI-wrapped metadata is present but no Active value) to ensure the function correctly leaves YAML untouched. Ensure the new test references mergeLivePolicyIntoSandboxOutput so it exercises the ANSI-path of the parser.
🤖 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.
Outside diff comments:
In `@src/lib/actions/sandbox/gateway-state.ts`:
- Around line 58-74: Determine the delimiter using an ANSI-stripped version of
livePolicyOutput so color codes don't hide the `---` marker: call
stripAnsi(livePolicyOutput) first, search for the delimiter on that stripped
string to compute delimIdx, then derive metadataPart and yamlPart from the
stripped text (instead of raw livePolicyOutput) before running the existing
checks and version rewrite logic (references: livePolicyOutput, stripAnsi,
delimIdx, metadataPart, yamlPart, activeMatch, rewrittenYaml).
---
Nitpick comments:
In `@test/gateway-state.test.ts`:
- Around line 250-283: Add a test in test/gateway-state.test.ts for
mergeLivePolicyIntoSandboxOutput that covers ANSI-escaped metadata lines: create
a livePolicy string where the "Active:" metadata and the YAML document separator
("---") are wrapped with ANSI escape sequences (e.g., CSI codes) and assert that
the merged output still rewrites the YAML "version" to the active value (e.g.,
expects " version: 6" and not " version: 1"); also include a negative case
(e.g., when only ANSI-wrapped metadata is present but no Active value) to ensure
the function correctly leaves YAML untouched. Ensure the new test references
mergeLivePolicyIntoSandboxOutput so it exercises the ANSI-path of the parser.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 676816c4-d313-4b24-8aec-72118c1049d0
📒 Files selected for processing (2)
src/lib/actions/sandbox/gateway-state.tstest/gateway-state.test.ts
Contributor
|
✨ Thanks for submitting this PR that fixes the policy version mismatch in Related open issues: |
…-active-version-in-status Signed-off-by: Tinson Lai <tinsonl@nvidia.com> # Conflicts: # test/gateway-state.test.ts
Contributor
There was a problem hiding this comment.
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 `@test/gateway-state.test.ts`:
- Around line 295-309: The test must ensure Active is used rather than Version:
change the livePolicy fixture so the "Version:" and "Active:" lines have
different numbers (e.g., "Version: 5" and "Active: 6") and update
assertions to expect " version: 6" and to not contain " version: 5"; this
targets the mergeLivePolicyIntoSandboxOutput behavior using the livePolicy and
sandboxOutput variables so failures indicate the function is reading Active
rather than Version.
🪄 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: 33e76a48-b6a1-4048-8b2d-a91bb6923014
📒 Files selected for processing (2)
src/lib/actions/sandbox/gateway-state.tstest/gateway-state.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/sandbox/gateway-state.ts
…data When 'openshell policy get --full' emits ANSI-coloured output, the raw '---' delimiter search failed and the metadata path was treated as YAML, skipping the version rewrite. Strip ANSI on the full input so delimiter detection, metadata parsing and YAML extraction all see the same plain-text view. Also use distinct values for Version: and Active: in the regression test so it actually distinguishes which line is read, and add an ANSI-wrapped test case to lock in the colour-aware path. Per #3185 review feedback. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
cv
approved these changes
May 8, 2026
miyoungc
added a commit
that referenced
this pull request
May 9, 2026
## Summary - Bump the docs release metadata to `0.0.38`. - Document release-prep updates for status policy versions, Local Ollama validation and cleanup, blueprint policy additions, rebuild backup handling, and NemoHermes uninstall branding. - Refresh generated `nemoclaw-user-*` skills from the updated docs. ## Source summary - #3185 -> `docs/reference/commands.md`: Documents that `nemoclaw <name> status` displays the gateway active policy version when OpenShell reports one. - #3167 -> `docs/reference/commands.md`, `docs/inference/use-local-inference.md`: Documents uninstall cleanup for matching Local Ollama auth proxy processes. - #2737 -> `docs/inference/use-local-inference.md`, `docs/network-policy/customize-network-policy.md`, `docs/manage-sandboxes/lifecycle.md`, `docs/reference/commands.md`: Documents stricter Local Ollama tool-call validation, blueprint policy additions, and partial rebuild backup handling. - #3220 -> `docs/reference/commands.md`: Documents NemoHermes-specific uninstall progress and completion text. - #3158 -> `.agents/skills/nemoclaw-user-configure-inference/*`: Refreshes generated user skills from existing `docs/inference/switch-inference-providers.md` heartbeat documentation. - #3199 -> `.agents/skills/nemoclaw-user-get-started/SKILL.md`: Refreshes generated user skills from existing `docs/get-started/quickstart.md` Model Router wording. ## Skipped - #3272 and #3268 were already documented by their merged docs updates on `main`. - #3154, #3216, #3166, and #3195 have no additional user-facing docs impact for this release-prep pass. - No commits matched `docs/.docs-skip`. ## Test plan - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user` - `make docs` - `npm run build:cli` - Commit and pre-push hooks: markdownlint, docs-to-skills verification, gitleaks, commitlint, skills YAML tests, CLI typecheck Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Behavior Changes** * Rebuild now safely handles partial backups, preserving successfully captured entries while reporting only unarchived paths * Uninstall for Local Ollama setups now stops proxy processes before cleanup * Local Ollama models require stricter tool-call response validation during onboarding * Blueprint policy additions enable custom network policy extensions via `components.policy.additions` * New `NEMOCLAW_AGENT_HEARTBEAT_EVERY` configuration controls agent periodic task frequency * Status display now shows active policy version when available <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cursor <cursoragent@cursor.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
3 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
nemoclaw <sandbox> statusrendered the merged policy YAML's schemaversion: 1even after onboarding had advanced the gateway active version to 6, leaving users uncertain whether their preset selections actually took effect.Related Issue
Closes #1961
Changes
src/lib/actions/sandbox/gateway-state.ts: capture theActive: Nline emitted byopenshell policy get --full(the metadata block above---) and rewrite the leadingversion:in the YAML payload to that value before display.test/gateway-state.test.ts: cover the rewrite, the no-Active:fallback (YAML left untouched), and the error-output case (original output preserved).Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests