fix(policies): cross-reference gateway state in policy-list to detect desync

[yimoj]

Summary

policy-list previously read only from CLI-side registry metadata (~/.nemoclaw/sandboxes.json), not from the gateway's actual loaded policy. After rebuild, this caused presets to show as ○ (not applied) while the gateway still enforced them. This PR adds gateway cross-referencing so policy-list shows the true state of both sources.

Related Issue

Closes #2010

Changes

• src/lib/policies.ts — Add getGatewayPresets(sandboxName) which queries the gateway via openshell policy get --full, parses the YAML response, and matches network_policies keys against known preset definitions. Returns null when gateway is unreachable vs [] when reachable but empty.
• src/nemoclaw.ts — Update sandboxPolicyList() to compare registry and gateway state, showing discrepancy markers:
  • ● (active on gateway, missing from local state) — the [All Platforms]nemoclaw rebuild: policy-list shows telegram as ○ (not applied) but gateway still allows telegram traffic — policy state inconsistency #2010 scenario
  • ○ (recorded locally, not active on gateway) — inverse desync
  • Falls back to registry-only display with ⚠ warning when gateway is unreachable
• test/repro-2010.test.ts — 9 tests covering:
  • getGatewayPresets matching logic via subprocess (telegram-only, multi-preset, npm exclusion, unreachable gateway, empty policy)
  • sandboxPolicyList CLI output via subprocess (all 4 marker/suffix states including gateway-unreachable fallback)

Type of Change

• Code change for a new feature, bug fix, or refactor.
• Code change with doc updates.
• Doc only. Prose changes without code sample modifications.
• Doc only. Includes code sample changes.

Testing

• npx prek run --all-files passes (or equivalently make check).
• npm test passes.
• make docs builds without warnings. (for doc-only changes)

71 tests pass (62 existing policy tests + 9 new). TypeScript typecheck, ESLint, and Prettier all clean.

Checklist

General

• I have read and followed the contributing guide.

Signed-off-by: Yimo Jiang yimoj@nvidia.com

[coderabbitai]

📝 Walkthrough

Walkthrough

Added getGatewayPresets(sandboxName) to read the gateway's loaded policy and updated sandboxPolicyList() to compare registry-applied presets with gateway-reported presets, showing discrepancy markers and a gateway-unreachable fallback. Added tests that reproduce registry/gateway divergence scenarios.

Changes

Cohort / File(s)	Summary
Gateway State Integration src/lib/policies.ts	New exported getGatewayPresets(sandboxName) runs policy get --full against the sandbox gateway, parses the loaded YAML via parseCurrentPolicy, and returns: null on command failure/unparseable output, [] when YAML contains no usable network_policies, or an array of preset names whose network_policies keys are all present in the gateway policy.
Policy List Display Enhancement src/nemoclaw.ts	sandboxPolicyList() now queries both getAppliedPresets() (local registry) and getGatewayPresets() (gateway), selects per-preset markers by comparing the two sources, appends explanatory suffixes for mismatches, and falls back to registry-only behavior with a warning when gateway returns null.
Repro Test Suite test/repro-2010.test.ts	Added Vitest suite that spawns subprocesses to exercise getGatewayPresets() using synthesized gateway YAML (including gateway-unreachable and empty-network_policies cases) and validates CLI rendering for gateway-only, registry-only, synced, and unreachable scenarios with mocked registry/policies functions.

Sequence Diagram

sequenceDiagram
    participant User as User / CLI
    participant Nemoclaw as nemoclaw.ts
    participant Registry as policies (local registry)
    participant GatewayProc as Gateway process

    User->>Nemoclaw: sandboxPolicyList(sandboxName)
    Nemoclaw->>Registry: getAppliedPresets(sandboxName)
    Registry-->>Nemoclaw: [local preset list]
    Nemoclaw->>GatewayProc: getGatewayPresets(sandboxName) (runs `policy get --full`)
    alt Gateway Reachable & YAML parseable
        GatewayProc-->>Nemoclaw: [gateway preset list]
        rect rgba(100, 200, 100, 0.5)
        Nemoclaw->>Nemoclaw: compare local vs gateway → determine markers/suffixes
        end
    else Gateway Unreachable or parse failure
        GatewayProc-->>Nemoclaw: null
        rect rgba(200, 100, 100, 0.5)
        Nemoclaw->>User: Warning: showing local state only
        Nemoclaw->>Nemoclaw: use registry-only markers
        end
    end
    Nemoclaw->>User: Render preset list with markers and mismatch suffixes

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped to check the gateway's light,

comparing keys by moonlit night.
If things don't match, I twitch my nose,
and mark the gaps where mismatch shows.
A little sync, a joyful thump — policies align, then I will jump!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding gateway state cross-referencing to the policy-list command to detect desynchronization between CLI registry and gateway enforcement.
Linked Issues check	✅ Passed	The PR fully addresses issue #2010 by implementing gateway state querying via getGatewayPresets(), comparing it with registry state, displaying discrepancy markers, and providing a fallback when the gateway is unreachable.
Out of Scope Changes check	✅ Passed	All changes directly support the objective to reflect gateway state in policy-list output and detect desync; no unrelated modifications detected in policies.ts, nemoclaw.ts, or the new test file.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

test/repro-2010.test.js (1)

106-133: Consider using vi.spyOn for more idiomatic Vitest mocking.

The direct property mutation with try/finally restore works but vi.spyOn with mockImplementation would integrate better with Vitest's cleanup and provide clearer intent:

vi.spyOn(policies, "getAppliedPresets").mockReturnValue([]);
vi.spyOn(policies, "getGatewayPresets").mockReturnValue(["telegram"]);

Vitest automatically restores spies after each test when configured appropriately. Additionally, the logSpy at line 106 is created but only used to suppress output—if assertions on log calls aren't needed, vi.spyOn(console, "log").mockImplementation(() => {}) can be placed in a beforeEach hook.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/repro-2010.test.js` around lines 106 - 133, Replace direct mutation of
policies.getAppliedPresets and policies.getGatewayPresets with Vitest spies: use
vi.spyOn(policies, "getAppliedPresets").mockReturnValue([]) and
vi.spyOn(policies, "getGatewayPresets").mockReturnValue(["telegram"]) so the
test integrates with Vitest's spy lifecycle instead of manually saving/restoring
origGetApplied/origGetGateway; also replace the manual logSpy creation/restore
by using vi.spyOn(console, "log").mockImplementation(() => {}) in a per-test
setup (beforeEach) or keep a local spy but use vi.spyOn to ensure automatic
cleanup.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/repro-2010.test.js`:
- Around line 106-133: Replace direct mutation of policies.getAppliedPresets and
policies.getGatewayPresets with Vitest spies: use vi.spyOn(policies,
"getAppliedPresets").mockReturnValue([]) and vi.spyOn(policies,
"getGatewayPresets").mockReturnValue(["telegram"]) so the test integrates with
Vitest's spy lifecycle instead of manually saving/restoring
origGetApplied/origGetGateway; also replace the manual logSpy creation/restore
by using vi.spyOn(console, "log").mockImplementation(() => {}) in a per-test
setup (beforeEach) or keep a local spy but use vi.spyOn to ensure automatic
cleanup.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f44cacc-c3e3-47bd-be5f-893f873dcf07

📥 Commits

Reviewing files that changed from the base of the PR and between 67db244 and 1720c42.

📒 Files selected for processing (3)

• src/lib/policies.ts
• src/nemoclaw.ts
• test/repro-2010.test.js

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/repro-2010.test.ts`:
- Around line 105-194: These tests currently only assert mocked return values
instead of exercising the real rendering logic in sandboxPolicyList; update each
test to either (A) call sandboxPolicyList() after setting the
policies.getAppliedPresets and policies.getGatewayPresets mocks and assert the
function's output contains the expected marker (●/○) and suffix text for
"telegram", or (B) extract the marker/suffix selection into a small helper
(e.g., determinePresetMarker(presetName, registryPresets, gatewayPresets)) and
replace the in-test boolean logic with direct unit tests of that helper that
assert the exact marker and suffix; reference policies.getAppliedPresets,
policies.getGatewayPresets, and sandboxPolicyList (or the new
determinePresetMarker) when locating where to change the tests.
- Around line 42-100: The tests currently reimplement parsing/key-matching
instead of exercising the exported function; update them to call
policies.getGatewayPresets() directly (stub any gateway response used by
getGatewayPresets, e.g., replace the call that fetches/parses the gateway YAML
or inject buildGatewayYaml(...) as the stubbed response) and assert its returned
array (or empty string) equals the expected presets for the given gateway;
reference policies.getGatewayPresets, buildGatewayYaml, and policies.loadPreset
when locating the test logic to replace so you remove the inline parsing/key
comparisons and instead assert the direct output of getGatewayPresets().

🪄 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: 53d2c3df-0aaa-4191-8c95-510d48cb7708

📥 Commits

Reviewing files that changed from the base of the PR and between 1720c42 and a88d15a.

📒 Files selected for processing (3)

• src/lib/policies.ts
• src/nemoclaw.ts
• test/repro-2010.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/nemoclaw.ts
• src/lib/policies.ts

[wscurran]

✨ Closes

• #2010 [NemoClaw][All Platforms]nemoclaw rebuild: policy-list shows telegram as ○ (not applied) but gateway still allows telegram traffic — policy state inconsistency

---

Possibly related open issues:

• #2010 [NemoClaw][All Platforms]nemoclaw rebuild: policy-list shows telegram as ○ (not applied) but gateway still allows telegram traffic — policy state inconsistency

[ericksoa]

LGTM. Clean approach to the registry/gateway desync problem.

• null vs [] semantic for unreachable vs empty is the right design
• All 4 display states (synced/desynced x both directions) plus unreachable fallback covered
• CLI subprocess tests exercise the real rendering logic end-to-end
• CI all green

One note: the getGatewayPresets matching-logic tests reimplement the algorithm in subprocess rather than calling the real function (due to CJS closure capture of runCapture). This is a known trade-off and acceptable — CodeRabbit flagged the same. The CLI output tests provide the real end-to-end coverage.