Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .agents/skills/nemoclaw-maintainer-find-review-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ Flag a PR when its body contains one of these phrases:
- `follow-up to #NNN` / `supersedes #NNN` / `replaces #NNN` / `folds in #NNN`

The phrase must name another open candidate PR. It indicates that one PR can include the other.
It records a relationship but does not prove that the target carries the source PR's work.

When the target claims the source PR's full scope, compare their commits and diffs.
If material code, tests, or documentation from another contributor remains in the target,
apply the canonical policy in
`../nemoclaw-maintainer-policies/references/workflow-policy.md`.
Do not recommend closing the source PR until the target contains the required attribution.

## Step 5: Present results

Expand All @@ -92,7 +99,7 @@ For superseded PRs:
### Superseded PRs

- #1416 supersedes/folds in #1392 (shell-quote sandboxName)
Consider closing #1392 if #1416 contains its full scope.
Consider closing #1392 only if #1416 contains its full scope and preserves any required contributor attribution.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
```

### Clean candidates
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,35 @@ The contributor must correct a failure.
Maintainers must reject a PR with an unverified commit or no DCO declaration.
Do not merge, approve, or repair it for the contributor.

## Superseded PR Attribution

A supersession declaration records a relationship between PRs.
It does not prove that one PR covers the other, satisfies contributor requirements, or ranks above another candidate.

When a replacement PR carries material code, tests, or documentation from another contributor's PR:

- Add `Supersedes #<number>` to the replacement PR body and identify the transferred contribution.
- Preserve the source contributor as the Git author when cherry-picking their commit.
- When the replacement combines or reconstructs their work, add a
`Co-authored-by: Name <email>` commit trailer for each contributor whose work remains.
Use the exact author name and email from the source commit. Never guess or substitute an attribution identity.
- Confirm that the source PR already contains that contributor's `Signed-off-by:` declaration.
Never add or copy a DCO declaration on another contributor's behalf.
- Keep the replacement author's own DCO declaration in the replacement PR body.
- Before recommending merge or closure, verify the replacement PR body and commit metadata,
and confirm that every replacement commit appears as `Verified` in GitHub.

If the source commit has no usable author identity, leave the winner unset and ask the contributor to provide machine-readable attribution.
If the source PR has no contributor DCO declaration, leave the winner unset and ask the contributor to add it.
A maintainer must not supply either declaration on the contributor's behalf.

An independent implementation based only on the issue, a reproduction, or public discussion does not require co-authorship.
Still link the related PRs when recommending closure so the decision remains discoverable.
A comment on the superseded PR does not replace attribution in the merged PR history.

This policy does not authorize a merge, close, comment, or other write.
Each operation still requires its own authorization.

## Issue Classification

Native GitHub Issue Type is the canonical issue-kind field:
Expand Down
19 changes: 16 additions & 3 deletions .agents/skills/nemoclaw-maintainer-pr-comparator/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ Tier 3 resolves ties. If no PR passes Tier 0, rank eligible PRs for salvage.
## Repo policy

The defaults use NemoClaw conventions for CODEOWNERS, DCO, CodeRabbit, and `docs/`.
Read the canonical superseded-PR attribution policy in
`../nemoclaw-maintainer-policies/references/workflow-policy.md`.
Edit `repo-policy.md` for another repository.

## Workflow
Expand All @@ -30,7 +32,7 @@ Copy this checklist into your response and check off each step:
PR Comparison Progress:
- [ ] Step 1: Parse issue (body + comments) for acceptance criteria
- [ ] Step 2: Discover candidate PRs in the defined order
- [ ] Step 3: Detect supersession (parse PR bodies)
- [ ] Step 3: Detect supersession and classify transferred work
- [ ] Step 4: Run Tier 0 gates per PR
- [ ] Step 5: Run Tier 1 correctness checks per PR
- [ ] Step 6: Run Tier 2 quality checks per PR
Expand All @@ -57,14 +59,24 @@ scripts/find-candidates.sh <issue-number>

Applies a single default order with stop conditions.

### Step 3: Detect supersession
### Step 3: Detect supersession and transferred work

```bash
scripts/parse-supersession.sh <pr-number-1> <pr-number-2> ...
```

Parse these statements from each PR body: `supersedes #N`, `replaces #N`, `closes in favor of #N`, and `folds in #N`.
Use supersession as the first tiebreaker.
These statements record a relationship.
They do not rank a candidate or prove that its diff contains another contributor's work.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

For each declared or suspected replacement, compare the commits and diffs and classify the relationship:

- `independent`: The PR implements the issue without carrying material code, tests, or documentation from another contributor.
- `transferred`: The PR carries material work from another contributor.
- `unclear`: The available evidence does not establish whether another contributor's work remains.

For `transferred`, apply the canonical superseded-PR attribution policy before setting a winner or recommending that the source PR be closed.
For `unclear`, leave `winner` null and request maintainer judgment.

### Step 4: Tier 0 gates

Expand Down Expand Up @@ -102,6 +114,7 @@ Apply the four model checks in `checks/tier-2-quality.md`.
Compute the mode from the Tier 0 results. Do not accept a mode from the caller.
In happy mode, set `winner` only to an eligible PR and set `closest_to_ready` to null.
Leave `winner` null when the evidence does not support a merge recommendation.
Do not set `winner` for a replacement with transferred work until the required attribution is present and verified.
In degraded mode, set `winner` to null.
Set `closest_to_ready` only to an open PR that passes contributor requirements.
See `tiebreakers.md`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,26 @@ Render the final scorecard with `scripts/render-verdict.py`. Below is the human-
| <criterion 2> | covered | missing |
| <criterion 3> | missing | covered |

### Verdict: MERGE PR #A
### Verdict: No clear winner — see scorecard for recommended action

Reasoning trace:
- PR #B failed Tier 0. Check `<name>` failed on `<short-sha>` after the force-push.
- PR #A scored 14.5. PR #B scored 9.0.
- PR #A misses criterion 3. Cherry-pick PR #B's test at `<file>:<line>` to cover it.
- PR #A misses criterion 3. PR #B contains the test at `<file>:<line>`, so PR #A needs a transfer before it can be selected.

### Suggested action

1. Merge PR #A.
2. Cherry-pick the test from PR #B at `<file>:<line-range>` to cover criterion 3.
3. Close PR #B with a comment that links to #A and records the cherry-pick.
1. Transfer the test from PR #B before merge. Prefer `git cherry-pick -S -x <source-sha>` so the source contributor remains the Git author.
2. If the work must be combined or reconstructed, add a `Co-authored-by: Name <email>` trailer using the exact author identity from the source commit. Never guess it. If no usable identity exists, leave the winner unset and ask the contributor.
3. Add `Supersedes #B` to PR #A's body and identify the transferred test.
4. Verify the updated commits, attribution, and CI, then run the comparator again on the updated SHA.
5. Merge PR #A only if the new verdict selects it. Then close PR #B with a comment that links to #A.

### Reasoning evidence
- CI: all 12 required checks passed on PR #A commit `<short-sha>`. On PR #B commit `<short-sha>`, `test-cli` failed at `<log-line>`.
- Tier 1.1 PR #A: The test at `<file>:<line-range>` asserts on `<output>`. The previous code returned `<wrong-output>`, so the assertion would have failed.
- Tier 1.3 PR #A fail: no test for empty-input edge case despite issue commenter raising it at `issue.comment.4`
- Attribution: PR #A carries `<contribution>` from PR #B. Its body names the source PR, and commit `<sha>` preserves `<contributor>` as Git author or co-author.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
- ... <one entry per judgment> ...
```

Expand Down
16 changes: 11 additions & 5 deletions .agents/skills/nemoclaw-maintainer-pr-comparator/tiebreakers.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@ Eliminate any PR failing Tier 0. Among survivors:
Set the winner when the evidence distinguishes a PR.
Otherwise, leave `winner` null.

### Supersession relationship

A supersession statement records a relationship between candidates.
It does not prove coverage or attribution, and it does not rank a candidate.
Classify transferred work and complete the canonical attribution checks before recommending the replacement PR.

### Tiebreakers (in order)

1. **Supersession.** Prefer a PR whose body states that it supersedes another candidate. See `scripts/parse-supersession.sh`.
2. **Smaller diff.** Prefer the smaller diff when both PRs cover the issue scope.
3. **Better edge-case test coverage.** Compare Tier 1.3 (negative test coverage) outputs.
4. **Most recent activity.** Prefer the PR with the most recent commit.
5. **Lower PR number.** Use the lower PR number if the PRs remain tied.
1. **Smaller diff.** Prefer the smaller diff when both PRs cover the issue scope.
2. **Better edge-case test coverage.** Compare Tier 1.3 (negative test coverage) outputs.
3. **Most recent activity.** Prefer the PR with the most recent commit.
4. **Lower PR number.** Use the lower PR number if the PRs remain tied.

## Degraded mode (no PR passes Tier 0)

Expand Down Expand Up @@ -68,5 +73,6 @@ For each acceptance criterion (from issue body + comments), build a row showing

Use the matrix to find tests or changes that the selected PR does not include.
The verdict can recommend a small transfer from another PR.
Complete the transfer and required attribution, then rerun the comparator before selecting a winner.

Per-criterion winner cells: `covered` (full), `partial` (yellow), `missing` (red).
45 changes: 45 additions & 0 deletions test/maintainer-skills-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,51 @@ describe("maintainer skills follow canonical workflow policy", () => {
);
});

it("requires replacement PRs to preserve transferred contributor attribution", () => {
const policy = read(
".agents/skills/nemoclaw-maintainer-policies/references/workflow-policy.md",
);
const comparator = read(".agents/skills/nemoclaw-maintainer-pr-comparator/SKILL.md");
const tiebreakers = read(".agents/skills/nemoclaw-maintainer-pr-comparator/tiebreakers.md");
const verdict = read(".agents/skills/nemoclaw-maintainer-pr-comparator/templates/verdict.md");
const finder = read(".agents/skills/nemoclaw-maintainer-find-review-pr/SKILL.md");

expect(policy).toContain("Supersedes #<number>");
expect(policy).toContain("Preserve the source contributor as the Git author");
expect(policy).toContain("Co-authored-by: Name <email>");
expect(policy).toContain("Use the exact author name and email from the source commit");
expect(policy).toContain("Never guess or substitute an attribution identity");
expect(policy).toContain("Never add or copy a DCO declaration");
expect(policy).toContain("leave the winner unset and ask the contributor");
expect(policy).toContain("does not require co-authorship");
expect(policy).toContain("does not replace attribution in the merged PR history");

expect(comparator).toContain("../nemoclaw-maintainer-policies/references/workflow-policy.md");
expect(comparator).toContain("They do not rank a candidate");
expect(comparator).toContain("`transferred`");
expect(comparator).toContain("`unclear`");
expect(comparator).toContain("leave `winner` null");

expect(tiebreakers).toContain("it does not rank a candidate");
expect(tiebreakers).not.toContain("**Supersession.**");
expect(tiebreakers).toContain("rerun the comparator before selecting a winner");

expect(verdict).toContain("git cherry-pick -S -x <source-sha>");
expect(verdict).toContain("Co-authored-by: Name <email>");
expect(verdict).toContain("using the exact author identity from the source commit");
expect(verdict).toContain("run the comparator again on the updated SHA");
expect(verdict).toContain("Transfer the test from PR #B before merge");
expect(verdict).toContain("Merge PR #A only if the new verdict selects it");
expect(verdict.indexOf("Transfer the test from PR #B before merge")).toBeLessThan(
verdict.indexOf("Merge PR #A only if the new verdict selects it"),
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

expect(finder).toContain("../nemoclaw-maintainer-policies/references/workflow-policy.md");
expect(finder).toContain(
"Do not recommend closing the source PR until the target contains the required attribution",
);
});

it("keeps PR workflow writes behind their safety checks", () => {
const createPr = read(".agents/skills/nemoclaw-contributor-create-pr/SKILL.md");
const judgment = read(
Expand Down
Loading