chore(release): make daily tag flow semi-automatic - #8485
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.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:
📝 WalkthroughWalkthroughThe release process now uses a 4 PM locally signed tag based on the planned ChangesRelease flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseAgent
participant originMain
participant Maintainer
participant E2EWorkflow
ReleaseAgent->>originMain: Generate 4 PM release plan
ReleaseAgent->>Maintainer: Request exact tag confirmation
Maintainer->>ReleaseAgent: Confirm local signing
ReleaseAgent->>originMain: Create signed tag with release:cut
originMain->>E2EWorkflow: Trigger SHA-bound E2E run
E2EWorkflow-->>ReleaseAgent: Return diagnostic result for overnight triage
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🤖 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 @.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md:
- Around line 20-21: Update the changelog validation instructions around the
line-40 search to use the selected release version rather than the literal
vX.Y.Z placeholder. Require exactly one dated docs/changelog/YYYY-MM-DD.mdx file
containing the exact planned ## vX.Y.Z heading, and fail validation when zero or
multiple entries match.
🪄 Autofix
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: 026e4a89-ea4b-4838-9ba9-f79f2f697508
📒 Files selected for processing (12)
.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md.agents/skills/nemoclaw-maintainer-cut-release-tag/scripts/release-e2e-evidence.mts.agents/skills/nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md.agents/skills/nemoclaw-maintainer-e2e/SKILL.md.agents/skills/nemoclaw-maintainer-evening/SKILL.md.agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md.agents/skills/nemoclaw-maintainer-policies/references/release-train.md.agents/skills/nemoclaw-skills-guide/SKILL.mdpackage.jsontest/maintainer-e2e-skill.test.tstest/maintainer-skills-policy.test.tstest/release-e2e-evidence.test.ts
💤 Files with no reviewable changes (3)
- package.json
- test/release-e2e-evidence.test.ts
- .agents/skills/nemoclaw-maintainer-cut-release-tag/scripts/release-e2e-evidence.mts
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/maintainer-skills-policy.test.ts (1)
246-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest changelog validation through observable behavior.
These assertions only check that implementation fragments occur in the Markdown. They do not prove that one matching changelog succeeds or that zero and multiple matches fail. Execute the validation command against temporary Git fixtures, or expose it through a testable script and assert its exit status and output.
As per path instructions, tests must provide behavioral confidence rather than implementation lock-in.
🤖 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 `@test/maintainer-skills-policy.test.ts` around lines 246 - 254, Replace the fragment-based assertions in the changelog validation section of maintainer-skills-policy tests with behavioral tests that run the release validation command against temporary Git fixtures. Cover exactly one matching dated changelog heading as success, and zero or multiple matches as failure, asserting exit status and relevant output rather than script text.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/maintainer-skills-policy.test.ts`:
- Around line 246-254: Replace the fragment-based assertions in the changelog
validation section of maintainer-skills-policy tests with behavioral tests that
run the release validation command against temporary Git fixtures. Cover exactly
one matching dated changelog heading as success, and zero or multiple matches as
failure, asserting exit status and relevant output rather than script text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c7fde54b-1ef5-48bc-a935-bfc551fc7b64
📒 Files selected for processing (2)
.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.mdtest/maintainer-skills-policy.test.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 4 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
🌿 Preview your docs: https://nvidia-preview-pr-8485.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit f4f8007 in the TypeScript / code-coverage/cliThe overall coverage in commit f4f8007 in the Show a code coverage summary of the most impacted files.
Updated |
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
This PR changes a release and security gate by making E2E permanently non-blocking for tags and merges, but it has no linked accepted issue or maintainer design decision. Record the decision that defines which post-merge failures can block release, who owns 4 PM–8 AM triage, and how a failing exact release SHA is handled before removing candidate-bound evidence. Then refresh onto current main and rerun the release-skill and policy contract checks.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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 @.agents/skills/nemoclaw-maintainer-policies/references/release-train.md:
- Line 34: Align the release membership contract across the release-tag skill,
label housekeeping, and tests so post-4 PM merges cannot inconsistently enter an
already planned release. Prefer freezing the candidate at the 4 PM cutoff and
tagging the immutable planned SHA, then update the rules around regenerating
plans, carry-forward handling, and related test expectations to enforce that
boundary consistently.
🪄 Autofix
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: 73dc88dc-867b-43ec-a56e-025c9a508a6d
📒 Files selected for processing (12)
.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md.agents/skills/nemoclaw-maintainer-cut-release-tag/scripts/release-e2e-evidence.mts.agents/skills/nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md.agents/skills/nemoclaw-maintainer-e2e/SKILL.md.agents/skills/nemoclaw-maintainer-evening/SKILL.md.agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md.agents/skills/nemoclaw-maintainer-policies/references/release-train.md.agents/skills/nemoclaw-skills-guide/SKILL.mdpackage.jsontest/maintainer-e2e-skill.test.tstest/maintainer-skills-policy.test.tstest/release-e2e-evidence.test.ts
💤 Files with no reviewable changes (3)
- test/release-e2e-evidence.test.ts
- package.json
- .agents/skills/nemoclaw-maintainer-cut-release-tag/scripts/release-e2e-evidence.mts
🚧 Files skipped from review as they are similar to previous changes (8)
- .agents/skills/nemoclaw-skills-guide/SKILL.md
- .agents/skills/nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md
- .agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md
- test/maintainer-e2e-skill.test.ts
- .agents/skills/nemoclaw-maintainer-e2e/SKILL.md
- test/maintainer-skills-policy.test.ts
- .agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md
- .agents/skills/nemoclaw-maintainer-evening/SKILL.md
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 (2)
.agents/skills/nemoclaw-maintainer-e2e/SKILL.md (2)
66-68: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not make every credentialed E2E the default for a PR revision.
Lines 21-28 state that empty-selector manual PR runs expose long-lived API keys and messaging credentials to candidate-controlled processes. Selecting every free-standing workflow E2E makes that exposure the default for a maintainer revision run.
maintainoradminauthorization protects dispatch. It does not restrict candidate code after checkout. Keep the default selection credential-free, or require explicit per-job opt-in with short-lived credentials and an isolated trusted boundary.🤖 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 @.agents/skills/nemoclaw-maintainer-e2e/SKILL.md around lines 66 - 68, Update the default controller-selected E2E targets in the skill so credentialed workflow tests are not selected by an empty-selector maintainer PR run. Keep the default set limited to credential-free tests, and require explicit per-job opt-in for any credentialed target with the required short-lived credentials and isolated trusted boundary.
199-199: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRequire hardware qualification evidence in full mode.
The workflow defaults both queue flags to
false, and both hardware jobs require their flag. The full-mode validator acceptsfalsefor both flags and checks only the Launchable job. Full evidence can therefore pass without Jetson or DGX Spark qualification. Restore the required inputs or validate both hardware jobs and their evidence.🤖 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 @.agents/skills/nemoclaw-maintainer-e2e/SKILL.md at line 199, Update the full-mode validation and workflow inputs around include_staging_brev_launchable so hardware qualification cannot be skipped: either restore required queue flags or validate both Jetson and DGX Spark jobs and their evidence. Ensure full mode rejects false flags and requires successful evidence from both hardware jobs, not only the Launchable job.
🤖 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 @.agents/skills/nemoclaw-maintainer-e2e/SKILL.md:
- Around line 66-68: Update the default controller-selected E2E targets in the
skill so credentialed workflow tests are not selected by an empty-selector
maintainer PR run. Keep the default set limited to credential-free tests, and
require explicit per-job opt-in for any credentialed target with the required
short-lived credentials and isolated trusted boundary.
- Line 199: Update the full-mode validation and workflow inputs around
include_staging_brev_launchable so hardware qualification cannot be skipped:
either restore required queue flags or validate both Jetson and DGX Spark jobs
and their evidence. Ensure full mode rejects false flags and requires successful
evidence from both hardware jobs, not only the Launchable job.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e902e606-cd5a-4332-a44d-784a706282ad
📒 Files selected for processing (1)
.agents/skills/nemoclaw-maintainer-e2e/SKILL.md
2 participants
Summary
Before this change, the release skill required a candidate-bound E2E ledger before a maintainer-confirmed tag. After this change, the 4 PM release remains maintainer-confirmed and locally signed, but E2E runs asynchronously on every push to
mainand never blocks tagging or merging.Changes
Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededdocs/page owns the maintainer-only daily tag procedure. Carlos's merged ci(e2e): run every workflow E2E on main #8477 owns and documents full E2E on everymainpush; historical changelog entries remain unchanged.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm exec vitest run -- test/maintainer-skills-policy.test.ts test/maintainer-e2e-skill.test.ts --testTimeout=30000(33 passed); review fix:npm exec vitest run -- test/maintainer-skills-policy.test.ts --testTimeout=30000(19 passed), plus direct portable command execution againstv0.0.103(exactly one match)npm run docsbuilds without warnings (doc changes only)Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit
Release Process
Documentation