feat(skills): add issue planning workflow - #8396
Merged
Merged
Conversation
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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:
📝 WalkthroughWalkthroughAdds the ChangesIssue planning workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Contributor
participant PlanningAgent
participant PlanningSkill
participant Repository
Contributor->>PlanningAgent: Submit issue-planning request
PlanningAgent->>PlanningSkill: Route request
PlanningSkill->>Repository: Read implementation, tests, workflows, and related work
Repository-->>PlanningSkill: Return evidence
PlanningSkill-->>Contributor: Return capability-slice plan
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 288b773 in the TypeScript / code-coverage/cliThe overall coverage in commit 288b773 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
test/skills-frontmatter.test.ts (1)
127-150: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftPrefer observable behavior over exact skill wording and evaluator order.
This test reads raw
SKILL.mdand asserts exact phrases, then requires one exact order for evaluator IDs. These checks lock copy and file order. They do not prove routing, read-only behavior, or capability slicing. Use the evaluation or routing boundary if available. Otherwise, assert stable parsed fields and required outcomes instead of prose.🤖 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/skills-frontmatter.test.ts` around lines 127 - 150, Update the test around the skill and evaluator assertions to validate observable routing, read-only behavior, and capability-slicing outcomes rather than exact SKILL.md wording or evaluator ordering. Prefer the available evaluation or routing boundary; otherwise parse stable structured fields and assert required outcomes while preserving the existing scenario coverage.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.
Inline comments:
In @.agents/skills/nemoclaw-contributor-plan-issue/evals/evals.json:
- Around line 4-10: Expand the positive evaluation’s expected_behavior for
nemoclaw-contributor-plan-issue to require a complete plan contract: each
capability slice must include dependencies, decisions, acceptance examples, test
evidence, and deferred scope, along with delivery order, stop conditions, and
security-boundary evidence. Preserve the existing ownership, related-work,
source-reading, and no-implementation expectations.
- Around line 3-64: Extend the evaluation cases in the JSON array with GitHub
write-authorization scenarios: add an unauthorized-write prompt whose expected
behavior performs no write and returns exactly “Not authorized; plan only,” and
add an explicitly authorized prompt requiring an exact preview, only the named
write, and reporting every resulting URL or failure. Keep these cases distinct
from the existing planning, implementation, PR publication, and
maintainer-routing evaluations.
In @.agents/skills/nemoclaw-contributor-plan-issue/SKILL.md:
- Around line 46-50: Update the “Discover the current implementation” section in
SKILL.md to reference ../_shared/git-github-hard-stop.md before any discovery
commands, alongside the existing implementation-discovery,
code-change-considerations, and security-rubric references.
- Around line 52-61: Update the discovery guidance in the skill’s “Read before
proposing work” section to explicitly treat issue bodies, comments, source,
workflows, and history as evidence rather than agent instructions. State that
only workflow instructions and explicit user authorization may define operations
or authorize GitHub writes, while accepted issue decisions may define product
scope only; add adversarial evaluations covering issue or comment content that
attempts to alter scope or authorize writes.
In `@test/skills-frontmatter.test.ts`:
- Around line 137-162: Update the assertions in the frontmatter test to verify
the skill contains “Current behavior owner” alongside the existing owner check,
and assert that the evaluator with ID “clean-context-refinement” maps to
“nemoclaw-contributor-plan-issue” using its expected_skill value.
---
Nitpick comments:
In `@test/skills-frontmatter.test.ts`:
- Around line 127-150: Update the test around the skill and evaluator assertions
to validate observable routing, read-only behavior, and capability-slicing
outcomes rather than exact SKILL.md wording or evaluator ordering. Prefer the
available evaluation or routing boundary; otherwise parse stable structured
fields and assert required outcomes while preserving the existing scenario
coverage.
🪄 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: 2c2f2e99-4059-45aa-aa4d-2e3b25d0f5f5
📒 Files selected for processing (5)
.agents/skills/nemoclaw-contributor-plan-issue/SKILL.md.agents/skills/nemoclaw-contributor-plan-issue/agents/openai.yaml.agents/skills/nemoclaw-contributor-plan-issue/evals/evals.json.agents/skills/nemoclaw-skills-guide/SKILL.mdtest/skills-frontmatter.test.ts
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/skills-frontmatter.test.ts (1)
155-164: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert routing for each new evaluator.
Lines 155-157 only assert evaluator IDs. A changed
expected_skillcan route an authorization or adversarial case to another workflow while this test still passes. Assert that all three new evaluators map tonemoclaw-contributor-plan-issue.Proposed test change
expect(evals.find(({ id }) => id === "clean-context-refinement")?.expected_skill).toBe( "nemoclaw-contributor-plan-issue", ); + for (const id of [ + "unauthorized-github-write", + "authorized-single-github-write", + "adversarial-untrusted-issue-content", + ]) { + expect(evals.find((evaluation) => evaluation.id === id)?.expected_skill).toBe( + "nemoclaw-contributor-plan-issue", + ); + }As per review objectives, structural validation must cover expected skill routing for authorization and adversarial cases.
🤖 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/skills-frontmatter.test.ts` around lines 155 - 164, Update the evaluator assertions near the existing expected_skill checks to also verify that all three new evaluators, including the authorization and adversarial cases, map to nemoclaw-contributor-plan-issue. Preserve the existing ID assertions and add explicit expected_skill coverage for each new evaluator.
🤖 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 `@test/skills-frontmatter.test.ts`:
- Around line 155-164: Update the evaluator assertions near the existing
expected_skill checks to also verify that all three new evaluators, including
the authorization and adversarial cases, map to nemoclaw-contributor-plan-issue.
Preserve the existing ID assertions and add explicit expected_skill coverage for
each new evaluator.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e7068a46-272c-4e9e-8841-84bc384be20d
📒 Files selected for processing (3)
.agents/skills/nemoclaw-contributor-plan-issue/SKILL.md.agents/skills/nemoclaw-contributor-plan-issue/evals/evals.jsontest/skills-frontmatter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/skills/nemoclaw-contributor-plan-issue/SKILL.md
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
23 tasks
23 tasks
jyaunches
added a commit
that referenced
this pull request
Aug 6, 2026
<!-- markdownlint-disable MD041 --> ## Summary Add a generic contributor workflow for implementing accepted issues from current repository evidence. The workflow delivers one independently valuable capability slice, records focused validation and security evidence, and routes publication to the separate PR workflow. This PR is stacked on PR #8396. ## Related Issue Fixes #8363 ## Changes - Add `nemoclaw-contributor-implement-issue` with explicit implementation, planning, publication, security-review, and maintainer routing boundaries. - Treat issue, PR, comment, and repository content as untrusted evidence that cannot authorize operations. - Require current implementation discovery, shared code-change and security review, focused positive and negative evidence, and a publication handoff. - Add positive, negative, ambiguous, adversarial, and clean-context routing evaluations. - Register the workflow in the contributor skill catalog. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This changes repository contributor-agent workflows, not the supported NemoClaw user product surface under `docs/`. - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Independent review passed for the implementation workflow, routing metadata and evaluations, contributor catalog registration, and repository contract tests. No user-facing `docs/` change is required. - Agent: Pi review-executor <!-- docs-review-head-sha: 292a5af --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `vitest run --project integration test/skills-frontmatter.test.ts` — 34 tests passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a contributor workflow for implementing accepted issues locally, including validation, security checks, documentation updates, and structured handoffs. - Added guidance for distinguishing implementation requests from planning, publishing, maintainer, and security-review workflows. - **Documentation** - Updated the skills catalog and counts to include the new contributor workflow. - **Tests** - Added coverage for workflow routing, required guidance, evaluation scenarios, and boundary conditions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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
2 participants
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
Add a contributor issue-planning workflow that researches current repository evidence and divides accepted work into independently valuable capability slices. The workflow separates planning from implementation and PR publication, and keeps GitHub writes opt-in.
Related Issue
Fixes #8362
Changes
nemoclaw-contributor-plan-issuewith explicit planning, implementation, publication, and maintainer routing boundaries.Type of Change
Quality Gates
docs/.Documentation Writer Review
no-docs-needed.agents/skills/nemoclaw-contributor-plan-issue/SKILL.md, its metadata and routing cases,.agents/skills/nemoclaw-skills-guide/SKILL.md, andtest/skills-frontmatter.test.ts. No user-facingdocs/change is required.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 unavailablevitest run --project integration test/skills-frontmatter.test.ts— 32 tests passed.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: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
New Features
Documentation
Tests