Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
37 changes: 33 additions & 4 deletions .agents/skills/nemoclaw-maintainer-find-review-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,39 @@ gh pr view <number> --json number,title,author,createdAt,additions,deletions,rev

## Step 4: Check for superseded PRs

Flag a PR when its body contains one of these phrases:
Run the comparator's canonical detector with the open candidate PR numbers:

- `follow-up to #NNN` / `supersedes #NNN` / `replaces #NNN` / `folds in #NNN`
```bash
../nemoclaw-maintainer-pr-comparator/scripts/parse-supersession.sh <pr-number-1> <pr-number-2> ...
```

It recognizes the comparator parser's case-insensitive statement families:

- `supersed[a-z]*` before `#N`: `supersedes #N` points from the current PR to `#N`; `superseded by #N` points from `#N` to the current PR.
- `replac[a-z]*` before `#N`: `replaces #N` points from the current PR to `#N`; `replaced by #N` points from `#N` to the current PR.
- `clos[a-z]* in favor of` before `#N`: `closes in favor of #N` and `closed in favor of #N` point from `#N` to the current PR.
- `fold[a-z]* in` before `#N`: `folds in #N` points from the current PR to `#N`; `folded into #N` points from `#N` to the current PR.

The bracket expressions describe the parser grammar; they are not literal PR body text.
A `follow-up to #N` statement is a related-PR signal, not a supersession declaration,
unless one of these phrases also appears.

Each supersession 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`.

This skill reports recommendations only.
Do not recommend closing the source PR until another authorized workflow has:

The phrase must name another open candidate PR. It indicates that one PR can include the other.
- completed any required transfer
- verified the updated commits, attribution, and CI
- rerun the comparator and selected the target
- confirmed that the target contains the source's full scope and required contributor attribution
- merged the selected target

## Step 5: Present results

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

- #1416 supersedes/folds in #1392 (shell-quote sandboxName)
Consider closing #1392 if #1416 contains its full scope.
Keep #1392 open while an authorized workflow completes any required transfer, verifies the updated commits, attribution, and CI, and reruns the comparator.
After the updated verdict selects #1416 and #1416 merges, consider closing #1392 only if #1416 contains its full scope and preserves any required contributor attribution.
```

### Clean candidates
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,40 @@ 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:

- 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.
- Use the exact author name and email from the source commit.
Never guess or substitute an attribution identity.

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.

After both checks pass:

- 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 verified source-commit identity.
- 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.

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
29 changes: 25 additions & 4 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,32 @@ 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.
Parse the case-insensitive statement families implemented by `scripts/parse-supersession.sh`:

- `supersed[a-z]*` before `#N`: `supersedes #N` points from the current PR to `#N`; `superseded by #N` points from `#N` to the current PR.
- `replac[a-z]*` before `#N`: `replaces #N` points from the current PR to `#N`; `replaced by #N` points from `#N` to the current PR.
- `clos[a-z]* in favor of` before `#N`: `closes in favor of #N` and `closed in favor of #N` point from `#N` to the current PR.
- `fold[a-z]* in` before `#N`: `folds in #N` points from the current PR to `#N`; `folded into #N` points from `#N` to the current PR.

The bracket expressions describe the parser grammar; they are not literal PR body text.
A `follow-up to #N` statement is a related-PR signal, not a supersession declaration, unless one of these phrases also appears.
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.

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 +122,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 @@ -5,8 +5,10 @@
# Parse PR bodies for supersession references and emit edges:
# <superseder_pr> -> <superseded_pr>
#
# Patterns matched (case-insensitive): "supersedes #N", "replaces #N",
# "closes in favor of #N", "closed in favor of #N", "folds in #N".
# Patterns matched case-insensitively. Active forms such as "supersedes #N",
# "replaces #N", and "folds in #N" point from the current PR to #N. Passive
# forms such as "superseded by #N", "replaced by #N", "closed in favor of
# #N", and "folded into #N" point from #N to the current PR.
#
# Usage: parse-supersession.sh <pr-1> <pr-2> [...] [--repo OWNER/REPO]

Expand Down Expand Up @@ -36,19 +38,34 @@ done
candidates_set=$(printf '%s\n' "${prs[@]}" | sort -u)

edges=()
append_edge() {
local candidate="$1"
local existing

for existing in "${edges[@]}"; do
[ "$existing" = "$candidate" ] && return
done
edges+=("$candidate")
}

reverse_pattern='(supersed[a-z]*[[:space:]]+by|replac[a-z]*[[:space:]]+by|clos[a-z]*[[:space:]]+in[[:space:]]+favor[[:space:]]+of|fold[a-z]*[[:space:]]+into)'
for pr in "${prs[@]}"; do
body=$(gh pr view "$pr" "${repo_args[@]}" --json body --jq .body 2>/dev/null || echo "")
[ -z "$body" ] && continue

# Extract referenced PR numbers from supersession patterns.
while IFS= read -r ref; do
# Extract supersession statements and orient passive forms toward this PR.
while IFS= read -r statement; do
ref="${statement##*#}"
# Only emit edges where the target is also a candidate.
if printf '%s\n' "$candidates_set" | grep -q "^${ref}$"; then
edges+=("$pr -> $ref")
normalized=$(printf '%s' "$statement" | tr '[:upper:]' '[:lower:]')
if [[ $normalized =~ $reverse_pattern ]]; then
append_edge "$ref -> $pr"
else
append_edge "$pr -> $ref"
fi
fi
done < <(printf '%s' "$body" | grep -oiE '(supersed[a-z]*|replac[a-z]*|clos[a-z]* in favor of|fold[a-z]* in)[^#]*#([0-9]+)' \
| grep -oE '#[0-9]+' \
| tr -d '#' \
| sort -u)
done

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,31 @@ 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. Confirm that PR #B contains the contributor's `Signed-off-by:` declaration. Do not add or copy that declaration on the contributor's behalf. If it is absent, leave the winner unset and ask the contributor.
2. Read the exact author name and email from the source commit. Never guess or substitute it. If no usable identity exists, leave the winner unset and ask the contributor.
3. Transfer the test from PR #B before merge. Prefer `git cherry-pick -S -x <source-sha>` so the source contributor remains the Git author.
4. If the work must be combined or reconstructed, add a `Co-authored-by: Name <email>` trailer using the verified source-commit identity.
5. Add `Supersedes #B` to PR #A's body and identify the transferred test.
6. Keep the replacement author's own DCO declaration in PR #A's body.
7. Confirm that every replacement commit appears as `Verified` in GitHub.
8. Verify the updated commits, attribution, and CI, then run the comparator again on the updated SHA.
9. Merge PR #A only if the new verdict selects it.
10. After PR #A merges, 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.
- ... <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).
Loading
Loading