fix(skills): require approved workflows before declaring CI green

[cv]

Summary

The gate checker and triage scripts treated "all present checks green" as passing, even when only 2 of ~9 checks existed. This caused premature approvals on fork PRs where workflows hadn't been triggered yet.

Root cause

Fork PRs from first-time contributors need a maintainer to click "Approve and run" before pull_request workflows execute. Until then, only pull_request_target checks (check-pr-limit) and external bots (CodeRabbit) appear in statusCheckRollup. The scripts saw 2/2 green and reported CI as passing.

A secondary bug: GitHub's statusCheckRollup returns two shapes — CheckRun (name/status/conclusion) and StatusContext (context/state). The scripts only read CheckRun fields, so CodeRabbit (a StatusContext) was always treated as "pending" even when state was SUCCESS.

Changes

• check-gates.ts: Add REQUIRED_CHECK_NAMES (checks, commit-lint, dco-check) validation. Add StatusCheck union type to correctly handle both CheckRun and StatusContext shapes. CI gate now fails with "required check(s) not found — workflows may need approval" when expected checks are absent.
• triage.ts: Add same required-check validation so triage does not score unapproved-workflow PRs as review-ready.
• MERGE-GATE.md: Add "Missing required checks" as first bullet in Step 2 interpretation guidance.

Before / After

PR scenario	Before	After
Fork PR, workflows not approved (2 checks)	"All 2 checks green" ✅	"3 required check(s) not found — workflows may need approval" ❌
Fork PR, workflows running, dco-check failing	"1 pending" (CodeRabbit misread)	"3 failing check(s): dco-check: FAILURE, ..." ❌
Internal PR, all 12 checks green	"1 pending" (CodeRabbit misread)	"All 12 checks green" ✅

Test plan

• Verified against PR fix(security): gate GitHub access behind an opt-in policy preset (#1583) #1660 (fork, workflows not approved) — correctly reports missing checks
• Verified against PR fix(security): drop POST allow rule from huggingface preset (#1432) #1663 (fork, workflows approved, dco-check failing) — correctly reports failures
• Verified against PR ci(ts-migration): guard migrated legacy paths #1683 (internal, all green) — correctly reports all 12 green
• Triage script correctly classifies fix(security): gate GitHub access behind an opt-in policy preset (#1583) #1660 as salvage-now with failing-checks reason instead of review-ready

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

• Chores
  • Enhanced merge gate to require specific CI checks be present and completed before a PR can be approved; missing required checks will block approval until workflows finish and validation is re-run.
  • Improved CI evaluation to better distinguish pending vs failed states across different check types.
  • PRs missing required check contexts are now classified as not-green.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 58a526e9-9d5e-4a34-98ac-da3eff6471f5

📥 Commits

Reviewing files that changed from the base of the PR and between 75bceb4 and 78c41c5.

📒 Files selected for processing (3)

• .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts
• .agents/skills/nemoclaw-maintainer-day/scripts/shared.ts
• .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts

✅ Files skipped from review due to trivial changes (1)

• .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts

---

📝 Walkthrough

Walkthrough

Enforces presence of required CI checks (checks, commit-lint, dco-check) by validating statusCheckRollup; gate checker and PR triage now treat missing required entries as failing/pending, preventing approval until checks appear and complete.

Changes

Cohort / File(s)	Summary
Gate instructions .agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md	Added explicit instruction: if checks, commit-lint, or dco-check are missing from statusCheckRollup, workflows are considered not triggered and PRs must not be approved until those checks appear and complete and the gate is re-run.
Gate checking logic .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts	Introduced StatusCheck typing and REQUIRED_CHECK_NAMES usage; checkCi() now returns missingChecks and fails early when required checks are absent. Adjusted evaluation: StatusContext uses state, CheckRun treats status !== "COMPLETED" as pending; extended gate output schema.
Shared types/constants .agents/skills/nemoclaw-maintainer-day/scripts/shared.ts	Added exported REQUIRED_CHECK_NAMES constant and StatusCheck interface to represent CheckRun/StatusContext union fields.
PR triage .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts	classifyPr() now treats pr.statusCheckRollup as StatusCheck[], computes present check identifiers, and sets checksGreen = false if any required check is missing; loop short-circuits once requirement fails.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through rollups, nose to the ground,
Seeking checks, commit-lint, dco-check found.
If any are missing I thump with a frown,
Wait for the Actions tab, then run the gate down.
Approve only when every green light is sound.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main change: requiring approved workflows before marking CI as green, which directly addresses the root cause of incomplete required checks being missed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/gate-checker-require-workflow-approval

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts (2)

54-58: Interface doesn't reflect StatusContext fields.

PrData.statusCheckRollup only declares CheckRun fields (name, status, conclusion), but the code at lines 221-224 accesses context via a cast. Consider aligning with the StatusCheck interface from check-gates.ts to accurately represent both shapes.

♻️ Suggested type alignment

   statusCheckRollup: Array<{
+    __typename?: string;
     name: string;
+    context?: string;    // StatusContext field
     status: string;
     conclusion: string;
+    state?: string;      // StatusContext field
   }>;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts around lines 54 -
58, PrData.statusCheckRollup is typed only with CheckRun-like fields (name,
status, conclusion) but the code reads a context property after casting; update
the type to match the full StatusCheck shape from check-gates.ts so both
CheckRun and StatusContext forms are represented. Replace the inline anonymous
type for statusCheckRollup with a union or the imported StatusCheck interface
(or mirror its fields) so references to PrData.statusCheckRollup and code that
accesses .context (and any usage in functions that read status/conclusion) no
longer require unsafe casts.

---

220-229: Consider extracting REQUIRED_CHECKS to shared.ts to avoid duplication.

This duplicates REQUIRED_CHECK_NAMES from check-gates.ts. If the required checks change (e.g., adding a new workflow), both files must be updated in sync, risking drift.

♻️ Suggested refactor

Export from shared.ts:

export const REQUIRED_CHECK_NAMES: string[] = [
  "checks",       // pr.yaml — lint, typecheck, test
  "commit-lint",  // commit-lint.yaml
  "dco-check",    // dco-check.yaml
];

Then import in both triage.ts and check-gates.ts:

 import {
   isRiskyFile,
   run,
   parseStringArg,
+  REQUIRED_CHECK_NAMES,
   // ...
 } from "./shared.ts";

-  const REQUIRED_CHECKS = ["checks", "commit-lint", "dco-check"];
+  const REQUIRED_CHECKS = REQUIRED_CHECK_NAMES;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts around lines 220 -
229, Extract the duplicated constant into a shared export and replace the local
copies: create/export a single array constant (e.g. export const
REQUIRED_CHECK_NAMES: string[] = ["checks","commit-lint","dco-check"]) from
shared.ts, then in triage.ts replace the local REQUIRED_CHECKS with an import of
REQUIRED_CHECK_NAMES and use that in the existing presence check (the code that
builds presentNames and checks REQUIRED_CHECKS.some(...)); do the same in
check-gates.ts (remove its local REQUIRED_CHECK_NAMES and import the shared
constant) so both modules reference the same symbol.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts:
- Around line 54-58: PrData.statusCheckRollup is typed only with CheckRun-like
fields (name, status, conclusion) but the code reads a context property after
casting; update the type to match the full StatusCheck shape from check-gates.ts
so both CheckRun and StatusContext forms are represented. Replace the inline
anonymous type for statusCheckRollup with a union or the imported StatusCheck
interface (or mirror its fields) so references to PrData.statusCheckRollup and
code that accesses .context (and any usage in functions that read
status/conclusion) no longer require unsafe casts.
- Around line 220-229: Extract the duplicated constant into a shared export and
replace the local copies: create/export a single array constant (e.g. export
const REQUIRED_CHECK_NAMES: string[] = ["checks","commit-lint","dco-check"])
from shared.ts, then in triage.ts replace the local REQUIRED_CHECKS with an
import of REQUIRED_CHECK_NAMES and use that in the existing presence check (the
code that builds presentNames and checks REQUIRED_CHECKS.some(...)); do the same
in check-gates.ts (remove its local REQUIRED_CHECK_NAMES and import the shared
constant) so both modules reference the same symbol.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3e43dee7-a1de-49c5-97d3-476de3c8438a

📥 Commits

Reviewing files that changed from the base of the PR and between 77051cc and 75bceb4.

📒 Files selected for processing (3)

• .agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md
• .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts
• .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts