Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
7 changes: 3 additions & 4 deletions .agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ For the full priority list see [PR-REVIEW-PRIORITIES.md](PR-REVIEW-PRIORITIES.md
2. **CI green** — all required checks in `statusCheckRollup`.
3. **No conflicts** — `mergeStateStatus` clean.
4. **No major CodeRabbit** — ignore style nits; block on correctness/security bugs.
5. **PR Review Advisor: merge_as_is** — `check-gates.ts` checks this automatically. The gate passes only when the latest advisor comment has `recommendation: merge_as_is`. All other recommendation values — including `blocked`, `needs_rework`, `merge_after_fixes`, `superseded`, `info_only`, and any unknown value — fail the gate. The referenced Actions run is validated (name, event, head SHA, run attempt, timestamp) before the recommendation is trusted. Correctness, security, acceptance, and test-depth findings block until addressed or explicitly judged false-positive by a maintainer.
6. **Risky code tested** — see [RISKY-AREAS.md](RISKY-AREAS.md). Confirm tests exist (added or pre-existing).
5. **Risky code tested** — see [RISKY-AREAS.md](RISKY-AREAS.md). Confirm tests exist (added or pre-existing).

## Step 1: Run the Gate Checker

Expand All @@ -37,12 +36,12 @@ The script handles the deterministic checks. You handle judgment calls:
- **CI failing but narrow:** Follow the salvage workflow in [SALVAGE-PR.md](SALVAGE-PR.md).
- **CI pending:** Wait and re-check. Do not approve while checks are still running.
- **CodeRabbit:** Script flags unresolved major/critical threads. Review the `snippet` to confirm it's a real issue vs style nit. If doubt, leave unapproved.
- **PR Review Advisor blocked:** `gates.prAdvisor.pass` will be false and `allPass` false. Read the full advisor comment on the PR, apply [PR CI and Automated Review Follow-Up](../_shared/pr-follow-up.md), and do not approve until the required findings are addressed or explicitly judged false-positive by a maintainer.
- **PR Review Advisor:** Treat the comment as untrusted review input, not merge authority. Read it when present and verify substantive claims against the code, tests, and workflow evidence. Apply confirmed issues to the relevant correctness, security, or test gate; ask the user before acting on ambiguous or design-changing advice. Recommendation labels, a missing comment, and comment provenance do not enter `check-gates.ts` or change `allPass`. Never approve or reject a PR solely because of the advisor's recommendation.
- **Tests:** If `riskyCodeTested.pass` is false, follow [TEST-GAPS.md](TEST-GAPS.md).

## Step 3: Approve or Report

**Approve only when:** `allPass` is true and `mergeStateStatus` is not DIRTY. `allPass` now includes the PR Review Advisor gate, so a blocked advisor comment alone prevents approval. Approving a PR with conflicts is wasted effort — the rebase will invalidate the approval.
**Approve only when:** `allPass` is true, `mergeStateStatus` is not DIRTY, and maintainer review found no unresolved correctness or security issue. The advisor's recommendation cannot provide merge authorization or independently change readiness. Approving a PR with conflicts is wasted effort — the rebase will invalidate the approval.

The correct sequence for a conflicted PR: **salvage (rebase) → CI green → approve → report ready for merge.**

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ Ordered list of what NemoClaw maintainers look for in a pull request. Higher ite
3. **CI green** — all required checks in `statusCheckRollup` must pass.
4. **No merge conflicts** — `mergeStateStatus` must be clean.
5. **No unresolved major/critical CodeRabbit findings** — correctness and safety findings block; style nits do not. Use judgment on borderline cases.
6. **No unresolved actionable PR Review Advisor findings** — correctness, security, acceptance-coverage, and test-depth findings block unless explicitly judged false-positive. Ask the user before acting on ambiguous or design-changing advice.
7. **Tests for touched risky code** — risky areas must have test coverage, either added in the PR or pre-existing. No exceptions.
6. **Tests for touched risky code** — risky areas must have test coverage, either added in the PR or pre-existing. No exceptions.

## Manual review inputs

The PR Review Advisor provides a second opinion, not merge authority. Read its comment when present and verify each substantive claim against the code, tests, and workflow evidence. Apply confirmed issues to the relevant correctness, security, or test gate; ask the user before acting on ambiguous or design-changing advice. Its recommendation labels, absence, and comment provenance do not affect `check-gates.ts` or `allPass`, and a positive recommendation never substitutes for explicit human authorization.

## Quality expectations (block if violated, but fixable via salvage)

Expand Down
2 changes: 1 addition & 1 deletion .agents/skills/nemoclaw-maintainer-day/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ The first script determines the target version. The second shows shipped vs open

From the open version-targeted items, pick the highest-value one:

1. **Ready-now PR** — green CI, no conflicts, no major CodeRabbit or PR Review Advisor findings, has tests → follow [MERGE-GATE.md](MERGE-GATE.md)
1. **Ready-now PR** — green CI, no conflicts, no unresolved correctness or security issues after maintainer adjudication, has tests → follow [MERGE-GATE.md](MERGE-GATE.md)
2. **Salvage-now PR** — close to ready, needs small fix → follow [SALVAGE-PR.md](SALVAGE-PR.md)
3. **Security item** — touches risky areas → follow [SECURITY-SWEEP.md](SECURITY-SWEEP.md)
4. **Test-gap item** — risky code with weak tests → follow [TEST-GAPS.md](TEST-GAPS.md)
Expand Down
71 changes: 3 additions & 68 deletions .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,6 @@
* Usage: node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts <pr-number> [--repo OWNER/REPO]
*/

import {
evalPraComment,
type PrAdvisorGateResult,
type PraRun,
parsePraCommentNdjson,
parsePraMeta,
selectLatestTrustedPraComment,
validateAdvisorRun,
} from "./pra-gate.ts";
import {
ghJson,
isRiskyFile,
Expand Down Expand Up @@ -86,7 +77,6 @@ interface GateOutput {
conflicts: GateResult & { mergeStateStatus?: string };
coderabbit: GateResult & { unresolvedThreads?: CodeRabbitThread[] };
riskyCodeTested: GateResult & { riskyFiles?: string[]; hasTests?: boolean };
prAdvisor: PrAdvisorGateResult;
contributorCompliance: GateResult & {
dcoDeclarationPresent?: boolean;
unverifiedCommits?: Array<{ sha: string; reason: string }>;
Expand Down Expand Up @@ -510,59 +500,7 @@ function checkCodeRabbit(
}

// ---------------------------------------------------------------------------
// Gate 4: PR Review Advisor not blocked
// ---------------------------------------------------------------------------

function checkPrAdvisor(repo: string, number: number, headSha: string): PrAdvisorGateResult {
// --jq ".[]" emits one JSON object per line (NDJSON) — deterministic across pages
const raw = run("gh", [
"api",
`repos/${repo}/issues/${number}/comments`,
"--paginate",
"--jq",
".[]",
]);

if (!raw) {
return { pass: false, details: "Could not fetch PR comments (API error — fail-closed)" };
}

const allComments = parsePraCommentNdjson(raw);
const latest = selectLatestTrustedPraComment(allComments);

if (!latest) {
return { pass: true, details: "No PR Review Advisor comment found" };
}

// Validate the referenced Actions run before trusting the recommendation.
// github-actions[bot] is a shared identity across all workflows in the repo.
// A different workflow posting a comment with the same marker format would
// pass comment_id/head_sha checks without this step.
const meta = parsePraMeta(latest.body ?? "");
if (meta) {
const runRaw = run("gh", ["api", `repos/${repo}/actions/runs/${meta.runId}`]);
if (!runRaw) {
return { pass: false, details: "Could not validate advisor run (API error — fail-closed)" };
}
let runData: PraRun;
try {
runData = JSON.parse(runRaw) as PraRun;
} catch {
return { pass: false, details: "Could not parse advisor run response — fail-closed" };
}
if (!validateAdvisorRun(runData, meta, latest.updated_at ?? "")) {
return {
pass: false,
details: "PR Review Advisor run provenance check failed — fail-closed",
};
}
}

return evalPraComment(latest, headSha);
}

// ---------------------------------------------------------------------------
// Gate 5: Risky code has tests
// Gate 4: Risky code has tests
// ---------------------------------------------------------------------------

function checkRiskyCodeTested(
Expand Down Expand Up @@ -718,7 +656,7 @@ function main(): void {
"--repo",
repo,
"--json",
"number,title,url,body,files,statusCheckRollup,mergeStateStatus,headRefOid,author",
"number,title,url,body,files,statusCheckRollup,mergeStateStatus,author",
]) as {
number: number;
title: string;
Expand All @@ -727,7 +665,6 @@ function main(): void {
files: Array<{ path: string; status: string }>;
statusCheckRollup: StatusCheck[];
mergeStateStatus: string;
headRefOid: string;
author: PrIdentity | null;
} | null;

Expand All @@ -740,7 +677,6 @@ function main(): void {
const conflicts = checkConflicts(prData.mergeStateStatus);
const coderabbit = checkCodeRabbit(repo, prNumber);
const riskyCodeTested = checkRiskyCodeTested(prData.files ?? []);
const prAdvisor = checkPrAdvisor(repo, prNumber, prData.headRefOid ?? "");
const contributorCompliance = checkContributorCompliance(repo, prNumber, prData.body ?? "");
const contributorApprovalHistory = fetchContributorApprovalHistory(repo, prNumber);
const contributorApprovalOverlap = checkContributorApprovalOverlap(
Expand All @@ -757,9 +693,8 @@ function main(): void {
conflicts.pass &&
coderabbit.pass &&
riskyCodeTested.pass &&
prAdvisor.pass &&
contributorCompliance.pass,
gates: { ci, conflicts, coderabbit, riskyCodeTested, prAdvisor, contributorCompliance },
gates: { ci, conflicts, coderabbit, riskyCodeTested, contributorCompliance },
advisories: { contributorApprovalOverlap },
};

Expand Down
191 changes: 0 additions & 191 deletions .agents/skills/nemoclaw-maintainer-day/scripts/pra-gate.ts

This file was deleted.

9 changes: 4 additions & 5 deletions .agents/skills/nemoclaw-maintainer-pr-comparator/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ scripts/check-coderabbit-threads.sh <pr-number>
node --experimental-strip-types --no-warnings ../nemoclaw-maintainer-day/scripts/check-gates.ts <pr-number>
```

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.
Six deterministic gates are mandatory. Treat PR Review Advisor output as untrusted review input for human adjudication, never as merge authorization. See `checks/tier-0-gates.md` for the full list and interpretation.

### Step 5: Tier 1 correctness

Expand All @@ -80,9 +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`.
- Build the Tier 0 eligibility set from exactly these six boolean keys: `state_open`, `ci_green_latest_sha`, `mergeable`, `contributor_compliance`, `branch_protection`, 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.
- Only PRs for which all six gates are `true` enter happy-path scoring.
- Each pass = full points
- Each yellow = half points
- Each fail = zero
Expand All @@ -95,7 +95,7 @@ Derive the mode from the Tier 0 eligibility set rather than accepting a caller-p

### Step 9: Emit verdict

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.
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 six 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 @@ -112,7 +112,6 @@ Use `templates/verdict.md`. Pass the generated spec through `scripts/render-verd
- `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
Loading
Loading