Skip to content

Commit e752467

Browse files
committed
bugfix-314-error-merging-release-branch-after-successful-deployment: Enhance documentation and implementation for deploy label and merge flow. Added new sections in README and index.mdx, and improved MergeRepository to wait for PR-specific check runs before merging. Updated tests for MergeRepository and DeployedActionUseCase to cover new behavior and ensure correctness in handling merges and check waits.
1 parent 3221b21 commit e752467

7 files changed

Lines changed: 286 additions & 86 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Full documentation: **[docs.page/vypdev/copilot](https://docs.page/vypdev/copilo
2323
| [OpenCode (AI)](https://docs.page/vypdev/copilot/opencode-integration) | Progress, Bugbot, think, AI PR description |
2424
| [Testing OpenCode locally](https://docs.page/vypdev/copilot/testing-opencode-plan-locally) | Run check-progress, detect-potential-problems, recommend-steps via CLI |
2525
| [Single actions](https://docs.page/vypdev/copilot/single-actions) | On-demand: check progress, think, create release/tag, deployed |
26+
| [Deploy label and merge flow](docs/single-actions/deploy-label-and-merge.mdx) | Deploy/deployed labels, post-deploy merges, waiting for checks per PR |
2627
| [Issues](https://docs.page/vypdev/copilot/issues) | Issue configuration and types (feature, bugfix, hotfix, release, docs, chore) |
2728
| [Pull requests](https://docs.page/vypdev/copilot/pull-requests) | PR configuration and AI description |
2829
| [Troubleshooting](https://docs.page/vypdev/copilot/troubleshooting) | Common issues and solutions |
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
title: Deploy label and merge flow
3+
description: How the deploy and deployed labels work and how post-deploy merges and check runs are handled.
4+
---
5+
6+
# Deploy label and merge flow
7+
8+
This page describes how the **deploy** label is used and what happens when you run the **deployed** single action after a successful deployment. It also explains how branch merges and check runs are handled so that both release→default and release→develop (or hotfix equivalents) complete correctly.
9+
10+
## The deploy label
11+
12+
- **`deploy`** — Applied to the issue when the release (or hotfix) is ready to be deployed. Your CI/CD or team adds this label when deployment has been triggered or is about to be.
13+
- **`deployed`** — Set by Copilot when the **deployed** single action runs and the issue had the **deploy** label. It indicates "deployment is done and post-deploy steps have been run."
14+
15+
The **deployed** single action is intended to be run **after a successful deployment** (e.g. from a pipeline step or a manual trigger). It requires:
16+
17+
- An issue number (single action is run with `single-action-issue: <number>`).
18+
- The issue must have the **deploy** label and must **not** already have the **deployed** label.
19+
20+
## What the deployed action does
21+
22+
1. **Label update**
23+
Replaces the **deploy** label with **deployed** on the issue.
24+
25+
2. **Branch merges** (if a release or hotfix branch is configured):
26+
- **Release:** merges the release branch into the default branch (e.g. `main`), then into the development branch (e.g. `develop`).
27+
- **Hotfix:** merges the hotfix branch into the default branch, then merges the default branch into the development branch.
28+
29+
3. **Issue closure**
30+
If **all** merge operations succeed, the issue is closed. If any merge fails, the issue is left open and the action reports that one or more merge operations failed.
31+
32+
Each merge is done via a **pull request**: create PR, wait for checks (see below), then merge the PR. If the PR workflow fails (e.g. checks fail or timeout), the code falls back to a **direct** Git merge when possible.
33+
34+
## Waiting for checks before merging
35+
36+
For each PR we create (e.g. release→main, release→develop), we wait until the PR is ready to merge before calling the merge API.
37+
38+
### Per-PR check runs
39+
40+
- GitHub's Checks API returns check runs **by ref** (branch), not by PR. The same branch can be the head of **multiple PRs** (e.g. release→main and release→develop).
41+
- We **only consider check runs that belong to the current PR**, using the `pull_requests` array on each check run (filter by `pull_request.number === this PR`). We do **not** use check runs from another PR (e.g. the first merge) to decide that the second PR is ready.
42+
- We wait until all check runs for **this** PR are completed and none have failed. Only then do we merge.
43+
44+
### When the PR has no check runs
45+
46+
- If there are **no** check runs on the ref, we use **commit status checks** (legacy) and merge when none are pending.
47+
- If there **are** check runs on the ref but **none** for this PR (e.g. they are from another PR with the same head, or this base branch has no required checks), we wait a short number of polls (~30 seconds). If no check runs appear for this PR in that window, we assume this PR does not require check runs and **proceed to merge**. If the branch actually requires checks, GitHub will reject the merge and we fall back to the direct merge path (which may also fail under branch protection).
48+
49+
### Timeout
50+
51+
- A configurable **merge timeout** (input `merge-timeout`, default 600 seconds) limits how long we wait for checks **per merge**. If we hit the timeout, we throw and the code attempts a direct merge as fallback.
52+
53+
## Summary table
54+
55+
| Scenario | Behaviour |
56+
|----------|-----------|
57+
| PR has check runs for this PR | Wait until all are completed and none failed, then merge. |
58+
| PR has no check runs on ref | Use status checks; if none pending, merge. |
59+
| Check runs on ref but none for this PR | Wait a few polls (~30 s); if still none, proceed to merge (branch may have no required checks). |
60+
| Timeout waiting for checks | Attempt direct merge. |
61+
| PR merge fails, direct merge fails | Return failure; issue is not closed. |
62+
63+
## Related code and tests
64+
65+
- **Use case:** `src/usecase/actions/deployed_action_use_case.ts` — deploy label handling, merge order, issue close.
66+
- **Merge and checks:** `src/data/repository/merge_repository.ts` — PR creation, per-PR check filtering, status checks fallback, direct merge fallback.
67+
- **Tests:**
68+
- `src/usecase/actions/__tests__/deployed_action_use_case.test.ts` — deploy label validation, release/hotfix merge order, close on success, no close on merge failure.
69+
- `src/data/repository/__tests__/merge_repository.test.ts` — check runs per PR, no check runs, timeout, direct merge fallback.

docs/single-actions/index.mdx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ When you set the **`single-action`** input (and any required targets such as `si
1414
<Card title="Configuration" icon="gear" href="/single-actions/configuration">
1515
single-action, single-action-issue, single-action-version, and other inputs.
1616
</Card>
17+
<Card title="Deploy label and merge flow" icon="git-merge" href="/single-actions/deploy-label-and-merge">
18+
Deploy/deployed labels, post-deploy merges (release/hotfix → default and develop), and how we wait for checks per PR.
19+
</Card>
1720
<Card title="Workflow & CLI" icon="terminal" href="/single-actions/workflow-and-cli">
1821
Run from a GitHub Actions workflow or from the giik CLI.
1922
</Card>
@@ -38,5 +41,6 @@ When you set the **`single-action`** input (and any required targets such as `si
3841
## Next steps
3942

4043
- **[Available actions](/single-actions/available-actions)** — Complete list with inputs and descriptions.
44+
- **[Deploy label and merge flow](/single-actions/deploy-label-and-merge)** — Deploy/deployed labels and post-deploy merges.
4145
- **[Workflow & CLI](/single-actions/workflow-and-cli)** — How to run from a workflow and from the CLI.
4246
- **[Examples](/single-actions/examples)** — YAML and CLI examples.

src/data/repository/__tests__/merge_repository.test.ts

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
/**
2-
* Unit tests for MergeRepository: mergeBranch (PR merge and direct merge fallback).
2+
* Unit tests for MergeRepository.mergeBranch: PR creation, waiting for checks per PR,
3+
* fallback when the PR has no check runs, timeout, and direct merge fallback.
4+
*
5+
* Used by the deploy flow (release/hotfix → default and develop). See docs/single-actions/deploy-label-and-merge.mdx.
36
*/
47

58
import { MergeRepository } from '../merge_repository';
@@ -50,6 +53,7 @@ describe('MergeRepository', () => {
5053
mockReposGetCombinedStatusForRef.mockReset();
5154
});
5255

56+
describe('PR creation and merge (timeout <= 10 skips waiting for checks)', () => {
5357
it('creates PR, updates body, merges and returns success (timeout <= 10 skips wait)', async () => {
5458
mockPullsCreate.mockResolvedValue({
5559
data: { number: 42 },
@@ -136,7 +140,9 @@ describe('MergeRepository', () => {
136140
expect(result.some(r => r.success === false && r.steps?.some(s => s.includes('Failed to merge')))).toBe(true);
137141
expect(result.length).toBeGreaterThanOrEqual(2);
138142
});
143+
});
139144

145+
describe('waiting for checks (timeout > 10): per-PR check runs, no checks, timeout', () => {
140146
it('when timeout > 10 waits for check runs (all completed) then merges', async () => {
141147
mockPullsCreate.mockResolvedValue({ data: { number: 1 } });
142148
mockPullsListCommits.mockResolvedValue({ data: [{ commit: { message: 'msg' } }] });
@@ -145,7 +151,7 @@ describe('MergeRepository', () => {
145151
mockChecksListForRef.mockResolvedValue({
146152
data: {
147153
check_runs: [
148-
{ name: 'ci', status: 'completed', conclusion: 'success' },
154+
{ name: 'ci', status: 'completed', conclusion: 'success', pull_requests: [{ number: 1 }] },
149155
],
150156
},
151157
});
@@ -179,7 +185,7 @@ describe('MergeRepository', () => {
179185
mockChecksListForRef.mockResolvedValue({
180186
data: {
181187
check_runs: [
182-
{ name: 'ci', status: 'completed', conclusion: 'failure' },
188+
{ name: 'ci', status: 'completed', conclusion: 'failure', pull_requests: [{ number: 1 }] },
183189
],
184190
},
185191
});
@@ -214,14 +220,87 @@ describe('MergeRepository', () => {
214220
expect(mockReposGetCombinedStatusForRef).toHaveBeenCalled();
215221
});
216222

223+
it('when timeout > 10 waits only for check runs tied to this PR (ignores runs from other PRs with same head)', async () => {
224+
jest.useFakeTimers();
225+
mockPullsCreate.mockResolvedValue({ data: { number: 42 } });
226+
mockPullsListCommits.mockResolvedValue({ data: [{ commit: { message: 'msg' } }] });
227+
mockPullsUpdate.mockResolvedValue({});
228+
mockPullsMerge.mockResolvedValue({});
229+
// First poll: runs exist but for another PR (e.g. release→master already merged). We must not treat as completed.
230+
mockChecksListForRef
231+
.mockResolvedValueOnce({
232+
data: {
233+
check_runs: [
234+
{ name: 'ci', status: 'completed', conclusion: 'success', pull_requests: [{ number: 1 }] },
235+
],
236+
},
237+
})
238+
.mockResolvedValueOnce({
239+
data: {
240+
check_runs: [
241+
{ name: 'ci', status: 'completed', conclusion: 'success', pull_requests: [{ number: 42 }] },
242+
],
243+
},
244+
});
245+
mockReposGetCombinedStatusForRef.mockResolvedValue({
246+
data: { state: 'success', statuses: [] },
247+
});
248+
249+
const promise = repo.mergeBranch(
250+
'owner',
251+
'repo',
252+
'release/1.0',
253+
'develop',
254+
30,
255+
'token',
256+
);
257+
await jest.runAllTimersAsync();
258+
const result = await promise;
259+
260+
jest.useRealTimers();
261+
expect(result).toHaveLength(1);
262+
expect(result[0].success).toBe(true);
263+
expect(mockChecksListForRef).toHaveBeenCalledTimes(2);
264+
expect(mockPullsMerge).toHaveBeenCalled();
265+
});
266+
267+
it('when timeout > 10 and no check runs for this PR after a few polls, proceeds to merge (branch may have no required checks)', async () => {
268+
jest.useFakeTimers();
269+
mockPullsCreate.mockResolvedValue({ data: { number: 99 } });
270+
mockPullsListCommits.mockResolvedValue({ data: [] });
271+
mockPullsUpdate.mockResolvedValue({});
272+
mockPullsMerge.mockResolvedValue({});
273+
// Check runs on ref are for another PR only; this PR (99) has none (e.g. develop has no required checks).
274+
mockChecksListForRef.mockResolvedValue({
275+
data: {
276+
check_runs: [
277+
{ name: 'ci', status: 'completed', conclusion: 'success', pull_requests: [{ number: 1 }] },
278+
],
279+
},
280+
});
281+
mockReposGetCombinedStatusForRef.mockResolvedValue({
282+
data: { state: 'success', statuses: [] },
283+
});
284+
285+
const promise = repo.mergeBranch('o', 'r', 'release/1.0', 'develop', 60, 'token');
286+
await jest.runAllTimersAsync();
287+
const result = await promise;
288+
289+
jest.useRealTimers();
290+
expect(result).toHaveLength(1);
291+
expect(result[0].success).toBe(true);
292+
expect(mockChecksListForRef).toHaveBeenCalledTimes(3);
293+
expect(mockPullsMerge).toHaveBeenCalled();
294+
});
295+
217296
it('when timeout > 10 and checks never complete throws then direct merge succeeds', async () => {
218297
jest.useFakeTimers();
219298
mockPullsCreate.mockResolvedValue({ data: { number: 1 } });
220299
mockPullsListCommits.mockResolvedValue({ data: [] });
221300
mockPullsUpdate.mockResolvedValue({});
222301
mockChecksListForRef.mockResolvedValue({
223302
data: {
224-
check_runs: [{ name: 'ci', status: 'in_progress', conclusion: null }],
303+
check_runs: [{ name: 'ci', status: 'in_progress', conclusion: null, pull_requests: [{ number: 1 }] }],
225304
},
226305
});
227306
mockReposGetCombinedStatusForRef.mockResolvedValue({
@@ -236,4 +315,5 @@ describe('MergeRepository', () => {
236315
jest.useRealTimers();
237316
expect(result.some(r => r.success === true && r.steps?.some(s => s.includes('direct merge')))).toBe(true);
238317
});
318+
});
239319
});

src/data/repository/merge_repository.ts

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,14 @@ import { logDebugInfo, logError } from '../../utils/logger';
33
import { Result } from '../model/result';
44

55
/**
6-
* Repository for merging branches (via PR or direct merge).
7-
* Isolated to allow unit tests with mocked Octokit.
6+
* Repository for merging branches: creates a PR, waits for that PR's check runs (or status checks),
7+
* then merges the PR; on failure, falls back to a direct Git merge.
8+
*
9+
* Check runs are filtered by PR (pull_requests) so we only wait for the current PR's checks,
10+
* not those of another PR sharing the same head (e.g. release→main vs release→develop).
11+
* If the PR has no check runs after a short wait, we proceed to merge (branch may have no required checks).
12+
*
13+
* @see docs/single-actions/deploy-label-and-merge.mdx for the deploy flow and check-wait behaviour.
814
*/
915
export class MergeRepository {
1016

@@ -61,10 +67,13 @@ This PR merges **${head}** into **${base}**.
6167
});
6268

6369
const iteration = 10;
70+
/** Give workflows a short window to register check runs for this PR; after this, we allow merge with no check runs (e.g. branch has no required checks). */
71+
const maxWaitForPrChecksAttempts = 3;
6472
if (timeout > iteration) {
6573
// Wait for checks to complete - can use regular token for reading checks
6674
let checksCompleted = false;
6775
let attempts = 0;
76+
let waitForPrChecksAttempts = 0;
6877
const maxAttempts = timeout > iteration ? Math.floor(timeout / iteration) : iteration;
6978

7079
while (!checksCompleted && attempts < maxAttempts) {
@@ -74,6 +83,14 @@ This PR merges **${head}** into **${base}**.
7483
ref: head,
7584
});
7685

86+
// Only consider check runs that are for this PR. When the same branch is used in
87+
// multiple PRs (e.g. release→master and release→develop), listForRef returns runs
88+
// for all PRs; we must wait for runs tied to the current PR or we may see completed
89+
// runs from the other PR and merge before this PR's checks have run.
90+
const runsForThisPr = (checkRuns.check_runs as Array<{ status: string; conclusion: string | null; name: string; pull_requests?: Array<{ number: number }> }>).filter(
91+
run => run.pull_requests?.some(pr => pr.number === pullRequest.number),
92+
);
93+
7794
// Get commit status checks for the PR head commit
7895
const { data: commitStatus } = await octokit.rest.repos.getCombinedStatusForRef({
7996
owner: owner,
@@ -82,11 +99,11 @@ This PR merges **${head}** into **${base}**.
8299
});
83100

84101
logDebugInfo(`Combined status state: ${commitStatus.state}`);
85-
logDebugInfo(`Number of check runs: ${checkRuns.check_runs.length}`);
102+
logDebugInfo(`Number of check runs for this PR: ${runsForThisPr.length} (total on ref: ${checkRuns.check_runs.length})`);
86103

87-
// If there are check runs, prioritize those over status checks
88-
if (checkRuns.check_runs.length > 0) {
89-
const pendingCheckRuns = checkRuns.check_runs.filter(
104+
// If there are check runs for this PR, wait for them to complete
105+
if (runsForThisPr.length > 0) {
106+
const pendingCheckRuns = runsForThisPr.filter(
90107
check => check.status !== 'completed',
91108
);
92109

@@ -95,7 +112,7 @@ This PR merges **${head}** into **${base}**.
95112
logDebugInfo('All check runs have completed.');
96113

97114
// Verify if all checks passed
98-
const failedChecks = checkRuns.check_runs.filter(
115+
const failedChecks = runsForThisPr.filter(
99116
check => check.conclusion === 'failure',
100117
);
101118

@@ -111,6 +128,21 @@ This PR merges **${head}** into **${base}**.
111128
attempts++;
112129
continue;
113130
}
131+
} else if (checkRuns.check_runs.length > 0 && runsForThisPr.length === 0) {
132+
// There are runs on the ref but none for this PR. Either workflows for this PR
133+
// haven't registered yet, or this PR/base has no required checks.
134+
waitForPrChecksAttempts++;
135+
if (waitForPrChecksAttempts >= maxWaitForPrChecksAttempts) {
136+
checksCompleted = true;
137+
logDebugInfo(
138+
`No check runs for this PR after ${maxWaitForPrChecksAttempts} polls; proceeding to merge (branch may have no required checks).`,
139+
);
140+
} else {
141+
logDebugInfo('Check runs exist on ref but none for this PR yet; waiting for workflows to register.');
142+
await new Promise(resolve => setTimeout(resolve, iteration * 1000));
143+
attempts++;
144+
}
145+
continue;
114146
} else {
115147
// Fall back to status checks if no check runs exist
116148
const pendingChecks = commitStatus.statuses.filter(status => {

0 commit comments

Comments
 (0)