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
11 changes: 8 additions & 3 deletions .agents/skills/nemoclaw-maintainer-pr-comparator/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@ Parses each PR body for `supersedes #N`, `replaces #N`, `closes in favor of #N`,
```bash
scripts/collect-gates.sh <pr-number>
scripts/check-coderabbit-threads.sh <pr-number>
node --experimental-strip-types --no-warnings ../nemoclaw-maintainer-day/scripts/check-gates.ts <pr-number>
```

Six gates, all mandatory. See `checks/tier-0-gates.md` for the full list and interpretation.
Seven gates, all mandatory. Use the shared gate result's PR Advisor recommendation for the Advisor gate: it is `true` only when the trusted exact-head result both passes and reports `merge_as_is`; a missing recommendation is `false`. See `checks/tier-0-gates.md` for the full list and interpretation.

### Step 5: Tier 1 correctness

Expand All @@ -79,6 +80,9 @@ Three checks, all LLM judgments. See `checks/tier-2-quality.md`.

### Step 7: Weighted score

- Build the Tier 0 eligibility set from exactly these seven boolean keys: `state_open`, `ci_green_latest_sha`, `mergeable`, `contributor_compliance`, `branch_protection`, `pr_advisor_merge_as_is`, and `coderabbit_threads_resolved`.
- Stop if any candidate omits a key, adds an unknown key, or supplies a non-boolean value.
- Only PRs for which all seven gates are `true` enter happy-path scoring.
- Each pass = full points
- Each yellow = half points
- Each fail = zero
Expand All @@ -87,11 +91,11 @@ Three checks, all LLM judgments. See `checks/tier-2-quality.md`.

### Step 8: Tier 3 ranking

Branch on whether any PR passes all Tier 0 gates. See `tiebreakers.md` for happy-path tiebreakers, degraded-mode distance-to-ready ranking, and the behavior-coverage matrix.
Derive the mode from the Tier 0 eligibility set rather than accepting a caller-provided mode. In happy mode, set `winner` only to a PR in that set and leave `closest_to_ready` null. In degraded mode, leave `winner` null and use `closest_to_ready` only for an open, contributor-compliant salvage candidate. See `tiebreakers.md` for happy-path tiebreakers, degraded-mode distance-to-ready ranking, and the behavior-coverage matrix.

### Step 9: Emit verdict

Use `templates/verdict.md`. Every judgment must carry evidence (file:line refs, diff snippets), reasoning chain, and the score it contributed.
Use `templates/verdict.md`. Pass the generated spec through `scripts/render-verdict.py`; do not render or recommend a merge if it exits nonzero. The renderer independently validates the seven gates, recomputes eligibility and mode, and rejects a `winner` outside the eligible set. Every judgment must carry evidence (file:line refs, diff snippets), reasoning chain, and the score it contributed.

## Reference files

Expand All @@ -108,6 +112,7 @@ Use `templates/verdict.md`. Every judgment must carry evidence (file:line refs,
- `scripts/find-candidates.sh` — PR discovery
- `scripts/collect-gates.sh` — Tier 0 gate evaluation
- `scripts/check-coderabbit-threads.sh` — GraphQL thread resolution
- `../nemoclaw-maintainer-day/scripts/check-gates.ts` — trusted PR Review Advisor result
- `scripts/parse-supersession.sh` — body parsing for supersession refs
- `scripts/render-verdict.py` — verdict scorecard renderer

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

# Tier 0 — Plumbing Gates

Mandatory prerequisites. Any gate failure means the PR cannot be merged in its current state. Six gates total. Run `scripts/collect-gates.sh <pr>` to evaluate gates 1-5 mechanically; run `scripts/check-coderabbit-threads.sh <pr>` for gate 6.
Mandatory prerequisites. Any gate failure means the PR cannot be merged in its current state. Seven gates total. Run `scripts/collect-gates.sh <pr>` to evaluate gates 1-5 mechanically, use the shared maintainer-day gate result for gate 6, and run `scripts/check-coderabbit-threads.sh <pr>` for gate 7.

## Contents

Expand All @@ -12,7 +12,8 @@ Mandatory prerequisites. Any gate failure means the PR cannot be merged in its c
- Gate 3: Mergeable, no conflicts
- Gate 4: Contributor compliance satisfied
- Gate 5: Branch protection satisfied
- Gate 6: Automated reviewer threads resolved
- Gate 6: PR Review Advisor recommends merge as-is
- Gate 7: Automated reviewer threads resolved

## Gate 1: PR state OPEN

Expand Down Expand Up @@ -52,7 +53,13 @@ The PR body must include a valid contributor `Signed-off-by:` declaration, and e

**Why defer:** Branch protection rules are the source of truth. Re-implementing the check in the skill would drift from repo policy. If your repo doesn't enforce CODEOWNERS via branch protection, set `codeowners_enforced_via_branch_protection: false` in `repo-policy.md` and add explicit team checks.

## Gate 6: Automated reviewer threads resolved
## Gate 6: PR Review Advisor recommends merge as-is

The trusted, exact-head PR Review Advisor result must report `recommendation: merge_as_is`. Use `.gates.prAdvisor` from `nemoclaw-maintainer-day/scripts/check-gates.ts`, and set `pr_advisor_merge_as_is` to `true` only when both `pass` is `true` and `recommendation` is exactly `merge_as_is`.

**Fail closed:** Missing, skipped, stale, malformed, untrusted, or non-`merge_as_is` Advisor results fail this gate. A successful CodeRabbit review or human approval does not substitute for the Advisor result.

## Gate 7: Automated reviewer threads resolved

All threads created by automated reviewers (e.g., CodeRabbit) must be in `resolved: true` state. **Zero unresolved threads is the bar.**

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
#
# Collect Tier 0 gate state for a PR and emit JSON for downstream scoring.
# Covers gates 1-5 (state, CI on latest SHA, mergeable, contributor compliance,
# branch protection). Gate 6 (CodeRabbit threads) is handled by
# branch protection). Gate 6 (PR Review Advisor) comes from the shared
# maintainer-day gate result, and gate 7 (CodeRabbit threads) is handled by
# check-coderabbit-threads.sh.
#
# Usage: collect-gates.sh <pr-number> [--repo OWNER/REPO]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
"supersession_edges": [{"superseder": 2851, "superseded": 2693}],
"tiebreaker_fired": "smaller_diff",
"winner": 2851,
"mode": "happy" // or "degraded"
"closest_to_ready": null,
"mode": "happy" // optional assertion; derived from Tier 0 gates
}

Usage:
Expand All @@ -45,6 +46,94 @@
# Tier 2 weight per check (each pass = 1 point, yellow = 0.5, fail = 0).
TIER_2_WEIGHT = 1.0

TIER_0_GATES = (
("state_open", "State open"),
("ci_green_latest_sha", "CI green on latest SHA"),
("mergeable", "Mergeable"),
("contributor_compliance", "Contributor compliance"),
("branch_protection", "Branch protection"),
("pr_advisor_merge_as_is", "PR Review Advisor: merge as-is"),
("coderabbit_threads_resolved", "Automated-review threads resolved"),
)
TIER_0_KEYS = tuple(key for key, _label in TIER_0_GATES)
INVALID_SPEC_EXIT = 64


class SpecValidationError(ValueError):
"""Raised when a verdict spec cannot produce a safe recommendation."""


def validate_spec(spec: Any) -> tuple[str, int | None]:
"""Validate untrusted renderer input and derive the verdict mode."""
if not isinstance(spec, dict):
raise SpecValidationError("top-level value must be an object")

prs = spec.get("prs")
if not isinstance(prs, list) or not prs:
raise SpecValidationError("prs must be a non-empty array")

pr_numbers: set[int] = set()
eligible_numbers: set[int] = set()
salvageable_numbers: set[int] = set()
for index, pr in enumerate(prs):
if not isinstance(pr, dict):
raise SpecValidationError(f"prs[{index}] must be an object")
number = pr.get("number")
if type(number) is not int:
raise SpecValidationError(f"prs[{index}].number must be an integer")
if number in pr_numbers:
raise SpecValidationError(f"duplicate PR number: {number}")
pr_numbers.add(number)

gates = pr.get("tier_0")
if not isinstance(gates, dict):
raise SpecValidationError(f"PR #{number} tier_0 must be an object")
missing = [key for key in TIER_0_KEYS if key not in gates]
extra = sorted(set(gates) - set(TIER_0_KEYS))
if missing:
raise SpecValidationError(
f"PR #{number} tier_0 is missing required gates: {', '.join(missing)}"
)
if extra:
raise SpecValidationError(f"PR #{number} tier_0 has unknown gates: {', '.join(extra)}")
non_boolean = [key for key in TIER_0_KEYS if type(gates[key]) is not bool]
if non_boolean:
raise SpecValidationError(
f"PR #{number} tier_0 gates must be boolean: {', '.join(non_boolean)}"
)

if all(gates[key] for key in TIER_0_KEYS):
eligible_numbers.add(number)
if gates["state_open"] and gates["contributor_compliance"]:
salvageable_numbers.add(number)

mode = "happy" if eligible_numbers else "degraded"
winner = spec.get("winner")
if winner is not None:
if type(winner) is not int or winner not in pr_numbers:
raise SpecValidationError("winner must reference a candidate PR number")
if winner not in eligible_numbers:
raise SpecValidationError(f"winner PR #{winner} did not pass every Tier 0 gate")

closest_to_ready = spec.get("closest_to_ready")
if closest_to_ready is not None:
if type(closest_to_ready) is not int or closest_to_ready not in pr_numbers:
raise SpecValidationError("closest_to_ready must reference a candidate PR number")
if mode != "degraded":
raise SpecValidationError("closest_to_ready is only valid in degraded mode")
if closest_to_ready not in salvageable_numbers:
raise SpecValidationError(
f"closest_to_ready PR #{closest_to_ready} must be open and contributor-compliant"
)

supplied_mode = spec.get("mode")
if supplied_mode is not None and supplied_mode != mode:
raise SpecValidationError(
f"supplied mode {supplied_mode!r} contradicts derived mode {mode!r}"
)

return mode, closest_to_ready


def status_emoji(status: str) -> str:
"""Map a check status to a short label. Matches templates/verdict.md."""
Expand Down Expand Up @@ -77,18 +166,10 @@ def render_scorecard(prs: list[dict[str, Any]]) -> str:

# Tier 0
rows.append(["**Tier 0 — gates**"] + [""] * len(prs))
tier_0_keys = [
"state_open",
"ci_green_latest_sha",
"mergeable",
"branch_protection",
"coderabbit_threads_resolved",
]
for key in tier_0_keys:
label = key.replace("_", " ").capitalize()
for key, label in TIER_0_GATES:
row = [label]
for pr in prs:
row.append(status_emoji(pr.get("tier_0", {}).get(key, "fail")))
row.append(status_emoji(pr["tier_0"][key]))
rows.append(row)

# Tier 1
Expand Down Expand Up @@ -176,13 +257,18 @@ def main() -> int:
spec = json.load(sys.stdin)
except json.JSONDecodeError as e:
print(f"Invalid JSON spec on stdin: {e}", file=sys.stderr)
return 64
return INVALID_SPEC_EXIT

try:
mode, closest_to_ready = validate_spec(spec)
except SpecValidationError as e:
print(f"Invalid verdict spec: {e}", file=sys.stderr)
return INVALID_SPEC_EXIT

issue = spec["issue"]
criteria = spec.get("criteria", [])
prs = spec.get("prs", [])
winner = spec.get("winner")
mode = spec.get("mode", "happy")
tiebreaker = spec.get("tiebreaker_fired")
supersession = spec.get("supersession_edges", [])

Expand All @@ -209,10 +295,10 @@ def main() -> int:
print(f"\n### Verdict: MERGE PR #{winner}\n")
else:
print("\n### Verdict: Neither mergeable yet\n")
if winner is not None:
print(f"PR #{winner} is closer to ready.\n")
if closest_to_ready is not None:
print(f"PR #{closest_to_ready} is closer to ready.\n")
else:
print("No PR is meaningfully closer; both need substantial salvage.\n")
print("No open, contributor-compliant PR is eligible for salvage.\n")

print("Reasoning trace:")
if supersession:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ Render the final scorecard with `scripts/render-verdict.py`. Below is the human-
| State OPEN | pass | pass |
| CI green (latest SHA) | pass | fail (stale) |
| Mergeable | pass | pass |
| Contributor compliance | pass | pass |
| Branch protection | pass | pass |
| CodeRabbit threads | pass | yellow (2 unresolved) |
| PR Review Advisor: merge as-is | pass | fail (changes requested) |
| Automated-review threads resolved | pass | fail (2 unresolved) |
| **Tier 1 — correctness** | | |
| Test exercises bug path | pass | pass |
| Comment-as-spec coverage | pass | yellow (misses ask 3) |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Final decision logic. Two paths: happy mode when at least one PR passes all Tier

Eliminate any PR failing Tier 0. Among survivors:

- Set `winner` only to a survivor and leave `closest_to_ready` null.

1. Compute weighted score across Tiers 1-2.
2. Build the **behavior-coverage matrix** (see below). If matrix has a clear winner per criterion, that wins.
3. Apply tiebreakers in order. First tiebreaker that distinguishes the PRs picks the winner.
Expand Down Expand Up @@ -43,6 +45,8 @@ Don't give up — pick the closest-to-ready and recommend salvage steps.
- Tie → fewer trivial failures wins
- Tie → higher Tier 1-2 weighted score wins (correctness beneath the broken plumbing)
3. Output:
- Leave `winner` null; it is reserved for a fully eligible merge recommendation
- Set `closest_to_ready` only to an open PR that passes contributor compliance; leave it null for a rejection-only verdict
- Per-PR Tier 0 failure list
- Per-PR Tier 1-2 scorecard (so the winner has objective merit beneath the gates)
- Verdict: "Neither mergeable yet. PR A is closer — fix [substantive list]. PR B has [issues]."
Expand Down
Loading