Skip to content
Draft
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
113 changes: 65 additions & 48 deletions .agents/skills/github-robot-review-gate/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,78 +10,95 @@ description: >-

## Core rule

Diagnose the exact merge blocker before changing code or repository settings.
CodeRabbit/check-run success can satisfy this repo's robot-review policy only
when current-head CodeRabbit blocking findings, warnings, and failures are fixed,
rebutted with evidence, or superseded. It is not a GitHub `APPROVED` review. If
GitHub rulesets require human approval, fix the ruleset contract rather than
waiting for humans or disabling security.
Diagnose the exact merge blocker from the live protected branch, rulesets, PR
head, review state, threads, and check runs before changing code or repository
settings. CodeRabbit/check-run success can satisfy this repository's
robot-review evidence path only when current-head blocking findings, warnings,
and failures are fixed, rebutted with evidence, or superseded. Robot-review
evidence does not replace a live GitHub approval-count rule unless the evidence
is itself a qualifying `APPROVED` review object.

The active organization ruleset currently requires one qualifying independent
approval (`required_approving_review_count=1`) and review-thread resolution. Do
not lower that count or the repository rule's
`require_last_push_approval=true` setting, add a bypass, dismiss a review, or
reinterpret a status or check as an approval merely to land a PR. The approval
must be recorded after the last push. If the canonical organization owner
later changes the rule, refetch the live ruleset first and update this skill and
the merge-gate policy from that evidence.

## Root-cause-first workflow

1. Capture the PR head SHA, mergeability, review decision, required checks, and
rule evaluation before proposing a fix.
2. Separate four signals: GitHub review state, CodeRabbit robot-review evidence,
required status contexts, and ruleset settings.
1. Capture the PR head SHA, base tip, mergeability, review decision, submitted
reviews, unresolved threads, required checks, and active rulesets.
2. Separate four signals: GitHub approval state, robot-review evidence,
required status/workflow contexts, and ruleset settings.
3. Identify the narrow blocker: missing current-head robot evidence, unresolved
robot findings, human-review ruleset count, unresolved threads, stale status
context, or failing check.
4. Apply only the minimal reversible fix, then re-capture the same evidence.
robot findings, missing qualifying independent approval, unresolved threads,
stale status context, queued runner acquisition, or a failing check.
4. Apply only a causal fix inside the correct ownership boundary. Do not mutate
a clean source head just to retrigger infrastructure and do not change the
approval count or bypass policy for an individual PR.
5. Re-capture the same evidence on the exact current head before any lifecycle
action. Predecessor reviews/checks do not transfer after a head movement.

## Evidence commands

```bash
gh pr view <pr> \
--json number,headRefOid,mergeable,mergeStateStatus,reviewDecision,statusCheckRollup,latestReviews
--json number,headRefOid,baseRefOid,mergeable,mergeStateStatus,reviewDecision,statusCheckRollup,latestReviews
gh pr checks <pr> --required
gh api repos/<owner>/<repo>/pulls/<pr>/reviews
gh api repos/<owner>/<repo>/commits/<sha>/status
gh api repos/<owner>/<repo>/commits/<sha>/check-runs
gh api repos/<owner>/<repo>/rulesets \
--jq '.[] | {name, enforcement, conditions, rules}'
--jq '.[] | {id, name, enforcement, conditions, rules}'
```

Record the current head SHA with every screenshot, review, and check summary so
stale evidence is not mistaken for current-head approval.
Record the current head SHA with every review, thread, check, screenshot, and
ruleset summary so stale evidence is not mistaken for current-head evidence.

## Guardrails

- Do not bypass branch protection, add bypass actors, use admin merge, force
push, dismiss reviews, or disable security checks unless explicitly requested.
- Do not treat `Review skipped`, CodeRabbit walkthroughs, or check-run success as
a GitHub `APPROVED` review object. They are robot-review gate evidence only.
- Do not wait for human review by default in this repo when robot-review policy
applies; instead verify `required_approving_review_count=0`.
- Do not remove required review thread resolution; keep
`required_review_thread_resolution=true`.
push, destructively rebase, self-approve, dismiss reviews, or disable security
checks for routine delivery.
- Do not lower `required_approving_review_count=1` to unblock a PR.
- Do not treat `Review skipped`, CodeRabbit walkthroughs, overview comments, or
check-run success as a GitHub `APPROVED` review object.
- Keep `required_review_thread_resolution=true`.
- A qualifying independent approval is still required while the live ruleset
requires it, even when current-head robot evidence is otherwise satisfactory.
- Pending, queued, requested, waiting, in-progress, absent-required, cancelled,
failed, stale-head, and predecessor evidence is not permission to merge.

## Stale required status contexts

If a PR that hardens or restores a workflow is blocked by a stale required
context (for example `strix` while fixing Strix), document the stale context and
use a temporary, reversible ruleset adjustment only when necessary. Capture
equivalent temporary evidence before merge, such as a trusted-base rerun,
scanner artifact, SARIF output, or manual security review evidence tied to the
current head SHA. The rollback requirement is part of the fix: restore the
`strix` required context immediately after the hardened workflow emits that
context successfully on the protected branch.

## Safe temporary handling

- Prefer rerunning or updating the branch before touching rulesets.
- If temporary removal is unavoidable, capture before/after ruleset JSON, owner,
expiry, current head SHA, equivalent temporary evidence, and a dated rollback
note in the PR.
- Restore required contexts and confirm `gh pr checks --required` shows the
hardened context before declaring the gate resolved.
context, prefer rerunning or updating the branch when that preserves exact-head
semantics. If the context cannot be emitted until the repair lands, document the
causal cycle and advance the canonical workflow/ruleset owner path instead of
silently weakening Naruon's gate.

A temporary required-context adjustment is exceptional. Use it only when the
canonical ruleset owner explicitly authorizes it and equivalent current-head
evidence is captured. Record before/after ruleset JSON, owner, expiry, current
head SHA, equivalent scanner/test evidence, and a named restore condition. The
approval-count rule is not a stale status context and must not be lowered by
this procedure.

## Common mistakes

- Equating CodeRabbit status with GitHub `APPROVED`: treat it as repo
robot-review evidence, then check ruleset review count.
- Waiting for human review despite policy: verify ruleset count is zero and
robot evidence is current-head.
- Removing `strix` permanently to unblock Strix fixes: temporarily remove only
with evidence, then restore once Strix emits.
- Disabling scanners to merge faster: keep security gates on; fix the gate
contract or the failing scanner.
- Equating CodeRabbit or OpenCode status/check evidence with a GitHub approval:
inspect the submitted review object and live approval rule separately.
- Following historical zero-approval guidance after the organization ruleset
changed: live ruleset evidence wins; update stale docs instead of weakening
the rule.
- Treating a queued hosted runner as a product defect: distinguish runner
acquisition/startup failure from executed test failure and advance the
canonical CI owner path while other safe Naruon lanes continue.
- Removing a required scanner permanently to unblock its own repair: preserve
the dependency and use only an owner-authorized, evidenced, reversible
recovery path when a true bootstrap cycle is proven.
- Disabling scanners or dismissing reviews to merge faster: fix the underlying
gate, workflow, source, or ownership-path defect instead.
53 changes: 53 additions & 0 deletions backend/tests/test_merge_gate_live_ruleset_contract.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from pathlib import Path


REPOSITORY_ROOT = Path(__file__).resolve().parents[2]
MERGE_GATE_POLICY = REPOSITORY_ROOT / "docs/development/merge-gate-policy.md"
ROBOT_REVIEW_SKILL = (
REPOSITORY_ROOT / ".agents/skills/github-robot-review-gate/SKILL.md"
)


def test_merge_gate_guidance_matches_live_one_approval_ruleset() -> None:
"""Operator guidance must not instruct agents to weaken the live review gate."""
policy = MERGE_GATE_POLICY.read_text(encoding="utf-8")
skill = ROBOT_REVIEW_SKILL.read_text(encoding="utf-8")

for guidance in (policy, skill):
assert "required_approving_review_count=0" not in guidance
assert "required_approving_review_count=1" in guidance
assert "do not lower" in guidance.lower()


def test_live_last_push_approval_rule_is_explicit() -> None:
"""Current guidance must preserve the repository's post-last-push approval rule."""
policy = MERGE_GATE_POLICY.read_text(encoding="utf-8")
skill = ROBOT_REVIEW_SKILL.read_text(encoding="utf-8")

for guidance in (policy, skill):
assert "require_last_push_approval=true" in guidance
assert "after the last push" in guidance.lower()


def test_robot_evidence_is_not_substituted_for_required_approval() -> None:
"""Robot evidence and the live GitHub approval requirement remain distinct gates."""
policy = MERGE_GATE_POLICY.read_text(encoding="utf-8")

assert "one qualifying independent approval" in policy.lower()
assert "robot-review evidence does not replace" in policy.lower()


def test_merge_gate_policy_tracks_live_develop_application_ci_targets() -> None:
"""Current operator policy names the protected develop CI path, not only legacy master."""
policy = MERGE_GATE_POLICY.read_text(encoding="utf-8")

assert "pull requests to `develop`, `master`, and `release/**`" in policy
assert "pushes to `develop` and `master`" in policy


def test_required_neutral_or_skipped_evidence_never_becomes_merge_permission() -> None:
"""Required neutral/skipped evidence remains incomplete even if a workflow emits it."""
policy = MERGE_GATE_POLICY.read_text(encoding="utf-8")

assert "skipped-required and neutral evidence are non-passing" in policy.lower()
assert "success, pass, skipped, and neutral states satisfy" not in policy.lower()
Loading
Loading