ci: add blueprint/policy validation job and scheduled E2E workflow - #395
ci: add blueprint/policy validation job and scheduled E2E workflow#395hulynn wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdded two GitHub Actions workflow changes: a scheduled weekday E2E workflow that runs full inference tests and notifies on failure, and a PR-time YAML validator that checks blueprint profiles and network policy files. (49 words) Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant Runner as CI Runner
participant Repo as Repository (code)
participant Nemoclaw as Nemoclaw build
participant OpenShell as OpenShell installer
participant NVIDIA as NVIDIA API (secret)
participant GitHubAPI as GitHub Issue API
GitHub->>Runner: schedule/run workflow (cron or dispatch)
Runner->>Repo: checkout code
Runner->>Runner: setup Node.js, npm cache
Runner->>Nemoclaw: install deps & build `nemoclaw`
Runner->>OpenShell: run `scripts/install-openshell.sh`
Runner->>Runner: run `test/e2e/test-full-e2e.sh` (uses NVIDIA secret)
Runner->>NVIDIA: (secret used by test) request inference
alt tests succeed
Runner->>GitHub: finish workflow (success)
else tests fail
Runner->>Runner: collect `/tmp/nemoclaw-*.log`
Runner->>GitHub: upload artifact (e2e-full-logs)
GitHub->>GitHubAPI: create issue with run link & labels
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can disable poems in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/e2e-scheduled.yaml (2)
53-70: Potential for duplicate issues on same-day failures.If the workflow is triggered multiple times on the same day (e.g., via
workflow_dispatchor if cron runs overlap), issues with identical titles will be created since the title only includes the date. Also, ensure thebugandcilabels exist in the repository, otherwise issue creation will fail.Consider searching for an existing open issue before creating a new one:
♻️ Optional: Check for existing issue before creating
- name: Create issue on failure uses: actions/github-script@v7 with: script: | const title = `E2E Full pipeline failed — ${new Date().toISOString().split('T')[0]}`; + const { data: issues } = await github.rest.issues.listForRepo({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'open', + labels: 'ci', + per_page: 100 + }); + if (issues.some(i => i.title === title)) { + console.log('Issue already exists for today, skipping creation'); + return; + } const body = `The scheduled full E2E pipeline failed.\n\nRun: ${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; await github.rest.issues.create({ owner: context.repo.owner, repo: context.repo.repo, title, body, labels: ['bug', 'ci'] });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-scheduled.yaml around lines 53 - 70, The notify-on-failure job currently creates issues with a date-only title causing duplicate issues and may fail if labels 'bug' and 'ci' don't exist; update the actions/github-script block to first search for an open issue matching the same day's title using github.rest.issues.list or the search API (refer to the existing title variable), and only call github.rest.issues.create if no matching open issue is found, and additionally either append a unique suffix (e.g., ${context.runId} or timestamp) to the title variable to guarantee uniqueness; also validate or create the 'bug' and 'ci' labels via github.rest.issues.getLabel / createLabel (or handle label-not-found errors) before creating the issue so creation cannot fail due to missing labels.
40-43: Verify the workflow behavior whenNVIDIA_API_KEYis not configured.The PR description notes this secret is "not yet configured." If the secret is empty/missing, the E2E test script will run with an empty
NVIDIA_API_KEYenvironment variable, which could lead to unclear failures.Consider adding a pre-check step that fails fast with a clear message if the secret is not set:
♻️ Optional: Add secret presence check
+ - name: Verify API key is configured + run: | + if [ -z "${{ secrets.NVIDIA_API_KEY }}" ]; then + echo "::error::NVIDIA_API_KEY secret is not configured" + exit 1 + fi + - name: Run full E2E env: NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} run: bash test/e2e/test-full-e2e.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-scheduled.yaml around lines 40 - 43, Add a pre-check step before the "Run full E2E" job to fail fast when the NVIDIA_API_KEY secret is missing or empty: create a new step (e.g., "Check NVIDIA_API_KEY") that runs a shell check against the NVIDIA_API_KEY environment variable (the same env name used in the "Run full E2E" step) and prints a clear error and exits non‑zero if it's unset/empty; ensure the existing run of test/e2e/test-full-e2e.sh only proceeds when that check passes so failures are explicit and informative..github/workflows/pr.yaml (1)
77-97: Consider using context managers for file operations.The inline script uses bare
open()calls without context managers (withstatements), which can leave file handles unclosed. While this is a short-lived CI script so it's unlikely to cause real issues, using context managers is a best practice.Additionally, if any file doesn't exist, the error message will be a raw Python traceback rather than a clear CI message.
♻️ Suggested improvement with context managers and clearer errors
- name: Validate blueprint and policy presets run: | python3 -c " import yaml, os, sys - bp = yaml.safe_load(open('nemoclaw-blueprint/blueprint.yaml')) + with open('nemoclaw-blueprint/blueprint.yaml') as f: + bp = yaml.safe_load(f) for p in ['default', 'ncp', 'nim-local', 'vllm']: assert p in bp['components']['inference']['profiles'], f'Missing profile: {p}' cfg = bp['components']['inference']['profiles'][p] assert 'provider_type' in cfg and 'model' in cfg print('Blueprint: 4/4 profiles OK') for f in sorted(os.listdir('nemoclaw-blueprint/policies/presets')): if f.endswith('.yaml'): - data = yaml.safe_load(open(f'nemoclaw-blueprint/policies/presets/{f}')) - assert data and 'network_policies' in data, f'{f}: invalid' + with open(f'nemoclaw-blueprint/policies/presets/{f}') as fp: + data = yaml.safe_load(fp) + assert data, f'{f}: empty or invalid YAML' + assert 'network_policies' in data, f'{f}: missing network_policies' print('Policy presets: all OK') - policy = yaml.safe_load(open('nemoclaw-blueprint/policies/openclaw-sandbox.yaml')) + with open('nemoclaw-blueprint/policies/openclaw-sandbox.yaml') as f: + policy = yaml.safe_load(f) assert 'version' in policy, 'Base policy missing version' assert 'network_policies' in policy, 'Base policy missing network_policies' print('Base policy: OK') "🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr.yaml around lines 77 - 97, Replace the bare open(...) calls with context managers and add clearer error handling: use "with open(...)" when loading files for bp, each preset file, and policy (references: bp = yaml.safe_load(...), cfg, the for loops iterating presets, and policy = yaml.safe_load(...)); wrap each load in try/except to catch FileNotFoundError/IOError and yaml errors and print a concise, actionable CI message (and exit nonzero) rather than a raw traceback; ensure you still assert the required keys ('components'/'inference'/'profiles', 'provider_type'/'model', 'network_policies', 'version') after successful reads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-scheduled.yaml:
- Around line 11-12: The workflow is missing the issues: write permission
required by the notify-on-failure job which calls github.rest.issues.create();
update the workflow permissions block to include issues: write so the API call
can succeed (e.g., add "issues: write" alongside "contents: read") and confirm
the notify-on-failure job still runs with the updated permissions.
---
Nitpick comments:
In @.github/workflows/e2e-scheduled.yaml:
- Around line 53-70: The notify-on-failure job currently creates issues with a
date-only title causing duplicate issues and may fail if labels 'bug' and 'ci'
don't exist; update the actions/github-script block to first search for an open
issue matching the same day's title using github.rest.issues.list or the search
API (refer to the existing title variable), and only call
github.rest.issues.create if no matching open issue is found, and additionally
either append a unique suffix (e.g., ${context.runId} or timestamp) to the title
variable to guarantee uniqueness; also validate or create the 'bug' and 'ci'
labels via github.rest.issues.getLabel / createLabel (or handle label-not-found
errors) before creating the issue so creation cannot fail due to missing labels.
- Around line 40-43: Add a pre-check step before the "Run full E2E" job to fail
fast when the NVIDIA_API_KEY secret is missing or empty: create a new step
(e.g., "Check NVIDIA_API_KEY") that runs a shell check against the
NVIDIA_API_KEY environment variable (the same env name used in the "Run full
E2E" step) and prints a clear error and exits non‑zero if it's unset/empty;
ensure the existing run of test/e2e/test-full-e2e.sh only proceeds when that
check passes so failures are explicit and informative.
In @.github/workflows/pr.yaml:
- Around line 77-97: Replace the bare open(...) calls with context managers and
add clearer error handling: use "with open(...)" when loading files for bp, each
preset file, and policy (references: bp = yaml.safe_load(...), cfg, the for
loops iterating presets, and policy = yaml.safe_load(...)); wrap each load in
try/except to catch FileNotFoundError/IOError and yaml errors and print a
concise, actionable CI message (and exit nonzero) rather than a raw traceback;
ensure you still assert the required keys ('components'/'inference'/'profiles',
'provider_type'/'model', 'network_policies', 'version') after successful reads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa012b89-0a7f-4ab9-8184-34289b1cee1c
📒 Files selected for processing (2)
.github/workflows/e2e-scheduled.yaml.github/workflows/pr.yaml
Add validate-profiles job to pr.yaml that checks: - All 4 inference profiles exist in blueprint.yaml - All policy preset YAMLs have valid structure - Base sandbox policy has required fields Add e2e-scheduled.yaml for daily live inference E2E: - Runs test/e2e/test-full-e2e.sh on weekday mornings - Auto-creates GitHub issue on failure - Requires NVIDIA_API_KEY secret (not yet configured) Made-with: Cursor
3cf3497 to
2a609d5
Compare
|
Thanks for looking into this! Here's a few things I noticed since I've been working on the same thing (PR here: #386 ) On the E2E workflow:
If you could look at my PR (#386) and leave a comment on there with your thoughts about it, that would be great. On the blueprint/policy validation job on PR workflows:
Would it make sense to split the |
|
Closing as a duplicate of #386 |
|
Thanks for this PR, Lynn — really solid work jumping in with both the blueprint/policy validation and the scheduled E2E workflow. A few small things we noticed:
Neither of these is a blocker — the core work here is great. Also, if you could enable "Allow edits from maintainers" on the PR, it makes it easier for us to help with small fixups like these. |
|
Thanks for the detailed reviews @jayavenkatesh19 @ericksoa! Great catches on the GITHUB_TOKEN, fork guard, and timezone — I missed those. Since #386 covers the E2E workflow, I'll:
Appreciate the feedback — learned a lot from this first PR! |
wscurran
added
area: ci
5 participants
Add validate-profiles job to pr.yaml that checks:
Add e2e-scheduled.yaml for daily live inference E2E:
Summary by CodeRabbit