Skip to content
Merged
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
45 changes: 40 additions & 5 deletions .github/workflows/pipeline-orchestrator.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,13 @@ jobs:
for PR in $PR_NUMBERS; do
echo "::group::Checking CI and reviews for PR #${PR}"

# Query PR state: CI status, labels, review threads
# Query PR state: CI status, labels, review threads, reviews
PR_DATA=$(gh api graphql -f query='
query($owner: String!, $repo: String!, $pr: Int!) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $pr) {
headRefName
headRefOid
labels(first: 15) { nodes { name } }
commits(last: 1) { nodes { commit {
statusCheckRollup { state contexts(first: 30) { nodes {
Expand All @@ -175,6 +176,11 @@ jobs:
id isResolved
comments(last: 1) { nodes { author { login } } }
}}
latestReviews(first: 10) { nodes {
author { login }
state
commit { oid }
}}
}
}
}' -f owner="$OWNER" -f repo="$REPO" -F pr="$PR") || {
Expand All @@ -185,6 +191,7 @@ jobs:

LABELS=$(echo "$PR_DATA" | jq -r '[.data.repository.pullRequest.labels.nodes[].name]')
BRANCH=$(echo "$PR_DATA" | jq -r '.data.repository.pullRequest.headRefName')
HEAD_SHA=$(echo "$PR_DATA" | jq -r '.data.repository.pullRequest.headRefOid')

# Check CI status — look for the "check" job specifically
CI_CONCLUSION=$(echo "$PR_DATA" | jq -r '
Expand Down Expand Up @@ -286,12 +293,40 @@ jobs:

echo " ✅ PR #${PR}: All threads resolved, CI passing."

# --- Step 4: Dispatch quality gate if not already approved ---
HAS_QG_APPROVED=$(echo "$LABELS" | jq -r 'if any(. == "aw-quality-gate-approved") then "yes" else "" end')
# --- Step 4: Dispatch quality gate if ready ---
# Check 1: Has Copilot reviewed the current head commit?
HAS_COPILOT_REVIEW=$(echo "$PR_DATA" | jq -r --arg sha "$HEAD_SHA" '
[.data.repository.pullRequest.latestReviews.nodes[]
| select(.author.login == "copilot-pull-request-reviewer")
| select(.commit.oid == $sha)]
| length > 0')

if [[ "$HAS_COPILOT_REVIEW" != "true" ]]; then
echo " ⏳ PR #${PR}: Waiting for Copilot review on current commit. Skipping quality gate."
echo "::endgroup::"
continue
fi

# Check 2: Is a Copilot code review currently in progress for this commit?
COPILOT_REVIEW_RUNNING=$(gh run list --workflow="Copilot code review" --json headSha,status --jq --arg sha "$HEAD_SHA" '[.[] | select(.headSha == $sha) | select(.status == "in_progress" or .status == "queued")] | length' 2>/dev/null || echo "1")
if [[ "$COPILOT_REVIEW_RUNNING" -gt 0 ]]; then
echo " ⏳ PR #${PR}: Copilot review in progress. Skipping quality gate."
echo "::endgroup::"
continue
fi

# Check 3: Is there a valid (non-dismissed) approval on the current commit?
HAS_VALID_APPROVAL=$(echo "$PR_DATA" | jq -r --arg sha "$HEAD_SHA" '
[.data.repository.pullRequest.latestReviews.nodes[]
| select(.author.login == "${{ github.repository_owner }}")
| select(.state == "APPROVED")
| select(.commit.oid == $sha)]
| length > 0')

HAS_QG_EVALUATED=$(echo "$LABELS" | jq -r 'if any(. == "aw-quality-gate-evaluated") then "yes" else "" end')

if [[ -n "$HAS_QG_APPROVED" ]]; then
echo " ✅ PR #${PR}: Already quality-gate-approved. Auto-merge should handle it."
if [[ "$HAS_VALID_APPROVAL" == "true" ]]; then
echo " ✅ PR #${PR}: Valid quality-gate approval on current commit. Auto-merge should handle it."
elif [[ -n "$HAS_QG_EVALUATED" ]]; then
echo " ⏸️ PR #${PR}: Quality gate already evaluated (needs human review). Skipping."
else
Expand Down
12 changes: 12 additions & 0 deletions docs/agentic-workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,18 @@ Branch protection's `dismiss_stale_reviews: true` dismisses the quality gate's A
### 27. Shared imports use `imports:` + `steps:` pattern
To pre-fetch data before the agent runs, create a shared `.md` file with a `steps:` block in the frontmatter. The importing workflow uses `imports: [shared/filename.md]`. The steps run as regular workflow steps (with full `gh` CLI access and `GITHUB_TOKEN`), writing data to `/tmp/gh-aw/` for the agent to read. This bypasses MCP tool limitations. Based on the pattern from `github/gh-aw`'s own `copilot-pr-data-fetch.md`.

### 28. Don't dispatch quality gate until Copilot has reviewed the current commit
Copilot code review runs asynchronously. If the orchestrator dispatches the quality gate as soon as CI passes and threads are resolved, it can approve before Copilot reviews. Then Copilot's comments arrive after merge — too late. Always verify a Copilot review exists on the current head commit AND no Copilot review is in-progress before dispatching the quality gate. Use `latestReviews` in GraphQL to check review state and `gh run list` to check for active review runs.

### 29. Check actual review approval state, not labels
Labels like `aw-quality-gate-approved` persist even after `dismiss_stale_reviews` invalidates the approval. Never use labels as the source of truth for review state. Query `latestReviews` and verify the APPROVE review is on the current head commit and not dismissed. Labels are hints for humans, not gates for automation.

### 30. Copilot code review always submits as COMMENTED
GitHub Copilot code review never submits APPROVED or CHANGES_REQUESTED — always COMMENTED. This means: (1) Copilot reviews don't count toward required approvals, (2) they're never dismissed by `dismiss_stale_reviews`, (3) when checking if Copilot has reviewed, filter by author login, not by review state. The quality gate approval (from a PAT) is separate from Copilot's review — don't conflate them.

### 31. Quality gate approval author is the PAT owner, not github-actions
The quality gate uses `GH_AW_WRITE_TOKEN` (a PAT) for `submit-pull-request-review`. The approval appears as the PAT owner (e.g., the repo owner), not as `github-actions[bot]`. Use `${{ github.repository_owner }}` to derive the identity at runtime — never hardcode usernames. This same mistake was made three times in this project (thread resolution, approval detection, changelog docs) before being codified as a rule.

</details>

---
Expand Down
15 changes: 15 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@ Append-only history of repo-level changes (CI, infra, shared config). Tool-speci

---

## fix: quality gate waits for Copilot review and checks actual approval state — 2026-03-21

**Problem 1 (#178)**: The orchestrator dispatched the quality gate as soon as CI was green and threads were resolved, without checking whether Copilot had reviewed yet. The quality gate approved PRs before Copilot's review arrived — then Copilot's comments went unaddressed because the PR was already approved/merged.

**Problem 2 (#187)**: The orchestrator checked for the `aw-quality-gate-approved` label to skip re-dispatch. But `dismiss_stale_reviews: true` in branch protection dismisses the approval when new code is pushed — while the label persists. The orchestrator saw the stale label and skipped the quality gate, leaving the PR stuck with no valid approval.

**Fix**: Replaced the label-based quality gate check with three verification steps:
1. **Copilot review exists** — check `latestReviews` for `copilot-pull-request-reviewer` on the current head commit. Skip if missing.
2. **Copilot review not in progress** — check `gh run list` for active "Copilot code review" runs. Skip if one is running.
3. **Valid approval check** — check `latestReviews` for a non-dismissed APPROVE from the quality gate's token owner (resolved via `github.repository_owner`) on the current head commit. Only skip dispatch if a valid approval exists.

Fixes #178, #187. Follow-up: #203 (no fallback if Copilot review never runs).

---

## fix: re-add labels config to implementer — 2026-03-21

**Problem**: The `labels: [aw]` config on `create-pull-request` was removed weeks ago due to a vague "node ID resolution error" that was never properly investigated. Without it, labeling depends on the agent including labels in its call — which is non-deterministic. Some PRs were created without the `aw` label.
Expand Down
Loading