Save this content to: .claude/skills/copilot-pr-review-loop/SKILL.md

# Copilot PR Review + CI Loop — MANDATORY

## Rule

**NEVER stop a subtask after a single commit-push.** After every push, the agent
**MUST** loop on the following sequence until both conditions are satisfied:

1. Copilot review has **0 outstanding comments** (all addressed)
2. CI has **0 failing checks** (all green or expected-skipped)

## The 9-step flow (canonical, applies to EVERY PR on EVERY repo)

```
┌──────────────────────────────────────────────────────────────────┐
│ 1. fine task — implementation complete                            │
│ 2. test tutti verdi in locale                                     │
│    (phpunit + vitest + playwright + architecture)                 │
│ 3. apri PR with --reviewer copilot   ← MANDATORY FLAG             │
│ 4. attendi CI GitHub verde   (60-180s)                            │
│ 5. attendi Copilot review commenti  (additional 2-15 min)         │
│ 6. leggi commenti (gh pr view N --comments + inline) e fix        │
│ 7. ri-attendi CI tutta verde (after fix push)                     │
│ 8. (se Copilot ri-review) GOTO step 5                             │
│ 9. merge solo dopo:                                               │
│    - Copilot reviewDecision is APPROVED OR no must-fix outstanding│
│    - All CI checks status COMPLETED + conclusion SUCCESS          │
│      (or SKIPPED with explicit reason)                            │
└──────────────────────────────────────────────────────────────────┘
```

**KEY POINT (2026-04-29 reinforcement):** Step 3's `--reviewer copilot` flag and step 5's wait-for-Copilot-review are **NOT optional**. CI green alone is **not enough** — Copilot review (or explicit absence of must-fix comments) is the second gate. Skipping step 5 ("CI green, merge now") is a protocol violation, even on docs-only PRs.

## The legacy loop (kept for fix-iteration phase only)

When a PR has been opened and the FIRST review/CI cycle has surfaced issues:

```
┌─────────────────────────────────────────────────────────┐
│ A. push fix commit                                       │
│ B. wait 60-180s for Copilot re-review + CI to re-run     │
│ C. read PR review comments  (gh pr view N --comments)    │
│ D. read inline review comments (gh api .../comments)     │
│ E. read CI status            (gh pr checks N)            │
│ F. for each failing CI: read failed log                  │
│ G. fix all issues + run local test gate                  │
│ H. commit + push                                         │
│ I. GOTO step B                                           │
│                                                           │
│ EXIT only when:                                          │
│   - Copilot reviewDecision is APPROVED or no outstanding │
│   - All checks status COMPLETED + conclusion SUCCESS     │
└─────────────────────────────────────────────────────────┘
```

## Why this exists

Failure mode this rule prevents: "Claude pushes a commit, sees CI red,
sends a status report to user, stops working." This wastes a CI cycle
and hands a half-broken state to the user. The user has explicitly
said this is unacceptable: when CI is red, the agent must fix the
failure rather than reporting status and stopping. The Copilot review
gate is a separate, non-bypassable check that runs in parallel with
CI; both must converge before merge.

## Scope

Applies to **every PR** opened on:
- `lopadova/AskMyDocs`
- `padosoft/askmydocs-pro`
- `padosoft/laravel-ai-regolo`
- `padosoft/laravel-flow`
- `padosoft/eval-harness`
- `padosoft/laravel-pii-redactor`
- `padosoft/regolo-php-client` (when created)
- Any future Padosoft/Lopadova repo

Applies to **every developer** (Lorenzo, future Padosoft team members,
any AI agent).

## Exact commands per phase

### Phase A — Open PR
```bash
gh pr create \
  --title "feat(...): ..." \
  --base main \
  --head feature/<branch> \
  --body-file .github/PULL_REQUEST_TEMPLATE.md \
  --reviewer copilot
```

Note: `--reviewer copilot` may fail with "could not resolve user". In
that case, the repo must have Copilot Code Review enabled at:
`Settings → General → Pull Requests → Allow GitHub Copilot to review`.
Ask the user to enable it once per repo (one-time manual setup).

### Phase B — Read review (after 60-180s wait)
```bash
# overview
gh pr view <PR> --json state,reviewDecision,mergeable,statusCheckRollup

# top-level comments
gh pr view <PR> --comments

# inline review comments (specific lines)
gh api repos/<owner>/<repo>/pulls/<PR>/comments --jq '.[] | {body, path, line}'
```

### Phase C — Read CI failures
```bash
# list runs for branch
gh run list --branch <branch> --limit 3 --json databaseId,status,conclusion,name

# read failed-job log
gh run view <run-id> --log-failed | head -200
```

### Phase D — Fix locally + test gate

Run the gates the **current repo** actually has. Each repo opts into
the subset that applies; do not run a step the repo does not ship.

```bash
# Always (PHP repos):
vendor/bin/phpunit --no-coverage              # unit + feature suite
vendor/bin/phpstan analyse --memory-limit=512M
vendor/bin/pint --test

# When the repo ships a frontend (presence of frontend/ or package.json):
cd frontend && npm test                       # vitest
npm run e2e                                   # playwright
cd ..

# When the repo defines a dedicated Architecture testsuite in phpunit.xml:
vendor/bin/phpunit --testsuite Architecture   # R30+R31+R32+R34+R35
```

The AskMyDocs main repo runs every step above. Standalone Padosoft
packages (laravel-ai-regolo, eval-harness, laravel-flow,
laravel-pii-redactor) run only the always-on row plus their own
unit suite.

### Phase E — Commit + push
```bash
git add <changed files>
git commit -m "fix(...): address Copilot review on PR #<N> + CI green"
git push origin <branch>
```

Then GOTO Phase B. Never stop after a single push.

## What counts as "Copilot must-fix"

- Bug (off-by-one, null deref, race condition)
- Security (XSS, SQLi, auth bypass, secret leak)
- R-rule violation (R30 cross-tenant, R32 memory privacy, R3 N+1, etc.)
- Test gap (untested branch, unhandled error path)

These MUST be fixed before merge.

## What counts as "should-fix"

- Code style, naming, idiom
- Documentation quality
- Minor refactoring

These SHOULD be fixed unless there's an explicit reason not to. If
declining, reply on the comment with a brief rationale.

## What counts as "discuss"

- Ambiguous suggestions where Copilot may have misunderstood context
- False positives
- Intentional design decisions

Reply explaining; mark resolved when consensus reached.

## Anti-patterns (NEVER DO)

- ❌ Push a commit, see CI red, stop and report to user
- ❌ Skip Copilot review because "it's just a small fix"
- ❌ Mark Copilot comment "resolved" without actually fixing
- ❌ Merge with even 1 outstanding Copilot must-fix comment
- ❌ Merge with CI red (any check failure)
- ❌ Run only phpunit and skip vitest / playwright / architecture
- ❌ Wait less than 60s after push before checking CI (CI may not have started)

## Operational tip — CI iteration time budget

Each CI run is ~2-5 minutes. Plan accordingly:
- Push 1: typical 5-10 Copilot comments + maybe CI red
- Push 2 (after fixes): 1-3 residual comments + CI usually green
- Push 3+: rare; if you reach push 4 without convergence, the issue is
  deeper than a quick fix — ask for human review.

## Reference

- `CLAUDE.md` R36 in repos that ship one (e.g.
  [`lopadova/AskMyDocs/CLAUDE.md`](https://github.com/lopadova/AskMyDocs/blob/main/CLAUDE.md))
  — the canonical statement of this rule and its scope
- The skill's own header `description` field — short trigger summary
  every Claude Code agent reads on session start

First-instance lessons and weekly status notes are tracked outside
the public skill so this file stays portable across public and
private repos.