Skip to content

docs(policy): clarify network request approval flow - #5886

Merged
cv merged 4 commits into
mainfrom
docs/fix-5082-approve-network-requests
Jun 27, 2026
Merged

docs(policy): clarify network request approval flow#5886
cv merged 4 commits into
mainfrom
docs/fix-5082-approve-network-requests

Conversation

@miyoungc

@miyoungc miyoungc commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Clarifies how operators find and act on blocked network requests in the OpenShell TUI.
This updates the how-to with the sandbox selection flow, Network Rules keybindings, remote-host expectations, and source-checkout prerequisites for the walkthrough.

Related Issue

Fixes #5082

Changes

  • Documented selecting a sandbox from the TUI dashboard and opening the Network Rules panel.
  • Added the a, x, and A approval and rejection keybindings, including the approve-all confirmation prompt.
  • Clarified that the walkthrough script requires a NemoClaw source checkout, tmux, NVIDIA_INFERENCE_API_KEY, and an onboarded sandbox.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification: doc-only how-to update with no runtime behavior changes.
  • 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: docs-only network-policy guidance; no runtime policy, credential, sandbox, or security enforcement code changed.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Verification notes:

  • npm run docs passed with 0 errors; Fern reported 1 hidden warning plus an upgrade notice, so the "without warnings" checkbox is intentionally unchecked.
  • The OpenShell TUI keybindings were checked against crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs in the sibling OpenShell checkout.

Signed-off-by: Miyoung Choi miyoungc@nvidia.com

Summary by CodeRabbit

  • Documentation
    • Clarified prerequisites for accessing the OpenShell TUI on the host running the NemoClaw sandbox, including explicit access requirements.
    • Improved the “Open the TUI” remote workflow with SSH host/alias placeholders and added guidance for locating the active sandbox in the dashboard (select with j/k, open with Enter).
    • Rewrote the approval walkthrough to manage pending entries in the Network Rules panel (focus r, inspect Enter, approve/reject selected, and bulk-approve with confirmation).
    • Updated walkthrough script setup to clone on the sandbox host (after SSH if needed) and run ./scripts/walkthrough.sh.

@miyoungc miyoungc added area: docs Documentation, examples, guides, or docs build area: policy Network policy, egress rules, presets, or sandbox policy labels Jun 26, 2026
@miyoungc miyoungc self-assigned this Jun 26, 2026
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The guide now explains sandbox host access, TUI navigation to a running sandbox, Network Rules keybindings for approving or rejecting requests, and expanded walkthrough setup requirements.

Changes

Network policy approval docs

Layer / File(s) Summary
TUI access and sandbox selection
docs/network-policy/approve-network-requests.mdx
Adds a prerequisite for host access, clarifies the SSH host substitution, and adds a dashboard flow to open a running sandbox in the TUI.
Rule actions and walkthrough setup
docs/network-policy/approve-network-requests.mdx
Documents Network Rules keybindings for inspecting, approving, rejecting, and bulk-approving pending rules, and expands walkthrough setup with source checkout and environment requirements.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5828: This earlier PR also updates docs/network-policy/approve-network-requests.mdx and relates to the TUI access and navigation guidance in the same doc.

Poem

A bunny hopped through terminal glow,
To sandbox lands with keys in tow.
j, k, Enter, then rules to choose,
a, x, A for approve or refuse.
🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the doc update about network request approval flow.
Linked Issues check ✅ Passed The documentation updates address the linked issue’s requested placeholder host, walkthrough prerequisites, sandbox navigation, and approve/deny key details.
Out of Scope Changes check ✅ Passed The changes stay within the requested documentation scope and do not introduce unrelated code or content changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/fix-5082-approve-network-requests

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. Docs-only change that cannot affect installer/onboarding, sandbox lifecycle, credentials, security boundaries, network policy enforcement, inference routing, deployment, or real assistant user flows at runtime.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • None. Docs-only change outside test/e2e-scenario/ and the Vitest scenario workflow; it cannot affect Vitest E2E scenario behavior.

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • None.

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: TUI keybinding documentation for security-critical approve/deny actions.
Open items: 0 required · 4 warnings · 1 suggestion · 2 test follow-ups
Since last review: 0 prior items resolved · 1 still applies · 2 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: TUI keybinding documentation for security-critical approve/deny actions
  • PRA-2 Resolve or justify: Source-of-truth review needed: MDX <Steps>/<Step> component availability
  • PRA-3 Resolve or justify: TUI keybindings documented but not verifiable in this repo (PRA-1 exacerbated) in docs/network-policy/approve-network-requests.mdx:58
  • PRA-4 Resolve or justify: MDX <Steps> and <Step> components not verified in codebase in docs/network-policy/approve-network-requests.mdx:17
  • PRA-T1 Add or justify test follow-up: TUI keybinding documentation for security-critical approve/deny actions
  • PRA-T2 Add or justify test follow-up: MDX <Steps>/<Step> component availability
  • PRA-5 In-scope improvement: Walkthrough requires repo clone even for installer users in docs/network-policy/approve-network-requests.mdx:92

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Resolve/justify docs docs/network-policy/approve-network-requests.mdx:58 Add a doc-review checklist item to verify keybindings against OpenShell releases; add a cross-repo link to OpenShell TUI source (crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs) or a version pin note in document frontmatter.
PRA-4 Resolve/justify docs docs/network-policy/approve-network-requests.mdx:17 Verify `<Steps>` and `<Step>` are supported Fern built-in components. If custom, add component definitions to docs/_components and register in fern/docs.yml. If Fern built-ins, add a comment referencing Fern documentation.
PRA-5 Improvement docs docs/network-policy/approve-network-requests.mdx:92 Consider distributing walkthrough.sh via the installer or providing a standalone download link. At minimum, the current explanation is adequate — no change required if installer distribution is out of scope.
Review findings by urgency: 0 required fixes, 4 items to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: TUI keybinding documentation for security-critical approve/deny actions

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Periodic docs freshness check job validating keybindings against OpenShell release tags
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: PR author verified against sibling OpenShell checkout manually; no automated linkage in NemoClaw repo

PRA-2 Resolve/justify — Source-of-truth review needed: MDX <Steps>/<Step> component availability

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Docs build validation in CI
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: fern/docs.yml mdx-components paths don't include Steps/Step components; not in docs/_components or fern/components

PRA-3 Resolve/justify — TUI keybindings documented but not verifiable in this repo (PRA-1 exacerbated)

  • Location: docs/network-policy/approve-network-requests.mdx:58
  • Category: docs
  • Problem: TUI keybindings (j/k, Enter, r, a, x, A, y/Enter) for network rule approval map to OpenShell TUI implementation in a separate repository. Previous review (PRA-1) flagged this; this PR adds 5 new keybindings (r, a, x, A, y/Enter) without adding any verification mechanism.
  • Impact: If OpenShell TUI keybindings change, NemoClaw docs become stale without detection, causing operators to press wrong keys during security-critical network egress approval/deny decisions.
  • Recommended action: Add a doc-review checklist item to verify keybindings against OpenShell releases; add a cross-repo link to OpenShell TUI source (crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs) or a version pin note in document frontmatter.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check OpenShell release notes/changelog for TUI keybinding changes; verify against current OpenShell source at crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs
  • Missing regression test: N/A — external dependency; consider a periodic docs freshness check job that validates keybindings against OpenShell release tags
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check OpenShell release notes/changelog for TUI keybinding changes; verify against current OpenShell source at crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs.
  • Evidence: PR verification notes state keybindings checked against OpenShell source in sibling checkout; no automated linkage exists in NemoClaw repo. Current file lines 58-63 document keybindings in detail but lack cross-repo verification reference.

PRA-4 Resolve/justify — MDX <Steps> and <Step> components not verified in codebase

  • Location: docs/network-policy/approve-network-requests.mdx:17
  • Category: docs
  • Problem: Uses `<Steps toc={true}>` and nested `<Steps>`/`<Step>` MDX components that are not defined in the project's component directories (docs/_components, fern/components). No import or registration visible in fern/docs.yml mdx-components paths.
  • Impact: If these are not Fern built-in components, the documentation build will fail or render incorrectly. If they are Fern built-ins, this should be documented for maintainability.
  • Recommended action: Verify `<Steps>` and `<Step>` are supported Fern built-in components. If custom, add component definitions to docs/_components and register in fern/docs.yml. If Fern built-ins, add a comment referencing Fern documentation.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run the docs build locally (fern build --docs) and verify the page renders correctly with table of contents and step navigation
  • Missing regression test: Add a docs build check in CI that validates this page renders without errors
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the docs build locally (fern build --docs) and verify the page renders correctly with table of contents and step navigation.
  • Evidence: File uses <Steps toc={true}> at line 17 and nested <Steps>/<Step> at lines 92-113. fern/docs.yml mdx-components paths point to ../docs/_components and ./components — neither contains Steps.tsx or Step.tsx

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-5 Improvement — Walkthrough requires repo clone even for installer users

  • Location: docs/network-policy/approve-network-requests.mdx:92
  • Category: docs
  • Problem: Walkthrough section now requires cloning the NemoClaw repo with explanation 'because the walkthrough script is a source-checkout helper.' This is accurate per walkthrough.sh but creates friction for users who installed via public installer.
  • Impact: Users who installed via public installer must still clone the repo to run the walkthrough, which may be unexpected. The explanation is clear but the workflow could be simplified.
  • Suggested action: Consider distributing walkthrough.sh via the installer or providing a standalone download link. At minimum, the current explanation is adequate — no change required if installer distribution is out of scope.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run ./scripts/walkthrough.sh from a fresh clone and verify it works; confirm installer users cannot run it without cloning
  • Missing regression test: N/A — workflow documentation
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: walkthrough.sh requires tmux, NVIDIA_INFERENCE_API_KEY, and onboarded sandbox. Docs at lines 92-113 now document git clone steps explicitly.
Simplification opportunities: 1 possible cut, net -15 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-4 native (docs/network-policy/approve-network-requests.mdx:17): <Steps toc={true}> and nested <Steps>/<Step> custom MDX components
    • Replacement: Verify Fern native step component support; if unavailable, use plain markdown with heading hierarchy
    • Net: -15 lines
    • Safety boundary: Documentation rendering must remain functional with TOC and step navigation
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 TUI keybinding documentation for security-critical approve/deny actions — Periodic docs freshness check job validating keybindings against OpenShell release tags. PR author verified against sibling OpenShell checkout manually; no automated linkage in NemoClaw repo
  • PRA-T2 MDX <Steps>/<Step> component availability — Docs build validation in CI. fern/docs.yml mdx-components paths don't include Steps/Step components; not in docs/_components or fern/components
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: TUI keybinding documentation for security-critical approve/deny actions

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Periodic docs freshness check job validating keybindings against OpenShell release tags
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: PR author verified against sibling OpenShell checkout manually; no automated linkage in NemoClaw repo

PRA-2 Resolve/justify — Source-of-truth review needed: MDX <Steps>/<Step> component availability

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Docs build validation in CI
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: fern/docs.yml mdx-components paths don't include Steps/Step components; not in docs/_components or fern/components

PRA-3 Resolve/justify — TUI keybindings documented but not verifiable in this repo (PRA-1 exacerbated)

  • Location: docs/network-policy/approve-network-requests.mdx:58
  • Category: docs
  • Problem: TUI keybindings (j/k, Enter, r, a, x, A, y/Enter) for network rule approval map to OpenShell TUI implementation in a separate repository. Previous review (PRA-1) flagged this; this PR adds 5 new keybindings (r, a, x, A, y/Enter) without adding any verification mechanism.
  • Impact: If OpenShell TUI keybindings change, NemoClaw docs become stale without detection, causing operators to press wrong keys during security-critical network egress approval/deny decisions.
  • Recommended action: Add a doc-review checklist item to verify keybindings against OpenShell releases; add a cross-repo link to OpenShell TUI source (crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs) or a version pin note in document frontmatter.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check OpenShell release notes/changelog for TUI keybinding changes; verify against current OpenShell source at crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs
  • Missing regression test: N/A — external dependency; consider a periodic docs freshness check job that validates keybindings against OpenShell release tags
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check OpenShell release notes/changelog for TUI keybinding changes; verify against current OpenShell source at crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs.
  • Evidence: PR verification notes state keybindings checked against OpenShell source in sibling checkout; no automated linkage exists in NemoClaw repo. Current file lines 58-63 document keybindings in detail but lack cross-repo verification reference.

PRA-4 Resolve/justify — MDX <Steps> and <Step> components not verified in codebase

  • Location: docs/network-policy/approve-network-requests.mdx:17
  • Category: docs
  • Problem: Uses `<Steps toc={true}>` and nested `<Steps>`/`<Step>` MDX components that are not defined in the project's component directories (docs/_components, fern/components). No import or registration visible in fern/docs.yml mdx-components paths.
  • Impact: If these are not Fern built-in components, the documentation build will fail or render incorrectly. If they are Fern built-ins, this should be documented for maintainability.
  • Recommended action: Verify `<Steps>` and `<Step>` are supported Fern built-in components. If custom, add component definitions to docs/_components and register in fern/docs.yml. If Fern built-ins, add a comment referencing Fern documentation.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run the docs build locally (fern build --docs) and verify the page renders correctly with table of contents and step navigation
  • Missing regression test: Add a docs build check in CI that validates this page renders without errors
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the docs build locally (fern build --docs) and verify the page renders correctly with table of contents and step navigation.
  • Evidence: File uses <Steps toc={true}> at line 17 and nested <Steps>/<Step> at lines 92-113. fern/docs.yml mdx-components paths point to ../docs/_components and ./components — neither contains Steps.tsx or Step.tsx

PRA-5 Improvement — Walkthrough requires repo clone even for installer users

  • Location: docs/network-policy/approve-network-requests.mdx:92
  • Category: docs
  • Problem: Walkthrough section now requires cloning the NemoClaw repo with explanation 'because the walkthrough script is a source-checkout helper.' This is accurate per walkthrough.sh but creates friction for users who installed via public installer.
  • Impact: Users who installed via public installer must still clone the repo to run the walkthrough, which may be unexpected. The explanation is clear but the workflow could be simplified.
  • Suggested action: Consider distributing walkthrough.sh via the installer or providing a standalone download link. At minimum, the current explanation is adequate — no change required if installer distribution is out of scope.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run ./scripts/walkthrough.sh from a fresh clone and verify it works; confirm installer users cannot run it without cloning
  • Missing regression test: N/A — workflow documentation
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: walkthrough.sh requires tmux, NVIDIA_INFERENCE_API_KEY, and onboarded sandbox. Docs at lines 92-113 now document git clone steps explicitly.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 3 prior items resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@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.

Actionable comments posted: 3

🧹 Nitpick comments (1)
docs/network-policy/approve-network-requests.mdx (1)

94-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Format tmux as inline code.

As per coding guidelines, "Use code formatting for commands, paths, flags, environment variables, file names, and literal values in documentation," and as per path instructions, "CLI commands, file paths, flags, parameter names, and values must use inline code formatting."

🤖 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 `@docs/network-policy/approve-network-requests.mdx` at line 94, The walkthrough
text is missing inline code formatting for the tmux command. Update the
documentation sentence in the approve-network-requests walkthrough so tmux is
wrapped in inline code, matching the existing formatting used for
NVIDIA_INFERENCE_API_KEY and other literal values in the docs.

Sources: Coding guidelines, 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.

Inline comments:
In `@docs/network-policy/approve-network-requests.mdx`:
- Around line 64-65: The navigation text mixes two different view names, so the
flow is unclear. Update the wording in the approval instructions to use one
exact TUI label consistently, or add the missing step that moves from the
sandbox view to the sandbox policy view before mentioning the `r` shortcut. Keep
the guidance aligned with the `Network Rules` panel and the
approve-network-requests flow.
- Around line 72-75: Update the approval-lifetime wording in the network policy
docs to match the sandbox-instance behavior described in the
enterprise-readiness docs. In the section that mentions approved endpoints and
rejected rules, change the phrasing so it reflects that approvals persist for
the sandbox instance and reset only when the sandbox is destroyed and recreated,
rather than saying they last only until the sandbox stops. Keep the surrounding
guidance about baseline policy files and policy YAML/presets intact.
- Around line 82-85: The checkout path example is inconsistent across the
network-policy docs, which can confuse readers on case-sensitive systems. Update
the `approve-network-requests.mdx` snippet to use the same repository directory
name as the earlier remote-host flow, or explicitly mark the path in the
clone-and-cd example as illustrative only. Keep the naming aligned in the
related checkout instructions so `git clone` and `cd` reference the same
directory consistently.

---

Nitpick comments:
In `@docs/network-policy/approve-network-requests.mdx`:
- Line 94: The walkthrough text is missing inline code formatting for the tmux
command. Update the documentation sentence in the approve-network-requests
walkthrough so tmux is wrapped in inline code, matching the existing formatting
used for NVIDIA_INFERENCE_API_KEY and other literal values in the docs.
🪄 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: 0910efe8-3a10-42e7-b510-761b1bfd5de1

📥 Commits

Reviewing files that changed from the base of the PR and between c5902a8 and 8f42951.

📒 Files selected for processing (1)
  • docs/network-policy/approve-network-requests.mdx

Comment thread docs/network-policy/approve-network-requests.mdx Outdated
Comment thread docs/network-policy/approve-network-requests.mdx Outdated
Comment thread docs/network-policy/approve-network-requests.mdx Outdated
@miyoungc
miyoungc marked this pull request as draft June 26, 2026 22:49
@copy-pr-bot

copy-pr-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@miyoungc

Copy link
Copy Markdown
Collaborator Author

Follow-up on the remaining non-blocking Nemotron Ultra suggestion (PRA-1): I considered adding a broader cross-repo keybinding verification hook or source link, but kept this PR scoped to the user-facing how-to fix for #5082.

The TUI keybindings documented here were manually verified against the current OpenShell TUI implementation in crates/openshell-tui/src/app.rs and crates/openshell-tui/src/ui/sandbox_draft.rs; that evidence is captured in the PR verification notes.

@miyoungc
miyoungc marked this pull request as ready for review June 27, 2026 00:00
@miyoungc miyoungc removed their assignment Jun 27, 2026

@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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/network-policy/approve-network-requests.mdx (1)

77-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use sandbox instance consistently for approval lifetime.

Line 77 says the rule is added for the “current session”, but Lines 81-82 say it persists for the sandbox instance until destroy/recreate.
That can mislead readers into thinking a TUI reconnect drops the approval.

🤖 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 `@docs/network-policy/approve-network-requests.mdx` around lines 77 - 82, The
approval lifetime wording is inconsistent in the network request guide: the
`approve/reject` keybinding text in the pending-rule section says approvals
apply to the “current session,” while the later note says they persist for the
sandbox instance until destroy/recreate. Update the wording around the
pending-rule actions and the “Approved endpoints remain…” note so it
consistently uses the sandbox instance as the approval scope, and make sure the
`approve-network-requests` instructions do not imply a TUI reconnect clears
approved endpoints.
🤖 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 `@docs/network-policy/approve-network-requests.mdx`:
- Around line 106-113: The walkthrough text is too vague because
scripts/walkthrough.sh expects the sandbox alias nemoclaw, so update the
instructions in approve-network-requests.mdx to either explicitly tell readers
to onboard a sandbox named nemoclaw or clearly state they must edit the script
for a different alias. Make sure the affected Step content around the onboarding
guidance and walkthrough command matches what the script actually does.

---

Outside diff comments:
In `@docs/network-policy/approve-network-requests.mdx`:
- Around line 77-82: The approval lifetime wording is inconsistent in the
network request guide: the `approve/reject` keybinding text in the pending-rule
section says approvals apply to the “current session,” while the later note says
they persist for the sandbox instance until destroy/recreate. Update the wording
around the pending-rule actions and the “Approved endpoints remain…” note so it
consistently uses the sandbox instance as the approval scope, and make sure the
`approve-network-requests` instructions do not imply a TUI reconnect clears
approved endpoints.
🪄 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: 53469b49-405d-4171-a604-0682178f3656

📥 Commits

Reviewing files that changed from the base of the PR and between fb0469d and 7e9f544.

📒 Files selected for processing (1)
  • docs/network-policy/approve-network-requests.mdx

Comment on lines +106 to +113
Onboard at least one sandbox and confirm that it is attached to the active gateway.
</Step>
<Step>
Run the walkthrough script from the NemoClaw repository root.

```bash
./scripts/walkthrough.sh
```

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Document the required sandbox alias, or this walkthrough is not literal.

The linked scripts/walkthrough.sh connects with openshell sandbox connect nemoclaw, so “at least one onboarded sandbox” is not enough to follow these steps verbatim.
Please either tell readers to onboard a sandbox named nemoclaw or call out that they must edit the script for a different alias.

Also applies to: 117-119

🤖 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 `@docs/network-policy/approve-network-requests.mdx` around lines 106 - 113, The
walkthrough text is too vague because scripts/walkthrough.sh expects the sandbox
alias nemoclaw, so update the instructions in approve-network-requests.mdx to
either explicitly tell readers to onboard a sandbox named nemoclaw or clearly
state they must edit the script for a different alias. Make sure the affected
Step content around the onboarding guidance and walkthrough command matches what
the script actually does.

@cv
cv merged commit 6466491 into main Jun 27, 2026
40 checks passed
@cv
cv deleted the docs/fix-5082-approve-network-requests branch June 27, 2026 07:37
cv pushed a commit that referenced this pull request Jun 29, 2026
## Summary
Adds the v0.0.69 release notes to the published release-notes page so
users can see the shipped sandbox recovery, Deep Agents Code, Hermes,
inference, policy, and release-validation changes.
The section is based on the v0.0.69 announcement and links each
user-facing theme to the deeper docs pages that already cover the
behavior.

## Changes
- Added a new `v0.0.69` section to `docs/about/release-notes.mdx`.
- Linked release-note themes to lifecycle, backup, troubleshooting, Deep
Agents Code, commands, workspace, messaging, Hermes, inference,
security, monitoring, and network-policy docs.

Source summary:
- #5455 -> `docs/about/release-notes.mdx`: Summarized persistent
workspace and state cleanup during sandbox destroy.
- #5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit
status preservation for failed hosted endpoint validation.
- #5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox
rediscovery when local registry state is missing.
- #5881 -> `docs/about/release-notes.mdx`: Summarized the
`nemo-deepagents` alias command surface.
- #5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent
2026.6.19 update.
- #5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived
messaging channel support.
- #5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash
managed-vLLM defaults for DGX Station.
- #5877 -> `docs/about/release-notes.mdx`: Summarized provider switch
metadata preservation.
- #5932 -> `docs/about/release-notes.mdx`: Summarized transient
inference smoke retry behavior.
- #5934 -> `docs/about/release-notes.mdx`: Summarized constrained
inference smoke retry boundaries.
- #5681 -> `docs/about/release-notes.mdx`: Summarized Shields
config-hash sealing during auto-restore.
- #5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect
process-limit enforcement.
- #5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure
provenance warnings.
- #5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log
breadcrumbs.
- #5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative
Shields status.
- #5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip
documentation updates.
- #5886 -> `docs/about/release-notes.mdx`: Summarized network request
approval-flow documentation updates.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only release-notes
prose; no runtime behavior changed.
- [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
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

`npm run docs` passed with 0 errors and the existing Fern light-mode
accent contrast warning.
`fern check --warnings` reported the same accent-color warning.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
* Added release notes for **v0.0.69**, covering improved sandbox
lifecycle recovery (state preservation across
destroy/recreate/rebuild/recovery/validation failures), clearer Deep
Agents Code terminal/CLI behavior, and safer Hermes messaging/provider
switching with manifest-driven channels.
* Improved inference setup validation guidance, including handling of
local/compatible endpoints and redaction of sensitive validation errors.
* Refreshed release-gate documentation with clearer approval examples
and validation behavior for NVIDIA API keys vs hosted inference keys.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Clarifies how operators find and act on blocked network requests in the
OpenShell TUI.
This updates the how-to with the sandbox selection flow, `Network Rules`
keybindings, remote-host expectations, and source-checkout prerequisites
for the walkthrough.

## Related Issue
Fixes NVIDIA#5082

## Changes
- Documented selecting a sandbox from the TUI dashboard and opening the
`Network Rules` panel.
- Added the `a`, `x`, and `A` approval and rejection keybindings,
including the approve-all confirmation prompt.
- Clarified that the walkthrough script requires a NemoClaw source
checkout, tmux, `NVIDIA_INFERENCE_API_KEY`, and an onboarded sandbox.

## 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
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only how-to update with
no runtime behavior changes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: docs-only
network-policy guidance; no runtime policy, credential, sandbox, or
security enforcement code changed.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

Verification notes:
- `npm run docs` passed with 0 errors; Fern reported 1 hidden warning
plus an upgrade notice, so the "without warnings" checkbox is
intentionally unchecked.
- The OpenShell TUI keybindings were checked against
`crates/openshell-tui/src/app.rs` and
`crates/openshell-tui/src/ui/sandbox_draft.rs` in the sibling OpenShell
checkout.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
* Clarified prerequisites for accessing the OpenShell TUI on the host
running the NemoClaw sandbox, including explicit access requirements.
* Improved the “Open the TUI” remote workflow with SSH host/alias
placeholders and added guidance for locating the active sandbox in the
dashboard (select with `j/k`, open with `Enter`).
* Rewrote the approval walkthrough to manage pending entries in the
`Network Rules` panel (focus `r`, inspect `Enter`, approve/reject
selected, and bulk-approve with confirmation).
* Updated walkthrough script setup to clone on the sandbox host (after
SSH if needed) and run `./scripts/walkthrough.sh`.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Adds the v0.0.69 release notes to the published release-notes page so
users can see the shipped sandbox recovery, Deep Agents Code, Hermes,
inference, policy, and release-validation changes.
The section is based on the v0.0.69 announcement and links each
user-facing theme to the deeper docs pages that already cover the
behavior.

## Changes
- Added a new `v0.0.69` section to `docs/about/release-notes.mdx`.
- Linked release-note themes to lifecycle, backup, troubleshooting, Deep
Agents Code, commands, workspace, messaging, Hermes, inference,
security, monitoring, and network-policy docs.

Source summary:
- NVIDIA#5455 -> `docs/about/release-notes.mdx`: Summarized persistent
workspace and state cleanup during sandbox destroy.
- NVIDIA#5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit
status preservation for failed hosted endpoint validation.
- NVIDIA#5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox
rediscovery when local registry state is missing.
- NVIDIA#5881 -> `docs/about/release-notes.mdx`: Summarized the
`nemo-deepagents` alias command surface.
- NVIDIA#5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent
2026.6.19 update.
- NVIDIA#5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived
messaging channel support.
- NVIDIA#5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash
managed-vLLM defaults for DGX Station.
- NVIDIA#5877 -> `docs/about/release-notes.mdx`: Summarized provider switch
metadata preservation.
- NVIDIA#5932 -> `docs/about/release-notes.mdx`: Summarized transient
inference smoke retry behavior.
- NVIDIA#5934 -> `docs/about/release-notes.mdx`: Summarized constrained
inference smoke retry boundaries.
- NVIDIA#5681 -> `docs/about/release-notes.mdx`: Summarized Shields
config-hash sealing during auto-restore.
- NVIDIA#5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect
process-limit enforcement.
- NVIDIA#5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure
provenance warnings.
- NVIDIA#5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log
breadcrumbs.
- NVIDIA#5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative
Shields status.
- NVIDIA#5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip
documentation updates.
- NVIDIA#5886 -> `docs/about/release-notes.mdx`: Summarized network request
approval-flow documentation updates.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only release-notes
prose; no runtime behavior changed.
- [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
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

`npm run docs` passed with 0 errors and the existing Fern light-mode
accent contrast warning.
`fern check --warnings` reported the same accent-color warning.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
* Added release notes for **v0.0.69**, covering improved sandbox
lifecycle recovery (state preservation across
destroy/recreate/rebuild/recovery/validation failures), clearer Deep
Agents Code terminal/CLI behavior, and safer Hermes messaging/provider
switching with manifest-driven channels.
* Improved inference setup validation guidance, including handling of
local/compatible endpoints and redaction of sensitive validation errors.
* Refreshed release-gate documentation with clearer approval examples
and validation behavior for NVIDIA API keys vs hosted inference keys.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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 area: policy Network policy, egress rules, presets, or sandbox policy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][Docs] Approve/Deny Network Requests doc: placeholder SSH host, missing walkthrough prerequisites, and no approve/deny key details

2 participants