From 4a331527a4b67a80ae1db78febfde3e097b87817 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Fri, 7 Aug 2026 20:32:25 -0700 Subject: [PATCH 1/2] fix(skills): repair unresolved PR findings --- .agents/skills/_shared/pr-follow-up.md | 20 ++++++++++++------- .../nemoclaw-contributor-create-pr/SKILL.md | 12 ++++++++++- test/maintainer-skills-policy.test.ts | 14 ++++++++++--- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/.agents/skills/_shared/pr-follow-up.md b/.agents/skills/_shared/pr-follow-up.md index ed0fb3efbc..68ad117b47 100644 --- a/.agents/skills/_shared/pr-follow-up.md +++ b/.agents/skills/_shared/pr-follow-up.md @@ -135,13 +135,19 @@ After editing: 4. If the review reports valid findings, apply them and rerun affected validation. 5. Commit the corrections and review the new `HEAD`. 6. Run one final complete collection for the latest PR commit. Restart the collection if `headRefOid` changes. -7. Classify every new or changed finding. -8. If the collection contains a new actionable finding, do not push. Return to classification and repair, rerun affected validation, commit the corrections, review the new `HEAD`, and repeat the final collection. -9. Remove retained collection evidence and verify its absence. -10. Push once when the receipt identifies the reviewed `HEAD` and no actionable finding remains. -11. Monitor the latest PR commit for new actionable findings. - -Repeat the applicable steps only when the commit under review or latest PR commit produces a new actionable finding. Stop if the user tells you to stop. +7. Classify every finding. +8. After classification, remove retained collection evidence by its exact artifact path or identifier. Verify its absence. +9. Determine which unresolved findings require a change. If the user explicitly defers a non-blocking suggestion, that suggestion does not require a change in this review cycle. +10. If any unresolved finding requires a change, do not push. Complete these actions: + - Repair each unresolved finding that requires a change. + - Rerun affected validation. + - Commit the corrections. + - Run the independent documentation writer review with that commit as the commit under review. + - Repeat the final collection. +11. Push once when the receipt identifies the reviewed `HEAD` and no unresolved finding requires a change. +12. Monitor the latest PR commit for new findings that require a change. + +Repeat the applicable steps whenever an unresolved finding requires a change. Stop if the user tells you to stop. If a push or GitHub query has an access error, follow [Git and GitHub Access Hard Stop](git-github-hard-stop.md). Resolve merge conflicts and dirty-worktree problems in the PR workflow. diff --git a/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md b/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md index c07c9c8a5a..d67db7219c 100644 --- a/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md +++ b/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md @@ -64,7 +64,17 @@ Group valid code-changing findings by root cause. Route each valid code-changing This workflow owns the push gate. After the routed repair returns, follow the numbered steps under `After editing:` in the [Handle results](../_shared/pr-follow-up.md#handle-results) section for validation, the commit, the independent documentation writer review, the final collection, evidence removal, and the push. If that review identifies a valid finding, return the repair to `nemoclaw-contributor-implement-issue`, commit the result, and rerun the review against the new `HEAD`. Push after the independent documentation writer review covers the final `HEAD`, every blocking finding is resolved, and the receipt identifies that commit. -Immediately before pushing, repeat the complete head-stable collection. Do not push while that collection contains an unclassified or actionable finding. Remove retained collection evidence by its exact artifact path or identifier and verify its absence. If the host retained no artifact, record `retained evidence: none`. If the user tells you to stop, stop without pushing. The user may defer only a non-blocking suggestion; record that disposition before pushing. +Immediately before pushing, repeat the complete collection. Confirm that its initial and final `headRefOid` values match. + +Apply these push conditions: + +- Do not push while any finding is unclassified. +- Do not push while any unresolved finding requires a change. +- After classification, remove retained collection evidence by its exact artifact path or identifier. +- Verify that the artifact is absent. +- If the host retained no artifact, record `retained evidence: none`. +- If the user tells you to stop, stop without pushing. +- The user may defer only a non-blocking suggestion. Record that disposition before pushing. ### Hook Evidence diff --git a/test/maintainer-skills-policy.test.ts b/test/maintainer-skills-policy.test.ts index 05616d6ac3..ed5a3cd1c5 100644 --- a/test/maintainer-skills-policy.test.ts +++ b/test/maintainer-skills-policy.test.ts @@ -664,7 +664,7 @@ describe("maintainer skills follow canonical workflow policy", () => { expect(createPr).toContain("Apply one coherent change set"); }); - it("requires PR guidance to review the final commit before push", () => { + it("requires PR guidance to complete the final review cycle before push", () => { const followUp = read(".agents/skills/_shared/pr-follow-up.md"); const writingReview = read(".agents/skills/_shared/documentation-writing-review.md"); const createPr = read(".agents/skills/nemoclaw-contributor-create-pr/SKILL.md"); @@ -676,10 +676,18 @@ describe("maintainer skills follow canonical workflow policy", () => { expect(createPr).toContain("receipt identifies that commit"); expect(followUp).toContain("Run one final complete collection for the latest PR commit"); - - expect(followUp).toContain("If the collection contains a new actionable finding, do not push"); + expect(followUp).toContain( + "After classification, remove retained collection evidence by its exact artifact path or identifier", + ); + expect(followUp).toContain( + "If the user explicitly defers a non-blocking suggestion, that suggestion does not require a change in this review cycle", + ); + expect(followUp).toContain("no unresolved finding requires a change"); expect(followUp).toContain("Deferral does not authorize a push with an unresolved blocking"); expect(createPr).toContain("The user may defer only a non-blocking suggestion"); + expect(createPr).toContain("Do not push while any finding is unclassified"); + expect(createPr).toContain("Do not push while any unresolved finding requires a change"); + expect(createPr).not.toContain("an unclassified or actionable finding"); expect(followUp).toContain("Push once when the receipt identifies the reviewed `HEAD`"); expect(writingReview).toContain("Do not stop after the first blocking finding"); expect(writingReview).toContain("Report all evidence-backed findings in one review result"); From bceca7901c8886459118b0280c2cb01a236de520 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 8 Aug 2026 08:38:12 -0700 Subject: [PATCH 2/2] fix(skills): align contributor push summary --- .agents/skills/nemoclaw-contributor-create-pr/SKILL.md | 2 +- test/maintainer-skills-policy.test.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md b/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md index d67db7219c..50a74a8e10 100644 --- a/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md +++ b/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md @@ -62,7 +62,7 @@ When this workflow pushes an update to an open PR, first follow [Follow Up on PR Group valid code-changing findings by root cause. Route each valid code-changing finding to `nemoclaw-contributor-implement-issue` as part of its root-cause group. That workflow owns the repair, its validation, and its evidence. Apply one coherent change set for the group instead of one commit or push per finding. -This workflow owns the push gate. After the routed repair returns, follow the numbered steps under `After editing:` in the [Handle results](../_shared/pr-follow-up.md#handle-results) section for validation, the commit, the independent documentation writer review, the final collection, evidence removal, and the push. If that review identifies a valid finding, return the repair to `nemoclaw-contributor-implement-issue`, commit the result, and rerun the review against the new `HEAD`. Push after the independent documentation writer review covers the final `HEAD`, every blocking finding is resolved, and the receipt identifies that commit. +This workflow owns the push gate. After the routed repair returns, follow the numbered steps under `After editing:` in the [Handle results](../_shared/pr-follow-up.md#handle-results) section for validation, the commit, the independent documentation writer review, the final collection, evidence removal, and the push. If that review identifies a valid finding, return the repair to `nemoclaw-contributor-implement-issue`, commit the result, and rerun the review against the new `HEAD`. Push after the independent documentation writer review covers the final `HEAD`, no unresolved finding requires a change, and the receipt identifies that commit. Immediately before pushing, repeat the complete collection. Confirm that its initial and final `headRefOid` values match. diff --git a/test/maintainer-skills-policy.test.ts b/test/maintainer-skills-policy.test.ts index ed5a3cd1c5..1cf7cd3158 100644 --- a/test/maintainer-skills-policy.test.ts +++ b/test/maintainer-skills-policy.test.ts @@ -688,6 +688,8 @@ describe("maintainer skills follow canonical workflow policy", () => { expect(createPr).toContain("Do not push while any finding is unclassified"); expect(createPr).toContain("Do not push while any unresolved finding requires a change"); expect(createPr).not.toContain("an unclassified or actionable finding"); + + expect(createPr).not.toContain("every blocking finding is resolved"); expect(followUp).toContain("Push once when the receipt identifies the reviewed `HEAD`"); expect(writingReview).toContain("Do not stop after the first blocking finding"); expect(writingReview).toContain("Report all evidence-backed findings in one review result");