fix(blueprint): reload private network cache on file changes#4092
Merged
ericksoa merged 2 commits intoMay 26, 2026
Conversation
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:
📝 WalkthroughWalkthroughThe private-networks YAML loader now checks ChangesPrivate Networks Cache Invalidation via File Metadata
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 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 unit tests (beta)
Comment |
a69f4be to
13f4f42
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the blueprint private-networks loader to support hot-reloading when the private-networks.yaml file changes, while keeping the SSRF validation behavior consistent.
Changes:
- Reload
node:netBlockListwhenprivate-networks.yaml’s file stats change (mtime/size) - Refactor IP check logic into a helper to reuse an already-loaded
BlockList - Extend unit tests and fs mocks to cover the new reload behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| nemoclaw/src/blueprint/private-networks.ts | Add stat-based cache invalidation + helper for IP checks using a provided BlockList |
| nemoclaw/src/blueprint/private-networks.test.ts | Update node:fs mock to support statSync and add a test for reload-on-change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@nemoclaw/src/blueprint/private-networks.test.ts`:
- Around line 285-297: The regression test "reloads when private-networks.yaml
mtime changes" currently rewrites the YAML with different content length so both
mtime and size change; make the test isolate mtime by ensuring the file size
stays the same when calling seedYaml or by updating only the file mtime instead
of replacing content. Concretely, in the test that calls getPrivateNetworks(),
change seedYaml("/blueprint/private-networks.yaml", ...) so the new YAML payload
is padded to the original byte length (or alternatively replace the seedYaml
call with a touch using fs.utimesSync or a helper that updates mtime only), then
assert that getPrivateNetworks() returns a new object and isPrivateHostname
behaves as expected. Ensure references to getPrivateNetworks(), seedYaml (or the
fs.utimesSync touch), and isPrivateHostname are used so the reload is driven
solely by mtime change.
In `@nemoclaw/src/blueprint/private-networks.ts`:
- Around line 149-162: The code currently translates ENOENT only during statSync
but not during the subsequent readFileSync, so if the file is deleted between
those calls you get a raw fs ENOENT instead of the standardized
missingPrivateNetworksError; update the logic in the private-networks loader
(the block that calls statSync, readFileSync and parseDocument) to catch ENOENT
from the read step as well and rethrow missingPrivateNetworksError(source) (e.g.
wrap the readFileSync + parseDocument call in a try/catch that checks err.code
=== "ENOENT" and throws missingPrivateNetworksError(source)), keeping the
existing cached check and preserving mtimeMs/size behavior.
🪄 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: 0b9cd3d5-73d5-48cd-8773-c017ceb23d77
📒 Files selected for processing (2)
nemoclaw/src/blueprint/private-networks.test.tsnemoclaw/src/blueprint/private-networks.ts
9f359e9 to
47874df
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
nemoclaw/src/blueprint/private-networks.test.ts (1)
329-334:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIsolate the mtime reload test from size changes
On Line 331, the test uses
seedYaml(...), which also changes filesizeviasizes.set(...). That means this case still validates “mtime-or-size changed”, not strictly mtime-driven reload.Suggested patch
it("reloads when private-networks.yaml mtime changes", () => { - const before = getPrivateNetworks(); - seedYaml( - "/blueprint/private-networks.yaml", - "ipv4:\n - address: 8.8.8.0\n prefix: 24\n purpose: after mtime change\nipv6: []\nnames: []\n", - ); + const path = "/blueprint/private-networks.yaml"; + const before = getPrivateNetworks(); + const original = store.get(path)!; + store.set(path, original.replace("10.0.0.0", "8.8.8.0")); + // keep size unchanged so this test isolates mtime-based invalidation + sizes.set(path, original.length); + mtimes.set(path, (mtimes.get(path) ?? 0) + 1); advanceStatInterval(); const after = getPrivateNetworks(); expect(after).not.toBe(before); expect(isPrivateHostname("8.8.8.1")).toBe(true); expect(isPrivateHostname("10.0.0.1")).toBe(false); });🤖 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 `@nemoclaw/src/blueprint/private-networks.test.ts` around lines 329 - 334, The test "reloads when private-networks.yaml mtime changes" currently uses seedYaml(...) which also updates the sizes map and therefore validates mtime-or-size changes; change the test to only modify the file mtime without altering size: either replace the seedYaml call with a pure-mtime update (e.g., fs.utimes/utimesSync or the project's touch helper) for the "/blueprint/private-networks.yaml" file, or if you must use seedYaml, immediately restore sizes.set("/blueprint/private-networks.yaml", previousSize) so that only the mtime differs; keep references to getPrivateNetworks() and the same file path so the test still asserts reload-on-mtime only.
🤖 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.
Duplicate comments:
In `@nemoclaw/src/blueprint/private-networks.test.ts`:
- Around line 329-334: The test "reloads when private-networks.yaml mtime
changes" currently uses seedYaml(...) which also updates the sizes map and
therefore validates mtime-or-size changes; change the test to only modify the
file mtime without altering size: either replace the seedYaml call with a
pure-mtime update (e.g., fs.utimes/utimesSync or the project's touch helper) for
the "/blueprint/private-networks.yaml" file, or if you must use seedYaml,
immediately restore sizes.set("/blueprint/private-networks.yaml", previousSize)
so that only the mtime differs; keep references to getPrivateNetworks() and the
same file path so the test still asserts reload-on-mtime only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 732fd618-d208-4dcc-9866-4ff68254c042
📒 Files selected for processing (2)
nemoclaw/src/blueprint/private-networks.test.tsnemoclaw/src/blueprint/private-networks.ts
4 tasks
Signed-off-by: 1PoPTRoN <vrxn.arp1traj@gmail.com>
47874df to
ce140e9
Compare
Contributor
Author
Contributor
|
✨ Thanks for submitting this detailed PR about refreshing the private network cache on file changes. This proposes a fix for the issue where the cache does not reload when private-networks.yaml changes, and also improves the cache behavior by tracking file metadata and reloading the parsed YAML when the file changes. Related open issues: |
ericksoa
approved these changes
May 26, 2026
ericksoa
left a comment
Contributor
There was a problem hiding this comment.
Approved. I reviewed the cache invalidation path against the linked issue and current main. The implementation reloads the plugin-side private network blocklist on source/stat changes while preserving the existing cached fast path and descriptive missing-file handling.
Local validation passed: focused private-networks tests, plugin suite, plugin build, CLI build, SSRF parity test, and a same-size mtime-only reload probe. The remaining CodeRabbit note is test precision rather than a behavior blocker.
4 tasks
cv
added a commit
that referenced
this pull request
May 27, 2026
## Summary Refresh NemoClaw documentation and regenerated user skills for the v0.0.52 release-prep window. Adds the v0.0.52 release-notes entry and regenerates `nemoclaw-user-*` skills so the published Fern docs and the agent-skill references stay in sync. ## Source summary - #4260 -> `docs/about/release-notes.mdx`: Document the OpenClaw runtime bump to 2026.5.22 and call out the `min_openclaw_version` compatibility floor versus `OPENCLAW_VERSION` Dockerfile pin. (Architecture and commands pages were already updated in #4260 itself.) - #4272 -> `docs/about/release-notes.mdx`: Document the Hermes v0.14 root-entrypoint sandbox layout repair (precreated runtime dirs, sticky group-writable `/sandbox/.hermes`, removed `gateway.pid` symlink precreation, legacy state cleanup at launch). - #4261 -> `docs/about/release-notes.mdx`: Document the onboard ready output restoration that points users at `nemoclaw <name> dashboard-url --quiet`. - #4200 -> `docs/about/release-notes.mdx`: Document Slack token validation in onboarding so invalid `SLACK_BOT_TOKEN` values trigger a re-prompt instead of silent advance. - #4278 -> `docs/about/release-notes.mdx`: Document the Windows bootstrap regression fix that restores the separate Ubuntu setup handoff window, keeps `Ubuntu-24.04` as the default distro, and documents `-DistroName Ubuntu` to reuse an existing distro. (The `docs/get-started/windows-preparation.mdx` page was already updated in #4278 itself.) - #4092 -> `docs/about/release-notes.mdx`: Document the blueprint private-network blocklist reload when `private-networks.yaml` changes on disk. - Release cleanup -> `.agents/skills/nemoclaw-user-*`: Regenerate user skills with `scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` so the agent-skill references pick up the v0.0.52 release-notes update plus the WeChat / WhatsApp doc changes that already landed in #4276. ## 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) ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` -> 10 skills, 1724 lines, 29 reference files. - `npm run docs` -> 0 errors, 1 warning (Fern check clean). - `npm run build:cli` -> success (refreshed `dist/` so the pre-push TypeScript hook passes). - Skip-list check against `docs/.docs-skip` `skip-terms`: no "permissive mode", "shields down", "shields up", "shields status", "config rotate-token", or "rotate-token" strings in `docs/` or generated skills. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Experimental WeChat messaging channel added (QR pairing), alongside Telegram, Discord, Slack, and WhatsApp. * **Documentation** * Updated onboarding, messaging-channel, CLI, and troubleshooting docs to include WeChat/WhatsApp (marked experimental) and new onboarding flags/notes. * Clarified provider validation and runtime routing behavior for OpenAI-compatible endpoints and Google Gemini. * Updated Windows bootstrap/WSL guidance to target Ubuntu 24.04. * **Chores** * Added v0.0.52 release notes (runtime upgrade, sandbox hardening, onboarding and network fixes). <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4293?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 --> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Refresh the plugin-side private network blocklist when
private-networks.yamlchanges during a long-running process. This prevents stale SSRF/private-network validation data while preserving the existing synchronous cache behavior for unchanged files.Related Issue
Fixes #4091
Changes
private-networks.yamlpath,mtimeMs, and file size in the cached private network document.BlockListwhen the file metadata changes.private-networks.yaml.isPrivateHostname()by reusing the loadedBlockList.private-networks.yamlchanges without callingresetCache().Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Additional focused verification run:
npm test -- private-networks.test.tsnpx biome check nemoclaw/src/blueprint/private-networks.ts nemoclaw/src/blueprint/private-networks.test.tsnpm --prefix nemoclaw run buildSigned-off-by: 1PoPTRoN vrxn.arp1traj@gmail.com
Summary by CodeRabbit