fix(policy): add weather preset for wttr.in and Open-Meteo#1918
fix(policy): add weather preset for wttr.in and Open-Meteo#1918BenediktSchackenberg wants to merge 7 commits into
Conversation
The built-in weather skill requires network access to wttr.in and Open-Meteo, but no policy preset existed for these endpoints. Without the preset, web_fetch calls fail and the skill cannot return weather data from inside the sandbox. Added nemoclaw-blueprint/policies/presets/weather.yaml with: - wttr.in:443 (GET) — primary weather source - api.open-meteo.com:443 (GET) — free weather API fallback - geocoding-api.open-meteo.com:443 (GET) — city name resolution Apply with: nemoclaw <sandbox> policy-add weather Also: - docs/reference/network-policies.md: note that weather preset exists but is not included in any default tier - test/validate-blueprint.test.ts: regression tests for the preset Fixes NVIDIA#1417 Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com>
|
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)
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
There was a problem hiding this comment.
Pull request overview
Adds an explicit, opt-in network policy preset to allow the built-in weather skill to fetch data from its upstream providers while keeping default policy tiers unchanged.
Changes:
- Added a new
weatherpolicy preset allowing GET-only access towttr.inand Open-Meteo (including geocoding). - Documented that the preset is not included in any default tier and must be manually applied.
- Added regression tests and regenerated the agent reference markdown artifacts.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/validate-blueprint.test.ts | Adds regression tests ensuring the new weather preset exists and covers required hosts/methods. |
| nemoclaw-blueprint/policies/presets/weather.yaml | Introduces the weather preset with required endpoints and binaries for the weather skill. |
| docs/reference/network-policies.md | Documents manual application of the weather preset and why it’s excluded from tiers. |
| .agents/skills/nemoclaw-user-reference/references/network-policies.md | Regenerated agent reference reflecting the new note about the weather preset. |
| .agents/skills/nemoclaw-user-reference/references/commands.md | Regenerated agent reference; picks up existing installer/session documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const hosts = weatherEndpoints().map((ep) => ep.host); | ||
| expect(hosts).toContain("api.open-meteo.com"); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The regression tests for the weather preset assert wttr.in and api.open-meteo.com, but do not assert geocoding-api.open-meteo.com even though the preset includes it and the PR description calls it required for city-name lookups. Add an assertion for the geocoding host so a future removal/rename doesn’t silently break the weather skill while tests still pass.
| it("regression #1417: weather preset covers geocoding-api.open-meteo.com", () => { | |
| const hosts = weatherEndpoints().map((ep) => ep.host); | |
| expect(hosts).toContain("geocoding-api.open-meteo.com"); | |
| }); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/network-policies.md`:
- Line 109: Replace the bolded callout with a MyST admonition (use :::note) and
split the sentences so each sentence is on its own source line; specifically
update the line describing the `weather` preset (`wttr.in`,
`api.open-meteo.com`, `geocoding-api.open-meteo.com`) and the installation hint
(`nemoclaw <sandbox> policy-add weather`) into separate lines inside the
admonition, and remove the GitHub issue URL
(https://github.com/NVIDIA/NemoClaw/issues/1417) entirely so the docs contain
only the official guidance and comply with the one-sentence-per-line rule.
In `@test/validate-blueprint.test.ts`:
- Around line 372-385: The test "regression `#1417`: weather preset covers
api.open-meteo.com" that collects hosts via weatherEndpoints() and asserts hosts
contains "api.open-meteo.com" should also assert that hosts contains
"geocoding-api.open-meteo.com"; update the test (the block using const hosts =
weatherEndpoints().map((ep) => ep.host) and the expect(hosts).toContain(...)
line) to include an additional
expect(hosts).toContain("geocoding-api.open-meteo.com") so removal of that host
will fail the suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fa14e68-9401-4f05-8c85-a63048c1cb6a
📒 Files selected for processing (5)
.agents/skills/nemoclaw-user-reference/references/commands.md.agents/skills/nemoclaw-user-reference/references/network-policies.mddocs/reference/network-policies.mdnemoclaw-blueprint/policies/presets/weather.yamltest/validate-blueprint.test.ts
- docs: replace Markdown blockquote with MyST admonition and remove GitHub issue URL from docs/ per coding guidelines - test: add missing assertion for geocoding-api.open-meteo.com Per Copilot + CodeRabbit review on NVIDIA#1918. Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com>
|
Addressed both review points:
|
- Split the weather preset note into one sentence per line - Keep the note concise and docs-compliant - Regenerated agent skills artifacts after the docs change Per CodeRabbit review on NVIDIA#1918. Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com>
|
Fixed the docs style issue (split into one sentence per line, kept the MyST note) and added the missing geocoding host assertion. 38 tests still pass. |
|
✨ Thanks for submitting this PR, which proposes a way to improve the NemoClaw policy by adding a weather preset for wttr.in and Open-Meteo. This could help resolve the issue reported in #1417, where the weather skill fails inside the sandbox. Possibly related open issues: |
|
Thanks for addressing the review feedback! Submitting this for team review now. |
|
Hi @BenediktSchackenberg. At this time, we are pausing net new defaults in onboarding in NemoClaw until June, once we have our partner integration story worked out. This is a good integration and I would love to do it, so we will keep this open for when we are ready to consume it. |
wscurran
added
area: cli
|
Friendly ping @ericksoa — back in April you mentioned holding off on new onboarding defaults until June while the partner integration story settled. We're there now, so wanted to surface this again. PR is still rebased and green from my side; happy to adjust the preset shape (split presets, opt-in flag, naming) if that helps it fit whatever direction the partner integrations took. |
## Summary - add read-only `weather` and `public-reference` presets with host/path/method-scoped public API egress - include `weather` in balanced/open defaults and `public-reference` in open defaults - include all Hermes Nous managed-tool policy presets for Hermes open policy selection while keeping OpenClaw open defaults agent-specific - add direct agent E2E coverage for OpenClaw balanced/open and Hermes open common-egress paths Closes #4767 Fixes #4814 ## Related - Likely supersedes #1918, which proposed an opt-in weather preset; this PR includes weather as part of the broader safe common-egress defaults. ## Tests - `npm run build:cli` - `npx vitest run test/policy-tiers.test.ts test/policy-tiers-onboard.test.ts test/onboard-policy-suggestions.test.ts test/policies.test.ts test/validate-config-schemas.test.ts` (302 passed) - `npx vitest run test/policies.test.ts test/validate-config-schemas.test.ts` (229 passed after preset binary-scope update) - `npm run validate:configs` - `bash -n test/e2e/test-common-egress-agent-e2e.sh && shellcheck test/e2e/test-common-egress-agent-e2e.sh` ## E2E status - `test/e2e/test-common-egress-agent-e2e.sh` exercises the allowed common-egress paths through agent tool use for OpenClaw balanced/open and Hermes open. - Full live execution provisions multiple sandboxes and is expected to run in CI/self-hosted runner diagnostics. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added "weather" and "public-reference" read-only presets for curated public data, geocoding, weather, and reference APIs. * **Behavior** * Tiers updated: balanced now includes "weather" by default; open includes "weather" and "public-reference". * Onboarding now filters presets by agent (Hermes vs OpenClaw) and auto-includes Hermes tool gateway presets for Hermes; agent-incompatible presets are removed when resuming with a different agent. * **Tests** * New e2e script and expanded unit/integration tests covering presets, onboarding, and agent-specific behavior. * **Chores** * Nightly CI wired to run the new e2e job. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Thanks again for putting this together and for keeping it rebased. The weather policy work has now landed on The implementation that landed is not a verbatim merge of this PR: it uses tighter path-scoped Open-Meteo / Weather.gov rules rather than the original broad |
Pull request was closed
Problem
The built-in
weatherskill fails inside the sandbox becausewttr.inandapi.open-meteo.comare not in any policy preset.web_fetchcalls returnfetch failedand the skill falls back to a text explanation that it cannot retrieve weather data.Fix
Added
nemoclaw-blueprint/policies/presets/weather.yamlwith:wttr.in:443(GET) — primary weather source used by the skillapi.open-meteo.com:443(GET) — free weather API, no key requiredgeocoding-api.open-meteo.com:443(GET) — city name → coordinates lookupApply with:
The preset is intentionally not included in any default tier — most workflows don't need weather data. Users who want the weather skill can add it explicitly.
Also updated
docs/reference/network-policies.mdwith a note about the preset, regenerated agent skills, and added 3 regression tests.Fixes #1417
Signed-off-by: Benedikt Schackenberg 6381261+BenediktSchackenberg@users.noreply.github.com
Summary by CodeRabbit
New Features
Documentation
Tests