Skip to content

Commit 071af98

Browse files
codepuncherCopilot
andcommitted
fix: wait for Copilot review workflow before merging
CRITICAL FIX: Copilot review runs as GitHub Actions workflow that takes 1-3 minutes. Previously instructions told Copilot CLI to merge after CI passes, but before review workflow completed, causing critical bugs to be missed. Root cause analysis: - Copilot review is 'Copilot code review' GitHub Actions workflow - Workflow starts ~5s after PR created, takes 1-3 minutes to complete - CI checks (Lint, etc.) complete in <1 minute - Old instructions merged after CI, before review workflow finished Real examples of the problem: - PR #147: Merged at 43s, review workflow completed at 2m24s - PR #149: Merged at 1m51s, review workflow completed at 3m2s - PR #150: Merged at 6m56s, review workflow completed at 7m5s Solution: - Wait for CI: gh pr checks --watch - Wait for review workflow: Loop checking for review from copilot-pull-request-reviewer bot - Only merge after BOTH complete Updated sections: - Merging to Default Branch (lines 721-817) - Pattern 1: FreshDesk workflow (lines 288-301) - Pattern 2: PR comments workflow (lines 335-347) - Pattern 3: ClickUp feature workflow (lines 396-409) - Complete Workflow section (lines 602-604) - Quick Reference Commands (lines 1308-1318) This prevents PRs from being merged before automated review completes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 83b9754 commit 071af98

1 file changed

Lines changed: 199 additions & 23 deletions

File tree

shell/copilot-instructions.md

Lines changed: 199 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,13 @@ For PHP, JavaScript, CSS: see "Before Committing" section (line 269 below) for s
150150
-**NEVER merge without staging verification** - Deploy to staging and test before merging
151151
-**NEVER push without running lints locally** - Zero tolerance for CI failures
152152
-**NEVER force push to default branch** - Can break production
153+
-**NEVER merge PR before Copilot review completes** - Wait for `@copilot` review, not just CI checks
153154

154155
### Code Review & PRs
155156
-**NEVER use `gh api` directly for PR comments** - Always use `~/Code/misc/itineris-bin/gh-pr-get-comments` and `gh-pr-reply-to-thread`
156157
-**NEVER use combined short flags** - Use `-m -d` not `-md` (gh doesn't support combined short flags)
157158
-**NEVER skip staging deployment** - Always verify changes on staging before merging
159+
-**NEVER merge immediately after CI passes** - Wait for human/Copilot reviews to complete
158160

159161
### Code Standards
160162
-**NEVER use `global $post`** in PHP - Use `get_post()` instead
@@ -212,6 +214,35 @@ git checkout -b clickup/task-id/description
212214
**Mistake:** Using generic names like `feature/fix` without ticket reference
213215
**Fix:** Include ticket source: `clickup/<task-id>/description` or `freshdesk/<ticket-id>/description`
214216

217+
### 9. Merging Before Copilot Review Completes
218+
**Mistake:** Running `gh pr checks --watch && gh pr merge` immediately after CI passes, before Copilot review workflow finishes
219+
**Fix:** Wait for both CI checks AND Copilot review workflow to complete
220+
221+
```bash
222+
# 1. Wait for CI checks
223+
gh pr checks <pr-number> --watch
224+
225+
# 2. Wait for Copilot review workflow to complete
226+
PR_NUM=$(gh pr view <pr-number> --json number -q .number)
227+
while true; do
228+
STATUS=$(gh run list --workflow="Copilot code review" --json headBranch,status \
229+
--jq ".[] | select(.headBranch == \"refs/pull/${PR_NUM}/head\") | .status")
230+
if [ "$STATUS" = "completed" ]; then
231+
echo "✓ Copilot review workflow complete"
232+
break
233+
fi
234+
echo "⏳ Copilot review workflow status: $STATUS (checking again in 30s)"
235+
sleep 30
236+
done
237+
238+
# 3. Now safe to merge
239+
gh pr merge <pr-number> -m -d --admin
240+
```
241+
242+
**Why this matters:** Copilot review runs as a GitHub Actions workflow that takes 1-3 minutes after CI passes. Merging immediately misses critical review comments and bugs.
243+
244+
**Real example:** PR #147 merged at 43s, review workflow completed at 2m24s - too late. PR #149 merged at 1m51s, review completed at 3m2s.
245+
215246
---
216247

217248
## Common Task Patterns
@@ -265,8 +296,22 @@ wp @staging cache flush
265296
# 9. Mark PR as ready for review
266297
gh pr ready
267298

268-
# 10. Wait for CI and merge
269-
gh pr checks <pr-number> --watch && gh pr merge <pr-number> -m -d --admin
299+
# 10. Wait for CI to pass
300+
gh pr checks <pr-number> --watch
301+
302+
# 11. Wait for Copilot review workflow to complete (DON'T SKIP THIS)
303+
PR_NUM=$(gh pr view <pr-number> --json number -q .number)
304+
while true; do
305+
STATUS=$(gh run list --workflow="Copilot code review" --json headBranch,status \
306+
--jq ".[] | select(.headBranch == \"refs/pull/${PR_NUM}/head\") | .status")
307+
if [ "$STATUS" = "completed" ]; then
308+
break
309+
fi
310+
sleep 30
311+
done
312+
313+
# 12. After BOTH CI and Copilot workflow complete, merge
314+
gh pr merge <pr-number> -m -d --admin
270315
```
271316

272317
### Pattern 2: Address PR Code Review Comments
@@ -298,17 +343,28 @@ Addresses review comment thread PRRT_xxx."
298343

299344
# 6. Repeat steps 2-5 for each comment
300345

301-
# 7. After all comments addressed, push all commits
302-
git push
303-
304-
# 8. Request new review
346+
# 8. After all comments addressed, request new review
305347
gh pr edit <pr-number> --add-reviewer @copilot
306348

307349
# 9. Deploy to staging to verify all fixes
308350
git push origin HEAD:staging --force
309351

310-
# 10. After re-approval, merge
311-
gh pr checks <pr-number> --watch && gh pr merge <pr-number> -m -d --admin
352+
# 10. Wait for CI checks to pass
353+
gh pr checks <pr-number> --watch
354+
355+
# 11. Wait for Copilot review workflow to complete
356+
PR_NUM=$(gh pr view <pr-number> --json number -q .number)
357+
while true; do
358+
STATUS=$(gh run list --workflow="Copilot code review" --json headBranch,status \
359+
--jq ".[] | select(.headBranch == \"refs/pull/${PR_NUM}/head\") | .status")
360+
if [ "$STATUS" = "completed" ]; then
361+
break
362+
fi
363+
sleep 30
364+
done
365+
366+
# 12. After both CI and Copilot workflow complete, merge
367+
gh pr merge <pr-number> -m -d --admin
312368
```
313369

314370
### Pattern 3: Implement New Feature from ClickUp
@@ -361,8 +417,22 @@ git push origin HEAD:staging --force
361417
# 7. Mark ready and get review
362418
gh pr ready
363419

364-
# 8. After approval, merge
365-
gh pr checks <pr-number> --watch && gh pr merge <pr-number> -m -d --admin
420+
# 8. Wait for CI to pass
421+
gh pr checks <pr-number> --watch
422+
423+
# 9. Wait for Copilot review workflow to complete (REQUIRED)
424+
PR_NUM=$(gh pr view <pr-number> --json number -q .number)
425+
while true; do
426+
STATUS=$(gh run list --workflow="Copilot code review" --json headBranch,status \
427+
--jq ".[] | select(.headBranch == \"refs/pull/${PR_NUM}/head\") | .status")
428+
if [ "$STATUS" = "completed" ]; then
429+
break
430+
fi
431+
sleep 30
432+
done
433+
434+
# 10. After both CI and Copilot workflow complete, merge
435+
gh pr merge <pr-number> -m -d --admin
366436

367437
# 9. Verify production deployment
368438
# (happens automatically when default branch is pushed)
@@ -560,8 +630,10 @@ When implementing a feature or fix:
560630
7. **Address code review feedback** - Validate accuracy, reply in threads, resolve when fixed
561631
8. **Deploy to staging for testing** - `git push origin <branch>:staging --force`
562632
9. **Verify changes on staging** - Test that the fix/feature works correctly
563-
10. **Merge to default branch** - `gh pr checks <pr> --watch && gh pr merge <pr> -m -d --admin`
564-
11. **Production deployment** - Happens automatically when default branch is pushed
633+
10. **Wait for CI checks to pass** - `gh pr checks <pr> --watch`
634+
11. **Wait for Copilot review workflow to complete** - Check `gh run list --workflow="Copilot code review"` until status is "completed"
635+
12. **Merge to default branch** - `gh pr merge <pr> -m -d --admin` (only after both CI and Copilot review complete)
636+
13. **Production deployment** - Happens automatically when default branch is pushed
565637

566638
**Key points:**
567639
- Always verify on staging BEFORE merging to default branch
@@ -679,12 +751,105 @@ Use `gh-pr-get-comments` script to retrieve all comments on a PR:
679751

680752
### Merging to Default Branch
681753

682-
- Before merging, ensure the most recent commit was deployed to staging and verified
683-
- Use `gh pr checks <pr-number> --watch --interval 2 && gh pr merge <pr-number> -m -d --admin` to verify CI and merge PRs to the default branch
684-
- Or use the alias: `gh_check_merge <pr-number> --admin` (the alias handles check-and-merge pattern; add `--admin` flag to bypass branch protection)
685-
- The `--admin` flag bypasses branch protection rules
686-
- The `-m -d` flags merge and delete the branch after merging
687-
- **Note**: This is only for merging to the default branch, NOT for deploying to staging
754+
**🚨 CRITICAL: NEVER merge until BOTH CI passes AND Copilot review workflow completes**
755+
756+
**The Problem:** Copilot review runs as a GitHub Actions workflow that takes 1-3 minutes. If you merge before it completes, you'll miss critical bugs.
757+
758+
**How Copilot Review Works:**
759+
- Triggered automatically when PR is created/updated
760+
- Runs as "Copilot code review" GitHub Actions workflow
761+
- Takes 1-3 minutes to analyze code and post review
762+
- Must wait for workflow to complete, not just CI checks
763+
764+
**Required Workflow:**
765+
766+
```bash
767+
# 1. Wait for CI checks (Lint, etc.)
768+
gh pr checks <pr-number> --watch
769+
770+
# 2. Wait for Copilot review workflow to complete (REQUIRED - don't skip!)
771+
while true; do
772+
# Check if Copilot review workflow is complete
773+
REVIEW=$(gh pr view <pr-number> --json reviews -q '.reviews[] | select(.author.login == "copilot-pull-request-reviewer") | .submittedAt')
774+
if [ -n "$REVIEW" ]; then
775+
echo "✓ Copilot review complete at $REVIEW"
776+
break
777+
fi
778+
echo "⏳ Waiting for Copilot review workflow... (checking again in 30s)"
779+
sleep 30
780+
done
781+
782+
# 3. Now safe to merge
783+
gh pr merge <pr-number> -m -d --admin
784+
```
785+
786+
**Alternative - Check workflow runs directly:**
787+
```bash
788+
# Wait for CI
789+
gh pr checks <pr-number> --watch
790+
791+
# Get PR number from branch
792+
PR_NUM=$(gh pr view --json number -q .number)
793+
794+
# Wait for Copilot review workflow to complete
795+
while true; do
796+
STATUS=$(gh run list --repo $(gh repo view --json nameWithOwner -q .nameWithOwner) \
797+
--workflow="Copilot code review" --json headBranch,status,conclusion \
798+
--jq ".[] | select(.headBranch == \"refs/pull/${PR_NUM}/head\") | .status")
799+
800+
if [ "$STATUS" = "completed" ]; then
801+
echo "✓ Copilot review workflow complete"
802+
break
803+
fi
804+
echo "⏳ Copilot review workflow status: $STATUS (checking again in 30s)"
805+
sleep 30
806+
done
807+
808+
# Now safe to merge
809+
gh pr merge <pr-number> -m -d --admin
810+
```
811+
812+
**How to check manually:**
813+
```bash
814+
# Check if Copilot review exists (posted by workflow)
815+
gh pr view <pr-number> --json reviews -q '.reviews[] | select(.author.login == "copilot-pull-request-reviewer")'
816+
817+
# If empty: workflow hasn't completed yet - WAIT
818+
# If shows data: workflow complete - OK TO MERGE
819+
820+
# Or check workflow run status
821+
gh run list --workflow="Copilot code review" --limit 5
822+
```
823+
824+
**WRONG (what causes the problem):**
825+
```bash
826+
# ❌ BAD: Merges after CI only, doesn't wait for Copilot review workflow
827+
gh pr checks <pr-number> --watch && gh pr merge <pr-number> -m -d --admin
828+
```
829+
830+
**Before merging, verify ALL of these:**
831+
1. ✅ Most recent commit was deployed to staging and verified
832+
2. ✅ CI checks (Lint, etc.) have passed
833+
3. ✅ Copilot review workflow has completed (review posted by copilot-pull-request-reviewer)
834+
4. ✅ Any review comments have been addressed
835+
836+
**Real examples of the problem:**
837+
- PR #147: Merged 43s after creation, review workflow completed at 2m24s
838+
- PR #149: Merged at 00:01:51, review workflow completed at 00:03:02 (1m11s after merge)
839+
- PR #150: Merged at 00:06:56, review workflow completed at 00:07:05 (9s after merge - close!)
840+
841+
**Timing data:**
842+
- Copilot review workflow starts ~5 seconds after PR created
843+
- Workflow takes 1-3 minutes to complete
844+
- CI checks usually complete in <1 minute
845+
- **Problem:** CI finishes before review workflow
846+
847+
**⚠️ Do NOT use:**
848+
- `gh_check_merge` alias (only waits for CI, not Copilot workflow)
849+
- `gh pr checks --watch && gh pr merge` (skips Copilot workflow check)
850+
- Any command that merges immediately after CI passes
851+
852+
**This is only for merging to default branch**, NOT for deploying to staging.
688853

689854
### After Pushing
690855

@@ -1151,11 +1316,22 @@ git push -u origin HEAD
11511316
# Open draft PR
11521317
gh pr create --draft --fill
11531318

1154-
# Check CI status and merge
1155-
gh pr checks <pr-number> --watch && gh pr merge <pr-number> -m -d --admin
1156-
1157-
# Or use alias
1158-
gh_check_merge <pr-number> --admin
1319+
# Wait for CI and Copilot review workflow, then merge
1320+
gh pr checks <pr-number> --watch
1321+
1322+
# Wait for Copilot review workflow to complete
1323+
PR_NUM=$(gh pr view <pr-number> --json number -q .number)
1324+
while true; do
1325+
STATUS=$(gh run list --workflow="Copilot code review" --json headBranch,status \
1326+
--jq ".[] | select(.headBranch == \"refs/pull/${PR_NUM}/head\") | .status")
1327+
if [ "$STATUS" = "completed" ]; then
1328+
break
1329+
fi
1330+
sleep 30
1331+
done
1332+
1333+
# After BOTH complete, merge
1334+
gh pr merge <pr-number> -m -d --admin
11591335

11601336
# Deploy any branch to staging
11611337
git push origin <branch-name>:staging --force

0 commit comments

Comments
 (0)