Skip to content

docs: fix stale architecture, commands, and network policy references - #615

Closed
gn00295120 wants to merge 2 commits into
NVIDIA:mainfrom
gn00295120:docs/fix-stale-architecture-and-references
Closed

docs: fix stale architecture, commands, and network policy references#615
gn00295120 wants to merge 2 commits into
NVIDIA:mainfrom
gn00295120:docs/fix-stale-architecture-and-references

Conversation

@gn00295120

@gn00295120 gn00295120 commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix documentation that references files deleted in #492 and policy entries that no longer exist in the baseline.

architecture.md

  • Add host CLI layerbin/nemoclaw.js and bin/lib/ were completely absent from the architecture diagram, despite being the actual CLI that users interact with
  • Replace 9 deleted TypeScript files with the files that actually exist (slash.ts, migration-state.ts, state.ts, config.ts)
  • Remove the "verify digest" lifecycle step (verify.ts was deleted in fix: remove openclaw nemoclaw CLI commands, keep provider and slash command #492)

commands.md

  • Remove openshell term section — this is an OpenShell command, not a NemoClaw command

network-policies.md

  • Remove telegram from baseline table — removed for security hardening, available as opt-in preset via nemoclaw preset apply telegram
  • Merge github and github_rest_api into one entry — the YAML has a single github block with both github.com and api.github.com
  • Fix npm_registry rules — YAML uses access: full, not GET-only

Test plan

  • All referenced file paths verified against current tree
  • Network policy table matches openclaw-sandbox.yaml
  • No broken cross-references in docs

Summary by CodeRabbit

  • Documentation
    • Revised architecture docs to describe a three-layer model with new host CLI, sandbox plugin, and blueprint responsibilities.
    • Clarified host vs. sandbox responsibilities and updated onboarding and execution flow.
    • Removed the standalone OpenShell terminal command documentation.
    • Updated network policies: added api.github.com, consolidated policy groups, removed Telegram group, and expanded npm registry method allowances.

architecture.md:
- Add host CLI layer (bin/nemoclaw.js + bin/lib/) — was completely missing
- Replace 9 deleted TypeScript files (cli.ts, launch.ts, connect.ts,
  status.ts, logs.ts, resolve.ts, fetch.ts, verify.ts, exec.ts) with
  the actual files that exist after PR NVIDIA#492
- Update plugin description to reflect current scope (slash command +
  inference provider only)
- Remove "verify digest" step from lifecycle (verify.ts was deleted)

commands.md:
- Remove openshell term section — this is an OpenShell command, not
  a NemoClaw command

network-policies.md:
- Remove telegram from baseline policy table (removed for security
  hardening — available as opt-in preset)
- Merge github and github_rest_api into one entry matching the actual
  YAML (single github block with both endpoints)
- Fix npm_registry rules to match YAML (access: full, not GET only)
Copilot AI review requested due to automatic review settings March 22, 2026 00:38
@coderabbitai

coderabbitai Bot commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 88745f12-0871-4fea-9f79-f8efd056f054

📥 Commits

Reviewing files that changed from the base of the PR and between 33393f4 and c470308.

📒 Files selected for processing (2)
  • docs/reference/architecture.md
  • docs/reference/commands.md
💤 Files with no reviewable changes (1)
  • docs/reference/commands.md
✅ Files skipped from review due to trivial changes (1)
  • docs/reference/architecture.md

📝 Walkthrough

Walkthrough

Architecture docs changed to a three-layer NemoClaw model (Host CLI, sandbox TypeScript plugin, Python blueprint); plugin and blueprint responsibilities reordered (resolve moved to host). Command docs removed openshell term. Network policy doc updated: api.github.com:443 added, github_rest_api and telegram groups removed, npm_registry now allows all methods.

Changes

Cohort / File(s) Summary
Architecture Restructuring
docs/reference/architecture.md
Converted NemoClaw from a two-component model to a three-layer model with explicit bin/ + lib/ host CLI layout; moved resolve responsibility to Host CLI; revised sandbox plugin tree (removed prior command/blueprint modules, added migration-state and onboarding config); updated blueprint execution flow and lifecycle steps.
Command Reference Cleanup
docs/reference/commands.md
Removed the openshell term standalone command section and associated usage/SSH guidance.
Network Policy Updates
docs/reference/network-policies.md
Added api.github.com:443 to allowed endpoints, removed the github_rest_api and telegram policy groups, and changed npm_registry from GET-only to allow all HTTP methods.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant HostCLI as Host CLI
    participant Sandbox as Sandbox Plugin
    participant Blueprint as Python Blueprint

    User->>HostCLI: onboarding request / run command
    HostCLI->>Sandbox: provision sandbox, deploy plugin
    HostCLI->>HostCLI: select inference, run preflight & health checks
    HostCLI->>Blueprint: invoke blueprint runner (subprocess) for onboarding
    Blueprint-->>HostCLI: onboarding result
    HostCLI->>Sandbox: start sandbox process / container lifecycle
    User->>HostCLI: run runner-led sequence (plan/apply/status)
    HostCLI->>Blueprint: execute plan/apply/status as subprocesses
    Blueprint-->>HostCLI: execution status/results
    HostCLI->>Sandbox: manage sandbox registry & policies
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through docs with bright delight,
Three layers now in morning light,
Commands trimmed and policies tuned,
A tidy map the team attuned,
Nibble, hop—documentation done!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main objective: fixing stale references in three documentation files (architecture, commands, and network policies) to reflect current codebase state.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 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.

🧹 Nitpick comments (1)
docs/reference/architecture.md (1)

28-28: Use active voice for the install instruction.

Line 28 uses passive voice (“It is installed…”). Prefer direct active phrasing (for example, addressing the reader directly).

As per coding guidelines, "Active voice required. Flag passive constructions."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/reference/architecture.md` at line 28, Change the passive sentence "It
is installed via `npm install -g nemoclaw`." to an active, reader-directed
instruction; for example, replace it with "Install it with `npm install -g
nemoclaw`." or "Run `npm install -g nemoclaw` to install the CLI." Ensure the
phrasing uses active voice and addresses the reader.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/reference/architecture.md`:
- Line 28: Change the passive sentence "It is installed via `npm install -g
nemoclaw`." to an active, reader-directed instruction; for example, replace it
with "Install it with `npm install -g nemoclaw`." or "Run `npm install -g
nemoclaw` to install the CLI." Ensure the phrasing uses active voice and
addresses the reader.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ba67944-5d57-447c-ba28-d7bdc448b20e

📥 Commits

Reviewing files that changed from the base of the PR and between 1dbf82f and 33393f4.

📒 Files selected for processing (3)
  • docs/reference/architecture.md
  • docs/reference/commands.md
  • docs/reference/network-policies.md
💤 Files with no reviewable changes (1)
  • docs/reference/commands.md

Copilot AI 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.

Pull request overview

This PR updates the documentation to reflect the post-#492 NemoClaw structure by refreshing the architecture reference, removing non-NemoClaw CLI content from the commands reference, and adjusting the baseline network policy table.

Changes:

  • Update architecture.md to include the host nemoclaw CLI layer and revise plugin/blueprint lifecycle descriptions.
  • Remove the openshell term section from commands.md (not a NemoClaw command).
  • Update network-policies.md baseline entries (GitHub consolidation, npm registry rules, remove Telegram from table).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
docs/reference/architecture.md Adds host CLI layer and revises plugin/blueprint diagrams and lifecycle text.
docs/reference/commands.md Removes an OpenShell-only command section from the NemoClaw command reference.
docs/reference/network-policies.md Updates the baseline endpoint-group table to reflect intended policy changes.
Comments suppressed due to low confidence (1)

docs/reference/architecture.md:74

  • The blueprint lifecycle section claims the host CLI resolves a blueprint artifact and enforces min_openshell_version / min_openclaw_version constraints from blueprint.yaml, and that onboarding invokes the blueprint runner as a subprocess. In the current implementation, the host CLI onboarding flow (bin/lib/onboard.js) directly drives openshell commands and doesn’t reference blueprint.yaml or those min-version fields. Please adjust this section to match the actual orchestration path (or avoid attributing these responsibilities to the host CLI if they’re not implemented).
The blueprint is a versioned Python artifact with its own release stream.
The host CLI invokes the blueprint runner as a subprocess during onboarding.
The blueprint drives all interactions with the OpenShell CLI.

```text
nemoclaw-blueprint/

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/reference/network-policies.md
3. Plan. The runner determines what OpenShell resources to create or update, such as the gateway, providers, sandbox, inference route, and policy.
4. Apply. The runner executes the plan by calling `openshell` CLI commands.
5. Status. The runner reports current state.
1. Resolve. The host CLI locates the blueprint artifact and checks the version against `min_openshell_version` and `min_openclaw_version` constraints in `blueprint.yaml`.

Copilot AI Mar 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step says the host CLI checks min_openshell_version / min_openclaw_version constraints in blueprint.yaml, but the current host CLI codepath (bin/lib/onboard.js) does not read blueprint.yaml or perform any version-constraint validation. Either implement these checks in the host CLI, or update the documentation to reflect what is actually validated today (e.g., the preflight checks around openshell -V).

Suggested change
1. Resolve. The host CLI locates the blueprint artifact and checks the version against `min_openshell_version` and `min_openclaw_version` constraints in `blueprint.yaml`.
1. Resolve. The host CLI locates the blueprint artifact and runs preflight checks (for example, invoking `openshell -V` to validate the local CLI), but it does not currently enforce version constraints from `blueprint.yaml`.

Copilot uses AI. Check for mistakes.
Comment thread docs/reference/network-policies.md
@wscurran

Copy link
Copy Markdown
Contributor

Thanks for submitting this proposed fix to update the documentation and remove references to deleted files and policy entries, which may help improve the overall clarity and accuracy of the documentation.

@wscurran

Copy link
Copy Markdown
Contributor

Thanks for the docs cleanup. The architecture documentation, command reference, and network policy docs have all evolved since March. Could you rebase against main and verify the corrections are still accurate and don't conflict with current content? Happy to review once it's updated.

@prekshivyas

Copy link
Copy Markdown
Collaborator

Thank you for the thorough effort here @gn00295120! Closing because this PR is deeply stale relative to current main:

The plugin file tree observation was correct though — if you'd like to submit a fresh, focused PR to update just the plugin section of architecture.md, that would be welcome!

@wscurran wscurran added needs: rebase PR needs rebase or conflict resolution area: docs Documentation, examples, guides, or docs build chore Build, CI, dependency, or tooling maintenance and removed status: rebase labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build chore Build, CI, dependency, or tooling maintenance needs: rebase PR needs rebase or conflict resolution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants