feat(skills): add signed catalog export#4284
Merged
Merged
Conversation
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-4284.docs.buildwithfern.com/nemoclaw |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a deterministic NemoClaw skills catalog export: a checked-in allowlist ( ChangesSkills Catalog Export System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
miyoungc
reviewed
May 27, 2026
Contributor
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
Contributor
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Contributor
PR Review AdvisorFindings: 4 needs attention, 8 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
Collaborator
|
Left a comment in the issue, and x-posting here. I'd avoid creating another dir of the skills. We already have set up NV Skills to pull from our .agents/skills/. I can't see clearly what benefits this secondary skills copy brings. Update: Discussed with @jyaunches offline -- I agree with the design, given the new design in the nv skills repo... |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/catalog-skills-signing-flow.md (1)
54-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the required
Next Stepssection with related links at the bottom.The page currently ends without a
Next Stepssection linking to related pages.As per coding guidelines, “A "Next Steps" section at the bottom links to related pages.”
🤖 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/catalog-skills-signing-flow.md` around lines 54 - 69, Add a "Next Steps" section at the bottom of the document (after the "Workflow steps added in this PR" section) that links to related pages and resources; include links to the catalog skills exporter script (scripts/export-catalog-skills.py), the `.agents/catalog-skills.yaml` allowlist guidance, the `skills/nemoclaw/` generated skills repo or diff review guidance, and the CI workflow docs (e.g., the `CI / Pull Request` and `Skills / Catalog Refresh` workflow descriptions) so readers have clear follow-up resources.
🧹 Nitpick comments (30)
docs/catalog-skills-signing-flow.md (1)
54-69: ⚡ Quick winAdd one intro sentence under each H2 before bullet lists.
At Line 54 and Line 61, both sections start immediately with list items instead of a brief introductory sentence.
As per coding guidelines, “Sections use H2 and H3, each starting with an introductory sentence.”
🤖 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/catalog-skills-signing-flow.md` around lines 54 - 69, Add a brief introductory sentence under each H2 heading "Human handoff points" and "Workflow steps added in this PR" before their bullet lists explaining the purpose of the section (e.g., "Key touchpoints for manual intervention and review." and "Summary of CI/workflow behaviors introduced by this change."). Edit the markdown headings `Human handoff points` and `Workflow steps added in this PR` to insert a one-line lead-in sentence immediately after each H2 and before the following list items so the sections follow the documented style requirement.skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md (12)
23-24: ⚡ Quick winUse active voice.
"The preferred path is to provision the VM" uses passive construction. State the recommendation directly.
-Run NemoClaw on a remote GPU instance through [Brev](https://brev.nvidia.com). -The preferred path is to provision the VM, run the standard NemoClaw installer on that host, and then run `nemoclaw onboard`. +Run NemoClaw on a remote GPU instance through [Brev](https://brev.nvidia.com). +Provision the VM, run the standard NemoClaw installer on that host, and then run `nemoclaw onboard`.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 23 - 24, Change the passive sentence "The preferred path is to provision the VM, run the standard NemoClaw installer on that host, and then run `nemoclaw onboard`" to an active-voice instruction; for example, replace it with a direct imperative like "Provision the VM, run the standard NemoClaw installer on that host, then run `nemoclaw onboard`" in SKILL.md so the guidance is stated directly and actively.
115-117: ⚡ Quick winUse active voice.
"is also evaluated at image build time" is passive. State what performs the evaluation.
-`NEMOCLAW_DISABLE_DEVICE_AUTH` is also evaluated at image build time. +The installer also evaluates `NEMOCLAW_DISABLE_DEVICE_AUTH` at image build time.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 115 - 117, Rewrite the passive sentence to active voice so it names the actor: change "`NEMOCLAW_DISABLE_DEVICE_AUTH` is also evaluated at image build time." to something like "The image build process also evaluates NEMOCLAW_DISABLE_DEVICE_AUTH." Ensure the surrounding sentence that references CHAT_UI_URL and the generated sandbox configuration (device pairing disabled when CHAT_UI_URL points at a non-loopback origin) still reads clearly with this change.
135-136: ⚡ Quick winUse active voice.
"Sandbox '' was created but did not become ready" is passive. State who created it.
-If onboard ends with `Sandbox '<name>' was created but did not become ready within 180s`, onboard deletes the partially-created sandbox first, so the next attempt with the raised budget starts from a clean state. +If onboard ends with `onboard created sandbox '<name>' but it did not become ready within 180s`, onboard deletes the partially-created sandbox first, so the next attempt with the raised budget starts from a clean state.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 135 - 136, Rewrite the passive sentence "Sandbox '<name>' was created but did not become ready within 180s" into active voice by naming the actor (the onboarding process or the onboard command) and updating the following sentence accordingly; for example, change it to "The onboarding process created sandbox '<name>' but it did not become ready within 180s, so onboarding deletes the partially-created sandbox first..." and ensure references to `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` and the `nemoclaw-user-configure-inference` skill remain unchanged.
149-150: ⚡ Quick winUse active voice.
"These values are baked into the sandbox image" is passive. State who bakes them.
-These values are baked into the sandbox image at build time. +The build process bakes these values into the sandbox image at build time.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 149 - 150, Rewrite the two passive sentences in SKILL.md to use active voice and name the actor (the build process): e.g., say that the build process bakes the values into the sandbox image at build time and that the build also forwards them into the runtime container during sandbox creation so /tmp/nemoclaw-proxy-env.sh uses the same host and port the image build used; update the sentences accordingly to replace passive constructions like "are baked" and "are also forwarded" with active ones referencing the build process.
54-56: ⚡ Quick winUse active voice.
"The sandbox created on the remote VM uses" contains a passive participle. Rewrite to make the subject act.
-The sandbox created on the remote VM uses `NEMOCLAW_SANDBOX_NAME`, or `my-assistant` when the variable is unset. +The remote VM creates a sandbox using `NEMOCLAW_SANDBOX_NAME`, or `my-assistant` when the variable is unset.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 54 - 56, Rewrite the passive sentence to active voice: change "The sandbox created on the remote VM uses `NEMOCLAW_SANDBOX_NAME`, or `my-assistant` when the variable is unset." to a direct subject-verb form that names the actor (e.g., "The deploy wrapper creates the sandbox on the remote VM and names it using `NEMOCLAW_SANDBOX_NAME`, or `my-assistant` when the variable is unset."). Keep the rest of the paragraph (naming rules and validation behavior by the deploy wrapper) intact and ensure `NEMOCLAW_SANDBOX_NAME`, `my-assistant`, and "deploy wrapper" are referenced so readers can locate related code or configuration.
28-29: ⚡ Quick winUse active voice.
"has already been onboarded" is passive. Describe the state directly.
-If your Brev instance is already up and has already been onboarded with a sandbox, start with the standard sandbox chat flow: +If your Brev instance is already up and you have already onboarded a sandbox, start with the standard sandbox chat flow:As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 28 - 29, The sentence "has already been onboarded with a sandbox" is passive; update the line in SKILL.md that currently reads "If your Brev instance is already up and has already been onboarded with a sandbox, start with the standard sandbox chat flow:" to use active voice—e.g., "If your Brev instance is already up and you have onboarded a sandbox, start with the standard sandbox chat flow:"—ensuring the passive phrase "has already been onboarded" is replaced with an active construction per docs/** active-voice guideline.
63-64: ⚡ Quick winUse active voice.
"Channel messaging is configured during onboarding" is passive. State who performs the action.
-4. Starts optional host auxiliary services (for example the cloudflared tunnel) when `cloudflared` is available. Channel messaging is configured during onboarding and runs through OpenShell-managed processes, not through `nemoclaw tunnel start`. +4. Starts optional host auxiliary services (for example the cloudflared tunnel) when `cloudflared` is available. The onboarding wizard configures channel messaging to run through OpenShell-managed processes, not through `nemoclaw tunnel start`.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 63 - 64, The sentence "Channel messaging is configured during onboarding" uses passive voice; change it to active voice and specify the actor (who configures it) — e.g., rewrite to "During onboarding, the OpenShell installer configures channel messaging" or "The administrator configures channel messaging during onboarding" so the doc line referencing "nemoclaw tunnel start" and channel messaging clearly states who performs the action.
168-170: ⚡ Quick winRemove unnecessary bold (LLM pattern detected).
Bold on "Load [references/...]" is unnecessary. These are routine reference links, not critical warnings or UI labels.
-- **Load [references/install-openclaw-plugins.md](references/install-openclaw-plugins.md)** when users ask how to install, build, or configure OpenClaw plugins under NemoClaw. Explains the difference between OpenClaw plugins and agent skills, and shows the current Dockerfile-based workflow for baking a plugin into a NemoClaw sandbox. -- **Load [references/brev-web-ui.md](references/brev-web-ui.md)** when a user wants to try NemoClaw without installing the CLI, or asks how to get started on Brev. Guides users through deploying NemoClaw with the Brev web UI. -- **Load [references/sandbox-hardening.md](references/sandbox-hardening.md)** when reviewing sandbox image security controls, auditing capability drops, or looking up the runtime resource limits. Includes the sandbox container image hardening reference, covering Docker capabilities and process limits. +- Load [references/install-openclaw-plugins.md](references/install-openclaw-plugins.md) when users ask how to install, build, or configure OpenClaw plugins under NemoClaw. Explains the difference between OpenClaw plugins and agent skills, and shows the current Dockerfile-based workflow for baking a plugin into a NemoClaw sandbox. +- Load [references/brev-web-ui.md](references/brev-web-ui.md) when a user wants to try NemoClaw without installing the CLI, or asks how to get started on Brev. Guides users through deploying NemoClaw with the Brev web UI. +- Load [references/sandbox-hardening.md](references/sandbox-hardening.md) when reviewing sandbox image security controls, auditing capability drops, or looking up the runtime resource limits. Includes the sandbox container image hardening reference, covering Docker capabilities and process limits.As per coding guidelines: Unnecessary bold on routine instructions is a common LLM-generated pattern. Bold is reserved for UI labels, parameter names, and genuine warnings.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 168 - 170, The three bullet items that currently wrap "Load [references/...]" in bold are unnecessary; edit the SKILL.md content to remove the bold markup around "Load [references/install-openclaw-plugins.md]", "Load [references/brev-web-ui.md]", and "Load [references/sandbox-hardening.md]" so they appear as plain sentence-starting links (retain the bracketed link text exactly as-is), keeping the rest of each sentence intact and preserving bold only for true UI labels, parameter names, or warnings per guidelines.
121-122: ⚡ Quick winUse active voice.
"the sandbox image is built locally and uploaded into the OpenShell gateway" is passive. State who builds and uploads.
-On a remote GPU host, the first `nemoclaw onboard` typically does the slowest work of the lifecycle: the sandbox image is built locally and uploaded into the OpenShell gateway, which can stream hundreds of MiB over the VM's link before the readiness wait even starts. +On a remote GPU host, the first `nemoclaw onboard` typically does the slowest work of the lifecycle: the installer builds the sandbox image locally and uploads it into the OpenShell gateway, which can stream hundreds of MiB over the VM's link before the readiness wait even starts.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 121 - 122, The sentence describing the onboarding work uses passive voice; change it to active voice by naming the actor and action—for example, rephrase "the sandbox image is built locally and uploaded into the OpenShell gateway" to "Nemoclaw builds the sandbox image locally and uploads it to the OpenShell gateway" (keep the rest of the paragraph and the NEMOCLAW_SANDBOX_READY_TIMEOUT reference intact) so the docs/SKILL.md line uses active voice per the docs/** guideline.
95-99: ⚡ Quick winOne sentence per line.
Lines 95-96 contain multiple sentences on the same source line. This makes diffs harder to read.
-The NemoClaw dashboard validates the browser origin against an allowlist baked -into the sandbox image at build time. By default the allowlist only contains -`http://127.0.0.1:18789`. When accessing the dashboard from a remote browser +The NemoClaw dashboard validates the browser origin against an allowlist baked +into the sandbox image at build time. +By default the allowlist only contains `http://127.0.0.1:18789`. +When accessing the dashboard from a remote browserAs per coding guidelines: One sentence per line in source (makes diffs readable). Flag paragraphs where multiple sentences appear on the same line.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 95 - 99, The paragraph describing the NemoClaw dashboard origin validation in SKILL.md has multiple sentences on the same source line (lines referencing the note about the default allowlist and CHAT_UI_URL), so split that block into one sentence per line: break the sentence that mentions the default allowlist (`http://127.0.0.1:18789`) into its own line, the sentence about remote access (Brev public URL or SSH port-forward) into its own line, and the instruction to set `CHAT_UI_URL` before running setup into its own line, ensuring each sentence is a separate source line for clearer diffs.
111-113: ⚡ Quick winOne sentence per line.
Lines 111-113 contain multiple sentences on the same source line.
-On Brev, set `CHAT_UI_URL` in the launchable environment configuration so it is -available when the installer builds the sandbox image. If `CHAT_UI_URL` is not -set on a headless host, the compatibility wrapper prints a warning. +On Brev, set `CHAT_UI_URL` in the launchable environment configuration so it is +available when the installer builds the sandbox image. +If `CHAT_UI_URL` is not set on a headless host, the compatibility wrapper prints a warning.As per coding guidelines: One sentence per line in source (makes diffs readable).
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 111 - 113, The paragraph containing "On Brev, set `CHAT_UI_URL` in the launchable environment configuration so it is available when the installer builds the sandbox image. If `CHAT_UI_URL` is not set on a headless host, the compatibility wrapper prints a warning." should be split so each sentence is on its own line; edit SKILL.md to place the sentence that starts with "On Brev, set `CHAT_UI_URL`..." on one line, the sentence about availability during the installer build on the next line, and the sentence beginning "If `CHAT_UI_URL` is not set on a headless host..." on its own line, preserving the exact wording and punctuation.
60-61: ⚡ Quick winUse active voice.
"if a GPU is present" is passive. Describe the condition directly.
-1. Installs Docker and the NVIDIA Container Toolkit if a GPU is present. +1. Installs Docker and the NVIDIA Container Toolkit if the host has a GPU.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.md` around lines 60 - 61, Update the first bullet in SKILL.md ("Installs Docker and the NVIDIA Container Toolkit if a GPU is present.") to use active voice by making the subject explicit, e.g., change it to something like "Installs Docker and the NVIDIA Container Toolkit when the host has a GPU" or "Installs Docker and the NVIDIA Container Toolkit on hosts with a GPU"; keep the second bullet ("Installs the OpenShell CLI.") unchanged.skills/nemoclaw/nemoclaw-user-deploy-remote/references/sandbox-hardening.md (5)
102-102: ⚡ Quick winUse active voice.
"Runtime proxy configuration is sourced from" is passive. State what sources it.
-Runtime proxy configuration is sourced from system-wide shell hooks that read `/tmp/nemoclaw-proxy-env.sh`. +System-wide shell hooks source runtime proxy configuration by reading `/tmp/nemoclaw-proxy-env.sh`.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/sandbox-hardening.md` at line 102, The sentence "Runtime proxy configuration is sourced from system-wide shell hooks that read `/tmp/nemoclaw-proxy-env.sh`" uses passive voice; change it to active voice by stating the subject explicitly, e.g., "System-wide shell hooks that read `/tmp/nemoclaw-proxy-env.sh` provide the runtime proxy configuration." Update the line in sandbox-hardening.md to that active-voice phrasing.
107-108: ⚡ Quick winUse active voice.
"Landlock enforcement is silently skipped" is passive. State what skips it.
-The NemoClaw sandbox policy uses `compatibility: best_effort`, which means Landlock enforcement is silently skipped on kernels that do not support it. +The NemoClaw sandbox policy uses `compatibility: best_effort`, which silently skips Landlock enforcement on kernels that do not support it.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/sandbox-hardening.md` around lines 107 - 108, The sentence uses passive voice ("Landlock enforcement is silently skipped"); update the sentence to active voice by making the kernel the subject: rewrite the line referencing `compatibility: best_effort` so it says that kernels that do not support Landlock skip enforcement (e.g. "Kernels that do not support Landlock skip enforcement when `compatibility: best_effort` is set"). Ensure the new wording replaces the phrase "Landlock enforcement is silently skipped" and keeps the mention of `compatibility: best_effort`.
74-76: ⚡ Quick winUse active voice.
"That is a runtime concern controlled by the container orchestrator" is passive. State what controls it.
-> **Note:** The `Dockerfile` itself cannot enforce `--cap-drop`. That is a -> runtime concern controlled by the container orchestrator. Always configure +> **Note:** The `Dockerfile` itself cannot enforce `--cap-drop`. +> The container orchestrator controls that at runtime. +> Always configureAs per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/sandbox-hardening.md` around lines 74 - 76, The sentence uses passive voice ("That is a runtime concern controlled by the container orchestrator"); change it to active voice by naming the actor: rewrite the line referencing `Dockerfile` so it reads something like "The Dockerfile cannot enforce `--cap-drop`; the container orchestrator controls this at runtime, so configure capability dropping in your `docker run` flags, Compose file, or Kubernetes manifests." Update the sentence in sandbox-hardening.md near the `Dockerfile` note to use this active-voice phrasing.
10-12: ⚡ Quick winUse active voice.
"are explicitly purged" and "are not needed" are passive. State what purges them.
-Build toolchains (`gcc`, `g++`, `make`) and network probes (`netcat`) are -explicitly purged from the runtime image. These tools are not needed at runtime -and would unnecessarily widen the attack surface. +The runtime image build explicitly purges build toolchains (`gcc`, `g++`, `make`) and network probes (`netcat`). +Runtime does not need these tools, and they would unnecessarily widen the attack surface.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/sandbox-hardening.md` around lines 10 - 12, Rewrite the two passive sentences in sandbox-hardening.md to active voice: change "Build toolchains (`gcc`, `g++`, `make`) and network probes (`netcat`) are explicitly purged from the runtime image." to something like "The runtime image explicitly purges build toolchains (`gcc`, `g++`, `make`) and network probes (`netcat`)." and change "These tools are not needed at runtime and would unnecessarily widen the attack surface." to "These tools do not run in production and would unnecessarily widen the attack surface, so we remove them from the runtime image." Ensure the revised wording appears in docs/sandbox-hardening.md where those phrases occur.
74-76: ⚡ Quick winOne sentence per line.
The note blockquote contains multiple sentences on the same source line.
-> **Note:** The `Dockerfile` itself cannot enforce `--cap-drop`. That is a -> runtime concern controlled by the container orchestrator. Always configure +> **Note:** The `Dockerfile` itself cannot enforce `--cap-drop`. +> That is a runtime concern controlled by the container orchestrator. +> Always configureAs per coding guidelines: One sentence per line in source (makes diffs readable).
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/sandbox-hardening.md` around lines 74 - 76, The blockquote starting with "**Note:** The `Dockerfile` itself cannot enforce `--cap-drop`." contains multiple sentences on one source line; split that blockquote so each sentence is on its own line (e.g., break after "runtime concern controlled by the container orchestrator." and after "Always configure capability dropping in your `docker run` flags, Compose file, or Kubernetes") to satisfy the "one sentence per line" guideline and make diffs readable.skills/nemoclaw/nemoclaw-user-deploy-remote/references/brev-web-ui.md (4)
32-32: ⚡ Quick winOne sentence per line.
Line 32 contains multiple sentences on the same source line.
-If you already have an NVIDIA API key skip this section. Otherwise, follow these steps to generate a new key: +If you already have an NVIDIA API key skip this section. +Otherwise, follow these steps to generate a new key:As per coding guidelines: One sentence per line in source (makes diffs readable).
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/brev-web-ui.md` at line 32, Split the combined-sentence line into separate lines so each sentence is on its own source line: replace the single line "If you already have an NVIDIA API key skip this section. Otherwise, follow these steps to generate a new key:" with two lines—one containing "If you already have an NVIDIA API key skip this section." and the next containing "Otherwise, follow these steps to generate a new key:"—ensuring the file's one-sentence-per-line guideline is followed.
101-102: ⚡ Quick winUse active voice.
"the warning is resolved and the connection is established" is passive.
-Refresh the dashboard to see if the warning is resolved and the connection is established. +Refresh the dashboard to see if the warning has cleared and the connection has completed.As per coding guidelines: Active voice required.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/brev-web-ui.md` around lines 101 - 102, Change the passive sentence "Refresh the dashboard to see if the warning is resolved and the connection is established." to active voice; e.g., instruct the user to "Refresh the dashboard to confirm that the dashboard clears the warning and the gateway shows as connected" or similar so the subject performs the action (e.g., "dashboard clears" / "gateway shows as connected"). Update the sentence in the same paragraph following "Wait for about a few minutes..." and ensure it maintains the same meaning and imperative tone.
10-11: ⚡ Quick winAvoid hedge words.
"want to" is a hedge word. State the use case directly.
-Use this guide when you want to try NemoClaw without installing the CLI or using a local GPU. -If you want to manage the remote host from a terminal, see Deploy to a Remote GPU Instance (use the `nemoclaw-user-deploy-remote` skill). +Use this guide when trying NemoClaw without installing the CLI or using a local GPU. +To manage the remote host from a terminal, see Deploy to a Remote GPU Instance (use the `nemoclaw-user-deploy-remote` skill).As per coding guidelines: No hedge words ("simply," "just," "easily," "of course," "want to").
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/brev-web-ui.md` around lines 10 - 11, Replace hedged phrasing "want to try NemoClaw without installing the CLI or using a local GPU" and "If you want to manage the remote host from a terminal" with direct statements: e.g., "Use this guide to try NemoClaw without installing the CLI or using a local GPU." and "To manage the remote host from a terminal, see Deploy to a Remote GPU Instance (use the `nemoclaw-user-deploy-remote` skill)." Update the two sentences containing "want to" so they state the use case directly without hedge words.
100-101: ⚡ Quick winUse active voice.
"Pairing required" is a passive UI message. "Wait for about a few minutes for pairing to finish automatically" is also passive.
-Wait for about a few minutes for pairing to finish automatically. Refresh the dashboard to see if the warning is resolved and the connection is established. +Wait about a few minutes for the gateway to finish pairing automatically. +Refresh the dashboard to see if the warning is resolved and the connection is established.As per coding guidelines: Active voice required.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/brev-web-ui.md` around lines 100 - 101, Change the passive UI text to active voice: replace the "Pairing required" message and the sentence "Wait for about a few minutes for pairing to finish automatically. Refresh the dashboard to see if the warning is resolved and the connection is established." with active phrasing (e.g., "Gateway is completing pairing in the background. Please wait a few minutes for it to finish, then refresh the dashboard to confirm the connection."). Update the UI string(s) that render "Pairing required" and the accompanying guidance text so they use active verbs and direct instructions.skills/nemoclaw/nemoclaw-user-deploy-remote/references/install-openclaw-plugins.md (1)
12-13: ⚡ Quick winUse active voice.
"the supported NemoClaw path for OpenClaw plugins is to bake" uses passive linking verb construction. State the path directly.
-Today, the supported NemoClaw path for OpenClaw plugins is to bake the plugin -into a custom sandbox image and onboard from that Dockerfile. +Today, NemoClaw supports OpenClaw plugins by baking the plugin into a custom sandbox image and onboarding from that Dockerfile.As per coding guidelines: Active voice required for docs/** files.
🤖 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 `@skills/nemoclaw/nemoclaw-user-deploy-remote/references/install-openclaw-plugins.md` around lines 12 - 13, Rewrite the passive sentence "Today, the supported NemoClaw path for OpenClaw plugins is to bake the plugin into a custom sandbox image and onboard from that Dockerfile." in active voice; e.g., start with the actor/action such as "Bake the plugin into a custom sandbox image and onboard it from that Dockerfile" or "Bake your OpenClaw plugin into a custom sandbox image and onboard it from that Dockerfile" to make the instruction direct and active in the install-openclaw-plugins.md content.skills/nemoclaw/nemoclaw-user-manage-policy/references/integration-policy-examples.md (3)
87-88: ⚡ Quick winUse active voice.
Line 88: "is not covered by" uses passive voice.
Rewrite to make the maintained preset the subject performing the action.
Suggested rewrite
If the tool still fails, run `openshell term`, trigger the workflow again, and inspect the blocked request. -If the blocked endpoint is not covered by the maintained `outlook` preset, treat it as a separate policy review instead of assuming it is part of the supported preset. +If the maintained `outlook` preset does not cover the blocked endpoint, treat it as a separate policy review instead of assuming it is part of the supported preset.As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 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 `@skills/nemoclaw/nemoclaw-user-manage-policy/references/integration-policy-examples.md` around lines 87 - 88, Rewrite the sentence to use active voice by making the maintained `outlook` preset the subject; replace the passive phrase "is not covered by" with an active construction such as "does not cover" so the sentence reads like "If the maintained `outlook` preset does not cover the blocked endpoint, treat it as a separate policy review…" referencing the phrase "is not covered by" and the `outlook` preset for locating the change.
5-5: ⚡ Quick winUse active voice.
"when a sandbox is already installed" uses passive voice.
Rewrite to make the action clearer.
Suggested rewrite
-Use these examples when a sandbox is already installed and an integration needs network access. +Use these examples when you have already installed a sandbox and an integration needs network access.Or:
-Use these examples when a sandbox is already installed and an integration needs network access. +Use these examples when a sandbox already exists and an integration needs network access.As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 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 `@skills/nemoclaw/nemoclaw-user-manage-policy/references/integration-policy-examples.md` at line 5, The sentence "Use these examples when a sandbox is already installed and an integration needs network access." is passive; rewrite it in active voice to make the actor explicit. Replace that line (the sentence starting "Use these examples when a sandbox is already installed and an integration needs network access.") with an active construction such as "Use these examples after you install a sandbox when an integration requires network access" or "After you install a sandbox, use these examples to grant network access to an integration," ensuring the phrasing makes the actor ("you") and action explicit.
33-34: ⚡ Quick winUse active voice.
Line 34: "that can be reviewed and replayed" uses passive voice.
Make the actor explicit.
Suggested rewrite
Approve a request only when you understand why the integration needs it. -An approval updates the running policy, but it does not create a NemoClaw preset entry that can be reviewed and replayed like `policy-add`. +An approval updates the running policy, but does not create a NemoClaw preset entry you can review and replay like `policy-add`.As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 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 `@skills/nemoclaw/nemoclaw-user-manage-policy/references/integration-policy-examples.md` around lines 33 - 34, Rewrite the passive phrase "that can be reviewed and replayed" to active voice by naming the actor (e.g., "reviewers" or "users"); update the sentence so it reads like: an approval updates the running policy but does not create a NemoClaw preset entry that reviewers/users can inspect and replay like `policy-add`. Locate and edit the sentence containing "that can be reviewed and replayed" in the integration-policy-examples.md paragraph referencing `policy-add`.skills/nemoclaw/nemoclaw-user-configure-security/references/best-practices.md (2)
110-110: ⚡ Quick winMultiple sentences on the same line in table cell.
This table cell contains four sentences on a single line, which makes diffs harder to read.
Format table cells with one sentence per line in the source.As per coding guidelines: "One sentence per line in source (makes diffs readable). Flag paragraphs where multiple sentences appear on the same line."
🤖 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 `@skills/nemoclaw/nemoclaw-user-configure-security/references/best-practices.md` at line 110, The single table cell starting with "Default | Some endpoints allow GET and POST on `/**`..." contains multiple sentences on one line; split this cell so each sentence is on its own source line (e.g., one line for "Some endpoints allow GET and POST on `/**` (for example, `clawhub.ai`).", one line for "Others restrict methods and paths to specific API routes (for example, `integrate.api.nvidia.com` allows POST only to inference and embedding paths and GET to model listings).", one line for "Read-only endpoints such as `docs.openclaw.ai`, the `npm_registry` baseline entry, and the `pypi` preset allow GET only (PyPI also allows HEAD).", and one line for "The `npm` preset is an intentional exception: npm/Yarn registry traffic uses L4 pass-through for Node 22 undici CONNECT compatibility."), preserving the table pipe separators and overall markdown table structure.
68-511: ⚖️ Poor tradeoffFormat table cells with one sentence per line.
Throughout this file, many table cells contain multiple sentences on a single line (e.g., lines 87, 101, 110, 112, 124, 135, and many others).
This makes diffs harder to review.Place each sentence on its own line within table cells in the source markdown.
As per coding guidelines: "One sentence per line in source (makes diffs readable). Flag paragraphs where multiple sentences appear on the same line."
🤖 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 `@skills/nemoclaw/nemoclaw-user-configure-security/references/best-practices.md` around lines 68 - 511, The markdown tables contain multiple sentences on single lines which makes diffs noisy; edit the table cells throughout this document (e.g., rows under "Deny-by-Default Egress", "Binary-Scoped Endpoint Rules", "Path-Scoped HTTP Rules", "L4-Only vs L7 Inspection", and other table entries) so each sentence in a table cell is on its own line. Locate the tables by their unique headings (for example the "Network Controls" section and the policy preset table with entries like `github`, `pypi`, `npm`) and for every cell that currently has multiple sentences on one line, break the cell content into separate lines so there is exactly one sentence per line; preserve existing wording and punctuation, avoid changing sentence text or meaning, and ensure table pipe alignment remains valid after the changes.skills/nemoclaw/nemoclaw-user-manage-policy/references/approve-network-requests.md (1)
45-47: ⚡ Quick winUse active voice.
Line 46: "They are not persisted" is passive.
Line 47: "To keep an endpoint allowed" uses a passive construction.Rewrite in active voice.
Suggested rewrite
Approved endpoints remain in the running policy until the sandbox stops. -They are not persisted to the baseline policy file. -To keep an endpoint allowed after a restart, update the policy YAML or apply a preset as described in Customize the Sandbox Network Policy (use the `nemoclaw-user-manage-policy` skill). +The system does not persist them to the baseline policy file. +To allow an endpoint after a restart, update the policy YAML or apply a preset as described in Customize the Sandbox Network Policy (use the `nemoclaw-user-manage-policy` skill).As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 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 `@skills/nemoclaw/nemoclaw-user-manage-policy/references/approve-network-requests.md` around lines 45 - 47, Rewrite the two passive sentences to active voice: replace "They are not persisted to the baseline policy file." with an active form such as "The sandbox does not persist approved endpoints to the baseline policy file." and replace "To keep an endpoint allowed after a restart, update the policy YAML or apply a preset as described in Customize the Sandbox Network Policy (use the `nemoclaw-user-manage-policy` skill)." with an active imperative like "Update the policy YAML or apply a preset (see Customize the Sandbox Network Policy) to keep an endpoint allowed after a restart." Ensure the new sentences preserve the original meaning and mention the `nemoclaw-user-manage-policy` skill.skills/nemoclaw/nemoclaw-user-get-started/SKILL.md (1)
340-344: ⚡ Quick winCLI code blocks must use
consolelanguage tag with$prompt prefix.Even when showing commands that span host and sandbox contexts, use the
consolelanguage tag for clarity.📝 Proposed fix
-```bash -nemoclaw my-assistant connect -# inside the sandbox: -openclaw tui -``` +```console +$ nemoclaw my-assistant connect +# inside the sandbox: +$ openclaw tui +```As per coding guidelines: CLI code blocks must use the
consolelanguage tag with$prompt prefix.🤖 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 `@skills/nemoclaw/nemoclaw-user-get-started/SKILL.md` around lines 340 - 344, Update the CLI code block showing "nemoclaw my-assistant connect" and "openclaw tui" to use the console language tag and include a leading $ prompt on each command: replace the existing ```bash block with ```console and change the lines to "$ nemoclaw my-assistant connect" and "$ openclaw tui" while keeping the comment "# inside the sandbox:" intact; ensure the closing ``` remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c88ddace-c39b-4ace-a596-768466c3cf9b
⛔ Files ignored due to path filters (1)
skills/nemoclaw/nemoclaw-user-overview/references/images/nemoclaw-highlevel-component-diagram.pngis excluded by!**/*.png
📒 Files selected for processing (51)
.agents/catalog-skills.yaml.gitattributes.github/CODEOWNERS.github/workflows/catalog-skills-refresh.yaml.github/workflows/pr.yaml.pre-commit-config.yamldocs/catalog-skills-signing-flow.mdscripts/export-catalog-skills.pyskills/nemoclaw/README.mdskills/nemoclaw/catalog-metadata.jsonskills/nemoclaw/nemoclaw-skills-guide/SKILL.mdskills/nemoclaw/nemoclaw-user-agent-skills/SKILL.mdskills/nemoclaw/nemoclaw-user-agent-skills/references/agent-skills.mdskills/nemoclaw/nemoclaw-user-configure-inference/SKILL.mdskills/nemoclaw/nemoclaw-user-configure-inference/references/inference-options.mdskills/nemoclaw/nemoclaw-user-configure-inference/references/set-up-sub-agent.mdskills/nemoclaw/nemoclaw-user-configure-inference/references/switch-inference-providers.mdskills/nemoclaw/nemoclaw-user-configure-inference/references/tool-calling-reliability.mdskills/nemoclaw/nemoclaw-user-configure-security/SKILL.mdskills/nemoclaw/nemoclaw-user-configure-security/references/best-practices.mdskills/nemoclaw/nemoclaw-user-configure-security/references/credential-storage.mdskills/nemoclaw/nemoclaw-user-configure-security/references/openclaw-controls.mdskills/nemoclaw/nemoclaw-user-deploy-remote/SKILL.mdskills/nemoclaw/nemoclaw-user-deploy-remote/references/brev-web-ui.mdskills/nemoclaw/nemoclaw-user-deploy-remote/references/install-openclaw-plugins.mdskills/nemoclaw/nemoclaw-user-deploy-remote/references/sandbox-hardening.mdskills/nemoclaw/nemoclaw-user-get-started/SKILL.mdskills/nemoclaw/nemoclaw-user-get-started/references/prerequisites.mdskills/nemoclaw/nemoclaw-user-get-started/references/quickstart-hermes.mdskills/nemoclaw/nemoclaw-user-get-started/references/windows-preparation.mdskills/nemoclaw/nemoclaw-user-manage-policy/SKILL.mdskills/nemoclaw/nemoclaw-user-manage-policy/references/approve-network-requests.mdskills/nemoclaw/nemoclaw-user-manage-policy/references/integration-policy-examples.mdskills/nemoclaw/nemoclaw-user-manage-sandboxes/SKILL.mdskills/nemoclaw/nemoclaw-user-manage-sandboxes/references/backup-restore.mdskills/nemoclaw/nemoclaw-user-manage-sandboxes/references/messaging-channels.mdskills/nemoclaw/nemoclaw-user-manage-sandboxes/references/runtime-controls.mdskills/nemoclaw/nemoclaw-user-manage-sandboxes/references/workspace-files.mdskills/nemoclaw/nemoclaw-user-monitor-sandbox/SKILL.mdskills/nemoclaw/nemoclaw-user-overview/SKILL.mdskills/nemoclaw/nemoclaw-user-overview/references/ecosystem.mdskills/nemoclaw/nemoclaw-user-overview/references/how-it-works.mdskills/nemoclaw/nemoclaw-user-overview/references/overview.mdskills/nemoclaw/nemoclaw-user-overview/references/release-notes.mdskills/nemoclaw/nemoclaw-user-reference/SKILL.mdskills/nemoclaw/nemoclaw-user-reference/references/architecture.mdskills/nemoclaw/nemoclaw-user-reference/references/cli-selection-guide.mdskills/nemoclaw/nemoclaw-user-reference/references/commands.mdskills/nemoclaw/nemoclaw-user-reference/references/network-policies.mdskills/nemoclaw/nemoclaw-user-reference/references/troubleshooting.mdtest/catalog-skills-export.test.ts
|
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. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
miyoungc
approved these changes
May 27, 2026
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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)
docs/catalog-skills-signing-flow.md (1)
1-4:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd required docs frontmatter to this new page.
This new
docs/page starts with SPDX + H1 but is missing the required YAML frontmatter fields.As per coding guidelines, “Frontmatter includes title, description, keywords, topics, tags, content type, difficulty, audience, and status fields.”
🤖 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/catalog-skills-signing-flow.md` around lines 1 - 4, The new Markdown page "NemoClaw catalog skills signing flow" is missing the required YAML frontmatter; add a YAML block at the very top of docs/catalog-skills-signing-flow.md containing the fields title, description, keywords, topics, tags, content type, difficulty, audience, and status (populate each with appropriate brief values), ensuring the frontmatter appears before the SPDX header and H1 so the docs pipeline can parse metadata correctly.
♻️ Duplicate comments (1)
.pre-commit-config.yaml (1)
164-164:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpand directory patterns so nested file changes trigger the hook.
The current regex matches the directory token, not files beneath it, so edits under
.agents/skills/**andskills/nemoclaw/**can be missed.Suggested fix
- files: ^(\.agents/catalog-skills\.yaml|\.agents/skills/|skills/nemoclaw/|scripts/export-catalog-skills\.py)$ + files: ^(\.agents/catalog-skills\.yaml|\.agents/skills/.*|skills/nemoclaw/.*|scripts/export-catalog-skills\.py)$🤖 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 @.pre-commit-config.yaml at line 164, Update the files regex inside .pre-commit-config.yaml (the files: entry) so directory tokens like ".agents/skills/" and "skills/nemoclaw/" match nested files too; replace the current patterns that only match the directory token with patterns that include recursive matches (e.g., change ".agents/skills/" and "skills/nemoclaw/" to patterns that allow ".*" beneath them or use a non-capturing group like "(?:.../.*)"), keeping the other tokens (".agents/catalog-skills.yaml" and "scripts/export-catalog-skills.py") intact.
🤖 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 `@docs/catalog-skills-signing-flow.md`:
- Around line 1-4: The new Markdown page "NemoClaw catalog skills signing flow"
is missing the required YAML frontmatter; add a YAML block at the very top of
docs/catalog-skills-signing-flow.md containing the fields title, description,
keywords, topics, tags, content type, difficulty, audience, and status (populate
each with appropriate brief values), ensuring the frontmatter appears before the
SPDX header and H1 so the docs pipeline can parse metadata correctly.
---
Duplicate comments:
In @.pre-commit-config.yaml:
- Line 164: Update the files regex inside .pre-commit-config.yaml (the files:
entry) so directory tokens like ".agents/skills/" and "skills/nemoclaw/" match
nested files too; replace the current patterns that only match the directory
token with patterns that include recursive matches (e.g., change
".agents/skills/" and "skills/nemoclaw/" to patterns that allow ".*" beneath
them or use a non-capturing group like "(?:.../.*)"), keeping the other tokens
(".agents/catalog-skills.yaml" and "scripts/export-catalog-skills.py") intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a597aeb3-9b52-4004-bbf3-2c2f7ed48047
📒 Files selected for processing (5)
.github/workflows/pr.yaml.pre-commit-config.yamldocs/catalog-skills-signing-flow.mdscripts/export-catalog-skills.pytest/catalog-skills-export.test.ts
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/pr.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- test/catalog-skills-export.test.ts
- scripts/export-catalog-skills.py
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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/catalog-skills-export.test.ts`:
- Around line 26-37: The test currently depends on repoRoot state; make it
hermetic by running the exporter in a fresh temporary workspace where the export
dir cannot exist: create a temp directory (like other tests do), set the
execFileSync cwd to that temp path instead of repoRoot, ensure no skills/export
dir is present, then run execFileSync(exporter, ["--check","--allow-missing"], {
cwd: tempPath, encoding: "utf8" }) and assert the exact "Catalog export is not
present yet" message; update the test that calls execFileSync and references
exporter/repoRoot to use the temp workspace variable and clean it up after the
test.
🪄 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: b97b3979-2cbf-47aa-90da-ee5a63728fe3
📒 Files selected for processing (2)
scripts/export-catalog-skills.pytest/catalog-skills-export.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/export-catalog-skills.py
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
added a commit
that referenced
this pull request
May 27, 2026
## Summary The Skills / Catalog Refresh workflow (merged via #4284 for issue #4282) fails immediately on every dispatch: ``` Unable to resolve action `peter-evans/create-pull-request@8ce3b843f60ac63fbde403f79364ff7d80b5fbb1`, unable to find version `8ce3b843f60ac63fbde403f79364ff7d80b5fbb1` ``` That SHA does not exist in the upstream `peter-evans/create-pull-request` repo, so GitHub fails at "Prepare all required actions" before any job step runs. Failing run: [26518855615](https://github.com/NVIDIA/NemoClaw/actions/runs/26518855615). ## Fix Replace the third-party action with an inline `gh`-CLI script. This is the pattern every other NemoClaw workflow uses for GitHub mutations: | Workflow | Pattern | |---|---| | `assign-linked-issue-author.yaml` | `gh issue edit … --add-assignee`, `gh pr view`, `gh pr list` | | `pr-limit.yaml` | `gh pr list`, `gh pr comment`, `gh pr close` | | `e2e-advisor.yaml` / `pr-review-advisor.yaml` | `gh api`, `gh pr comment` | | `catalog-skills-refresh.yaml` (already, for `/nvskills-ci`) | `gh pr comment` | `peter-evans/create-pull-request` was the **only** third-party mutation action in the repo, and the only place we had to maintain a SHA pin against an external action's release cadence. ## Changes - `.github/workflows/catalog-skills-refresh.yaml` - Removed the broken `peter-evans/create-pull-request@<bad-sha>` step. - Replaced it with an inline `git checkout -B` / `git push --force-with-lease` / `gh pr create` script, idempotent against an existing open PR via `gh pr list --head`. - Flipped `persist-credentials: false` → `true` on the checkout so `git push` has a token. - Switched the `/nvskills-ci` step from `secrets.GITHUB_TOKEN` to `github.token` for consistency with the rest of the workflow and the repo. - `.github/pr-bodies/catalog-skills-refresh.md` (new) - Moved the long PR body out of the YAML to keep the workflow readable. ## Validation - `actionlint` clean (no new warnings). - Idempotency: re-dispatch reuses the existing open PR via `gh pr list --head` instead of creating a duplicate. - Dry-run path is unchanged — the new step is gated by the same `if:` condition as before. ## Notes - We could re-pin `peter-evans/create-pull-request` to the real `v7.0.8` SHA (`271a8d0340265f705b14b6d32b9829c1cb33d45e`) instead. Choosing the inline script because it's the existing repo convention and removes the supply-chain pin entirely. Refs #4282 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved automation for catalog refreshes with safer credential handling, preservation of existing signing artifacts during refreshes, and a more robust inline process for creating/updating refresh branches and PRs. * Refined token usage for the signing request step. * **Documentation** * Added a PR body template detailing regeneration, validation checks, and how to request signing when needed. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4334?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
12 tasks
jyaunches
added a commit
that referenced
this pull request
May 27, 2026
…resh diff (#4342) <!-- markdownlint-disable MD041 --> ## Summary Two related fixes to the NemoClaw catalog skills export so the `Skills / Catalog Refresh` workflow actually opens a refresh PR on `main`: 1. **Detect untracked files in the change-detection diff.** The workflow ran `git diff --quiet` which only inspects tracked paths, so a freshly-exported catalog against an empty tracked tree looked unchanged and the workflow short-circuited via "Catalog skill export is already current." without ever pushing the 43 generated files. 2. **Flatten the export from `skills/nemoclaw/` to `skills/`.** Every other onboarded NVSkills product repo (`cuopt`, `nurec-skills`, `digital-health-skills`, `aiq`) — and `nvskills-ci` itself, with `skills/ci-smoke-test/SKILL.md` — uses `skills/<skill-name>/SKILL.md`. The per-product namespace layer in NemoClaw was redundant and didn't match anyone else's layout. ## Related Issue Follow-up to #4282 / #4284 / #4334. Observed on post-merge runs [26526695773](https://github.com/NVIDIA/NemoClaw/actions/runs/26526695773) and [26526772139](https://github.com/NVIDIA/NemoClaw/actions/runs/26526772139): both completed `success` after exporting 11 skills, then exited via `Stop after dry run` without producing a PR because no diff was detected against the empty `skills/` tree on `main`. ## Changes **Diff fix (commit 1):** - `.github/workflows/catalog-skills-refresh.yaml`: run `git add --intent-to-add` against the export paths before `git diff --quiet` so untracked files surface as additions. **Layout flattening (commit 2):** - `.agents/catalog-skills.yaml`: `export: skills/nemoclaw` → `export: skills` - `scripts/export-catalog-skills.py`: default target updated - `.github/workflows/catalog-skills-refresh.yaml`: every `skills/nemoclaw` reference (preserve-overlay, change-detection, commit-add) updated to `skills` - `.pre-commit-config.yaml`: hook regex now anchors to `skills/.*` - `.github/catalog-skills-signing-flow.md`, `.github/pr-bodies/catalog-skills-refresh.md`: prose updated - `test/catalog-skills-export.test.ts`: temp-fixture paths updated ## Why dropping the `nemoclaw/` subdir is safe Verified directly against `NVIDIA/nvskills-ci` (cloned locally): - `scripts/validate_request.py:21-26` — `WATCHED_PATH_PREFIXES = (".agents/skills/", "skills/", "team-skills/", "rules/team-rules/", "plugins/")` and `is_watched_path = path.startswith(WATCHED_PATH_PREFIXES)`. **Plain prefix match — no namespacing requirement.** - `.github/workflows/team-request.yml:114` mirrors the same `startswith("skills/")` filter. - `docs/team-onboarding.md:27` — *"Store NVSkills content under `skills/`, `team-skills/`, ..."* — no per-product subdir mentioned. - `nvskills-ci`'s own example skill is at `skills/ci-smoke-test/SKILL.md`. - Every other product repo follows the same pattern (verified via GitHub API on `cuopt`, `nurec-skills`, `digital-health-skills`, `aiq`). ## Type of Change - [x] 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) ## Verification Reproduced and validated locally on a clean `main` checkout: ``` $ python3 scripts/export-catalog-skills.py Exported 11 catalog skill(s) to skills $ ls skills/ catalog-metadata.json nemoclaw-skills-guide nemoclaw-user-agent-skills nemoclaw-user-configure-inference ... (11 skills, flat layout) # Old change-detection (broken — misses untracked tree): $ git diff --quiet -- .agents/catalog-skills.yaml skills && echo changed=false || echo changed=true changed=false # Fixed change-detection: $ git add --intent-to-add -- .agents/catalog-skills.yaml skills $ git diff --quiet -- .agents/catalog-skills.yaml skills && echo changed=false || echo changed=true changed=true $ git diff --stat -- .agents/catalog-skills.yaml skills | tail -1 44 files changed, 8907 insertions(+), 1 deletion(-) ``` Vitest: ``` $ npx vitest run test/catalog-skills-export.test.ts --project cli ✓ |cli| test/catalog-skills-export.test.ts (4 tests) 1201ms Test Files 1 passed (1) Tests 4 passed (4) ``` - [x] `npx prek run --all-files` passes - [x] `npm test` passes for the affected test file (4/4) - [x] Tests updated for changed export path - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes — N/A (CI-internal) - [ ] `npm run docs` builds without warnings — N/A - [ ] Doc pages follow the style guide — N/A - [ ] New doc pages include SPDX header and frontmatter — N/A --- Signed-off-by: Justin Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Refresh process updated to regenerate and preserve exports under the top-level skills/ directory, improving detection and staging of newly created files and signer artifacts. * Pre-commit checks widened to run against the broader skills/ export tree. * **Documentation** * Updated signing-flow and PR guidance to reference the skills/ export location. * **Tests** * Test fixtures aligned to the new skills/ export layout. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4342?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
10 tasks
cv
pushed a commit
that referenced
this pull request
May 27, 2026
## Summary - Add the v0.0.53 release notes with the user-facing onboarding, inference, policy, runtime, Hermes, and maintainer-tooling changes from the release range. - Refresh generated `nemoclaw-user-*` skills from the current Fern docs, including already-merged policy, inference, troubleshooting, and command-reference updates. - Remove skipped experimental shield wording from generated-doc source so the release-prep skip-term gate stays clean. ## Source summary - #4197 -> `docs/about/release-notes.mdx`, `docs/reference/commands.mdx`: Document pre-recreate workspace backup, abort-on-partial-backup behavior, and `NEMOCLAW_RECREATE_WITHOUT_BACKUP`. - #4273 -> `docs/about/release-notes.mdx`, `docs/reference/troubleshooting.mdx`: Document the under-provisioned runtime prompt defaulting to abort in interactive onboarding. - #4220 -> `docs/about/release-notes.mdx`, `docs/network-policy/customize-network-policy.mdx`, `docs/network-policy/integration-policy-examples.mdx`: Include the `openclaw-pricing` preset and generated skill refresh. - #4253 -> `docs/about/release-notes.mdx`, `docs/inference/use-local-inference.mdx`, `docs/inference/switch-inference-providers.mdx`: Carry the Ollama runtime context-window docs into generated skills. - #4298 -> `docs/about/release-notes.mdx`, `docs/reference/troubleshooting.mdx`: Carry WSL Docker Desktop GPU guidance into generated skills and release notes. - #4297, #4210, #4221, #4225, #4288, #4306, #4311, #4319, #4342, #4284, #3327 -> `docs/about/release-notes.mdx`: Summarize release-range fixes and maintainer tooling changes that did not need new standalone docs pages. ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `rg "permissive mode|shields down|shields up|shields status|config rotate-token|rotate-token" docs .agents/skills` returned no matches outside `docs/.docs-skip`. - `npm run docs` passes with full network access. Fern reports 0 errors and one existing light-mode accent contrast warning. - `FERN_VERSION=$(node -p "require('./fern/fern.config.json').version") && (cd fern && npx --yes "fern-api@${FERN_VERSION}" check --warnings)` reports 0 errors and the same contrast warning. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added v0.0.53 release notes with updates to onboarding, sandbox recreation, and gateway handling * Introduced `openclaw-pricing` preset for model pricing endpoint management * Clarified Ollama context window configuration and local model validation behavior * Updated sandbox recreation workflow documentation with backup/restore details * Enhanced interactive onboarding defaults for under-provisioned runtime warnings * Revised security guidance for configuration directory permissions and immutability verification <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4360?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced May 28, 2026
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
4 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
Adds a deterministic generated export for NemoClaw's public skills so NVSkills can sign and catalog-sync the customer-facing skill set. The PR also adds PR-time freshness checks, a manual/scheduled refresh workflow with dry-run support, and a Mermaid sequence diagram showing the human and automation handoffs.
Related Issue
Fixes #4282
Changes
.agents/catalog-skills.yamlas the explicit allowlist for catalog-safe skills.scripts/export-catalog-skills.pyto copy allowlisted skills intoskills/nemoclaw/, write deterministic metadata, and preserveskill.oms.sig/skill-card.mdsigning artifacts.skills/nemoclaw/export fornemoclaw-skills-guideandnemoclaw-user-*skills.CI / Pull Requestand pre-commit gates that runpython3 scripts/export-catalog-skills.py --check..github/workflows/catalog-skills-refresh.yamlwith dry-run refresh, PR creation/update, and optional/nvskills-cicomment.docs/catalog-skills-signing-flow.mdwith a Mermaid sequence chart for source → export → signing →NVIDIA/skillssync..gitattributescoverage for catalog skill paths.test/catalog-skills-export.test.tsfor export freshness, allowlist scope, and signing artifact preservation.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Notes:
python3 scripts/export-catalog-skills.py --check,npx vitest run test/catalog-skills-export.test.ts,npx prek run catalog-skills-export --files .agents/catalog-skills.yaml scripts/export-catalog-skills.py skills/nemoclaw/README.md,npx prek run check-yaml --all-files,python3 -m py_compile scripts/export-catalog-skills.py, andgit diff --check.npx prek run --all-filesfailed becausenemoclaw/distwas missing and local 5s timeouts were too low. Aftercd nemoclaw && npm run buildand rerunning withNEMOCLAW_TEST_TIMEOUT=30000, all unrelated checks passed except 7 pre-existing localtest/sandbox-connect-inference.test.tstimeout/status failures; the new catalog checks passed.Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
New Features
Tests
Documentation
Chores