feat(skills): add continuous e2e failure-fixing loop - #8592
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
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. |
|
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:
📝 WalkthroughWalkthroughAdds a persistent NemoClaw maintainer skill for automatic ChangesAutomatic E2E failure remediation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MainE2ERuns
participant MaintainerSkill
participant DraftFixPR
participant ReviewAndMerge
MainE2ERuns->>MaintainerSkill: provide changed failure runs
MaintainerSkill->>DraftFixPR: claim one root cause with evidence
DraftFixPR->>ReviewAndMerge: request exact-head review and merge validation
ReviewAndMerge->>MainE2ERuns: verify later main-run evidence
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 80af7ab in the TypeScript / code-coverage/cliThe overall coverage in commit 80af7ab in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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-fix-e2e-failures/references/queue-and-ownership.md:
- Around line 53-60: Update the duplicate-claim search command in the
queue-and-ownership guidance so every gh option appears before the `--`
separator, and pass the validated run ID or query token after `--` as the
positional search term. Preserve the existing token validation and search
behavior while preventing leading-hyphen tokens from being interpreted as flags.
In
@.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/review-and-merge.md:
- Around line 92-97: Add the trusted-checkout hard stop immediately before the
check-gates.ts command in the review-and-merge procedure: compare staged,
unstaged, and untracked execution-surface changes plus transitive code against
refreshed origin/main, and require explicit approval before running any changed
surface. Alternatively, invoke a separately reviewed trusted copy of
check-gates.ts, while preserving the existing gate-check command and its timing
before approval.
- Around line 105-119: Update the approval flow around the reviewer approval and
final merge gate: treat the pre-approval gate checks as preliminary, then after
the approval write re-read the PR state and rerun both the gate checker and
effective GitHub-rules check for the current head and base. Require these
post-approval results to pass before merging, restarting the gate if any
relevant state changed.
In @.agents/skills/nemoclaw-maintainer-fix-e2e-failures/SKILL.md:
- Line 64: Align the diagnostic-evidence policy in both
`.agents/skills/nemoclaw-maintainer-fix-e2e-failures/SKILL.md` lines 64-64 and
`.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/queue-and-ownership.md`
lines 84-84: explicitly choose whether an infeasible diagnostic or regression
test blocks the fix, then state the same rule at both sites.
🪄 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: f2d58841-ec14-4bb8-9781-92980ebab8ce
📒 Files selected for processing (9)
.agents/skills/nemoclaw-maintainer-fix-e2e-failures/SKILL.md.agents/skills/nemoclaw-maintainer-fix-e2e-failures/agents/openai.yaml.agents/skills/nemoclaw-maintainer-fix-e2e-failures/evals/evals.json.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/continuity-handoff.md.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/queue-and-ownership.md.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/review-and-merge.md.agents/skills/nemoclaw-skills-guide/SKILL.mdtest/maintainer-skills-policy.test.tstest/pr-limit-policy.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
1 additional E2E selection from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 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: None 1 warning · 0 suggestionsWarningsWarnings do not block.
|
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
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-fix-e2e-failures/references/review-and-merge.md:
- Line 109: Update the final merge write in the post-approval flow to bind it to
the previously captured head SHA using the selected merge API’s head
precondition, such as match-head-commit, sha, or expectedHeadOid. If that
precondition fails, restart the final gate and do not retry through another
merge method.
🪄 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: c2ec0c4a-962c-477e-989e-7a439321ac38
📒 Files selected for processing (4)
.agents/skills/nemoclaw-maintainer-fix-e2e-failures/SKILL.md.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/queue-and-ownership.md.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/review-and-merge.mdtest/maintainer-skills-policy.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- test/maintainer-skills-policy.test.ts
- .agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/queue-and-ownership.md
- .agents/skills/nemoclaw-maintainer-fix-e2e-failures/SKILL.md
There was a problem hiding this comment.
Review follow-up
The author resolved both requests from this review:
- Commit
d3aaf9fe3prevents GitHub from merging a commit that the reviewer did not evaluate. The merge request includes the reviewed commit SHA and restarts the final checks if the PR branch changes. - Commit
0e56769a1prohibits this workflow from changing any release, tag, or release artifact. It also routes release work to the release workflow.
The documentation writer receipt now identifies commit 0e56769a1 and lists the reviewed documentation paths.
I dismissed this changes-requested review because the author resolved its findings. Approval still requires successful required CI, or the repository-required maintainer waiver for a failed check.
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Commit d3aaf9f binds the merge write to the reviewed head. Commit 0e56769 generalizes the release boundary and refreshes the documentation writer receipt with the reviewed paths. Both findings from this review are resolved. Required CI must still pass, or the PR must record the repository-required maintainer waiver, before approval.
There was a problem hiding this comment.
remove this entire test, as it's a source-shape bundle of assertions and provides almost no verification value
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
-
.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/review-and-merge.md:109defines security-critical workflow approval, exact-commit review, ambiguous-write recovery, and merge gates only as prose..agents/skills/nemoclaw-maintainer-fix-e2e-failures/evals/evals.json:1contains declarative scenarios, but no checked-in runner or test executes them, and this PR deletestest/maintainer-skills-policy.test.ts. A later regression could permit an immediate retry after an ambiguous write, unsafe fork-workflow approval, self-approval, or a stale-commit merge without CI detecting it. Add executable scenario validation that invokes the skill evaluation path with controlled GitHub states and proves these actions remain denied. Do not replace the deleted bundle with source-shape assertions; provide failing-before and passing-after regression evidence in a Vitest project or another required executable gate. -
.agents/skills/nemoclaw-maintainer-fix-e2e-failures/SKILL.md:91and.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/continuity-handoff.md:39define obsolete-work closure and post-merge verification, but they do not define containment or rollback when the merged fix preserves the original failure or introduces a new regression. The persistent loop can continue merging fixes while the harmful merge remains onmain, with no named rollback owner, authorization boundary, or stop condition. Define the post-merge failure state and next actor. State whether the loop must open a guarded revert PR, route rollback to another maintainer workflow, or stop merge writes pending explicit authorization. Add executable scenario evidence for this path.
3 participants
Summary
Adds a persistent maintainer skill for fixing failures from automatic
mainE2E runs. The skill replaces repeated one-off prompts with one root-cause queue, shared ownership, gated peer review, and continuous monitoring.Changes
nemoclaw-maintainer-fix-e2e-failuresworkflow and its Codex interface metadata.test/maintainer-skills-policy.test.ts, the source-shape policy assertion suite, per maintainer review.Type of Change
Quality Gates
Documentation Writer Review
docs-updated.agents/skills/nemoclaw-maintainer-fix-e2e-failures/SKILL.md,.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/continuity-handoff.md,.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/queue-and-ownership.md,.agents/skills/nemoclaw-maintainer-fix-e2e-failures/references/review-and-merge.md, and.agents/skills/nemoclaw-skills-guide/SKILL.md. The latest change deletes only the pre-existing source-shape test file at maintainer request. It does not change skill behavior or require public documentation.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 unavailablenpx vitest run --project integration test/skills-frontmatter.test.ts test/pr-limit-policy.test.ts— 37 tests passed;npm run test:projects:check— passed with exact project membership.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit