diff --git a/.github/workflows/quality-gate.md b/.github/workflows/quality-gate.md index 35dcafc..c730cd9 100644 --- a/.github/workflows/quality-gate.md +++ b/.github/workflows/quality-gate.md @@ -44,9 +44,9 @@ This workflow runs when a review is submitted on a pull request. 1. First, check if the PR has the `aw` label. If it does NOT have the `aw` label, stop immediately — this workflow only evaluates agent-created PRs. 2. Check the review that triggered this workflow. This workflow should only proceed when: - - The review is an APPROVAL - The reviewer is `copilot-pull-request-reviewer` (the Copilot reviewer bot) - If the triggering review is not a Copilot approval, stop immediately. + - The review state is COMMENTED or APPROVED (Copilot auto-reviews submit as COMMENTED, not APPROVED) + If the triggering review is not from Copilot, stop immediately. 3. Verify that CI checks are passing on the PR. If CI is still running or has failures, stop — do not evaluate until CI passes. @@ -63,8 +63,10 @@ This workflow runs when a review is submitted on a pull request. - HIGH: Changes to core business logic, API contracts, data models, dependency updates, security-sensitive code 5. Make your decision: - - If code quality is good AND impact is LOW or MEDIUM: Submit an APPROVE review with a brief summary of what was evaluated (e.g., "Low-impact refactoring with good test coverage. Auto-approving."). - - If code quality is good but impact is HIGH: Add a comment to the PR explaining: what the high-impact areas are, why manual review is recommended, and what specifically a human reviewer should look at. Do NOT approve. + - If code quality is good AND impact is LOW or MEDIUM: Submit an APPROVE review with a brief summary of what was evaluated (e.g., "Low-impact test addition with good coverage. Auto-approving for merge."). The PR has auto-merge enabled — your approval satisfies the required review and triggers automatic merge. + - If code quality is good but impact is HIGH: Add a comment to the PR explaining: what the high-impact areas are, why manual review is recommended, and what specifically a human reviewer should look at. Do NOT approve — auto-merge will remain blocked until a human approves. - If code quality is poor: Add a comment explaining the quality concerns. Do NOT approve. Be conservative — when in doubt about impact level, round up. It's better to flag something for human review than to auto-merge a risky change. + +Note: PRs created by the Issue Implementer have auto-merge enabled. Your APPROVE review is what triggers the merge. This is intentional — the pipeline is: Implementer creates PR → CI passes → Copilot reviews → Quality Gate evaluates and approves → GitHub auto-merges. diff --git a/docs/agentic-workflows.md b/docs/agentic-workflows.md index 83dcdb9..efc9beb 100644 --- a/docs/agentic-workflows.md +++ b/docs/agentic-workflows.md @@ -31,10 +31,10 @@ Audit/Health Agent → creates issue (max 2) → dispatches Implementer → Implementer creates PR (lint-clean, non-draft, auto-merge, aw label) → CI runs + Copilot auto-reviews (parallel, via ruleset) → CI fails? → CI Fixer agent (1 retry, label guard) - → Copilot has comments? → Review Responder addresses them (1 attempt, label guard) - → Copilot approves → Quality Gate evaluates quality + blast radius + → Copilot has comments? → Review Responder addresses them (pushes fixes) + → Copilot reviews (COMMENTED state) → Quality Gate evaluates quality + blast radius → LOW/MEDIUM impact → approves → auto-merge fires - → HIGH impact → flags for human review + → HIGH impact → flags for human review (auto-merge stays blocked) ``` @@ -291,6 +291,8 @@ EOF - **Reviewer identity**: `copilot-pull-request-reviewer[bot]` (login: `copilot-pull-request-reviewer`) - **Event actor**: `Copilot` (the GitHub App identity — this is what `context.actor` returns and what `check_membership.cjs` matches against) +> **Pitfall**: Copilot auto-reviews almost always submit as `COMMENTED`, not `APPROVED`. Any downstream workflow that triggers on `pull_request_review` and checks the review state must accept `COMMENTED` reviews from Copilot — not just `APPROVED`. The Quality Gate handles this correctly. + ### Addressing Copilot review comments (GraphQL) ```bash diff --git a/docs/changelog.md b/docs/changelog.md index d94578f..382fd1e 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -4,6 +4,14 @@ Append-only history of repo-level changes (CI, infra, shared config). Tool-speci --- +## fix: Quality Gate trigger condition — accept COMMENTED reviews from Copilot — 2026-03-15 + +**Problem**: Quality Gate instructions required the triggering review to be an APPROVAL from Copilot. But Copilot auto-reviews almost always submit as `COMMENTED` (not `APPROVED`), so the Quality Gate would see the COMMENTED state and stop immediately (noop). This meant the Quality Gate never actually evaluated or approved agent PRs, and auto-merge stayed blocked. + +**Fix**: Updated Quality Gate instructions to accept both COMMENTED and APPROVED reviews from Copilot. Added documentation about the auto-merge flow: Quality Gate approval is what triggers GitHub auto-merge on agent PRs. (PR #80, closes #81) + +--- + ## fix: Copilot actor name mismatch in bots list — 2026-03-15 **Problem**: `check_membership.cjs` matches `context.actor` (`Copilot`) against `GH_AW_ALLOWED_BOTS`, but bots list only had `copilot-pull-request-reviewer` (the reviewer login). Actor name mismatch → `activated = false` → agent jobs skipped. diff --git a/src/copilot_usage/cli.py b/src/copilot_usage/cli.py index 329af2f..9a59e6a 100644 --- a/src/copilot_usage/cli.py +++ b/src/copilot_usage/cli.py @@ -16,6 +16,7 @@ import click from rich.console import Console from rich.table import Table +from watchdog.events import FileSystemEventHandler # type: ignore[import-untyped] from watchdog.observers import Observer # type: ignore[import-untyped] from copilot_usage.models import SessionSummary @@ -114,14 +115,15 @@ def _read_line_nonblocking(timeout: float = 0.5) -> str | None: return None -class _FileChangeHandler: +class _FileChangeHandler(FileSystemEventHandler): # type: ignore[misc] """Watchdog handler that triggers refresh on any session-state change.""" def __init__(self, change_event: threading.Event) -> None: + super().__init__() self._change_event = change_event self._last_trigger = 0.0 - def dispatch(self, event: object) -> None: # noqa: ANN001 + def dispatch(self, event: object) -> None: now = time.monotonic() if now - self._last_trigger > 2.0: # debounce 2s self._last_trigger = now @@ -132,7 +134,7 @@ def _start_observer(session_path: Path, change_event: threading.Event) -> object """Start a watchdog observer monitoring *session_path* for changes.""" handler = _FileChangeHandler(change_event) observer = Observer() - observer.schedule(handler, str(session_path), recursive=True) # type: ignore[arg-type] + observer.schedule(handler, str(session_path), recursive=True) observer.daemon = True observer.start() return observer diff --git a/tests/copilot_usage/test_cli.py b/tests/copilot_usage/test_cli.py index b16543b..2471ac5 100644 --- a/tests/copilot_usage/test_cli.py +++ b/tests/copilot_usage/test_cli.py @@ -586,3 +586,19 @@ def test_dispatch_fires_again_after_debounce_gap(self) -> None: handler._last_trigger = _time.monotonic() - 3.0 # pyright: ignore[reportPrivateUsage] handler.dispatch(object()) assert event.is_set() + + def test_inherits_from_filesystemeventhandler(self) -> None: + """_FileChangeHandler inherits from watchdog FileSystemEventHandler.""" + from watchdog.events import ( + FileSystemEventHandler, # type: ignore[import-untyped] + ) + + from copilot_usage.cli import ( + _FileChangeHandler, # pyright: ignore[reportPrivateUsage] + ) + + event = threading.Event() + handler = _FileChangeHandler(event) + assert isinstance(handler, FileSystemEventHandler) + handler.dispatch(object()) + assert event.is_set()