Fix "merge stack" button showing when it shouldn't

src/github/interface.ts

@@ -47,6 +47,19 @@ export enum PullRequestMergeability {
 	Behind,
 }
 
+export function isStackMergeable(stack: PullRequestStack, number: number): boolean {
+	const current = stack.pullRequests.find(entry => entry.number === number);
+	if (!current || current.position !== stack.position || current.state !== GithubItemStateEnum.Open
+		|| current.isDraft || current.mergeable !== PullRequestMergeability.Mergeable) {
+		return false;
+	}
+	return stack.pullRequests.every(entry =>
+		entry.position > stack.position ||
+		entry.state === GithubItemStateEnum.Merged ||
+		(entry.state === GithubItemStateEnum.Open && !entry.isDraft && entry.mergeable === PullRequestMergeability.Mergeable)
+	);
+}
+
 export enum MergeQueueState {
 	AwaitingChecks,
 	Locked,

src/github/pullRequestModel.ts

@@ -55,6 +55,7 @@ import {
 	IGitTreeItem,
 	IRawFileChange,
 	IRawFileContent,
+	isStackMergeable,
 	IssueReference,
 	ISuggestedReviewer,
 	ITeam,
@@ -533,6 +534,9 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 		if (!stack.pullRequests.some(entry => entry.number === this.number && entry.state === GithubItemStateEnum.Open)) {
 			throw new Error(`Pull request #${this.number} is not open in this stack.`);
 		}
+		if (!isStackMergeable(stack, this.number)) {
+			throw new Error(`Pull request stack for #${this.number} is not ready to merge.`);
+		}
 		const headSha = this.head?.sha;
 		if (!headSha) {
 			throw new Error(`Missing head commit for pull request #${this.number}.`);

src/test/github/pullRequestModel.test.ts

@@ -317,6 +317,41 @@ describe('PullRequestModel', function () {
 				const invalidStack: PullRequestStack = { ...stack, pullRequests: [{ ...stack.pullRequests[1], state: GithubItemStateEnum.Merged }] };
 				await assert.rejects(model.mergeStack(new MockRepository(), invalidStack, 'squash', 'direct_merge'), /not open in this stack/);
 			});
+
+			it('does not submit a merge when the current or a downstack PR is blocked', async function () {
+				const model = createModel();
+				const blocked = [
+					{ ...stack, pullRequests: [stack.pullRequests[0], { ...stack.pullRequests[1], mergeable: PullRequestMergeability.NotMergeable }] },
+					...[
+						{ mergeable: PullRequestMergeability.Conflict },
+						{ mergeable: PullRequestMergeability.Behind },
+						{ mergeable: PullRequestMergeability.Unknown },
+						{ isDraft: true },
+						{ state: GithubItemStateEnum.Closed },
+					].map(change => ({
+						...stack,
+						pullRequests: [{ ...stack.pullRequests[0], ...change }, stack.pullRequests[1]],
+					})),
+				];
+
+				for (const candidate of blocked) {
+					await assert.rejects(model.mergeStack(new MockRepository(), candidate, 'squash', 'direct_merge'), /not ready to merge/);
+				}
+			});
+
+			it('permits merging when a downstack PR is already merged', async function () {
+				const model = createModel();
+				const withMergedBelow = {
+					...stack,
+					pullRequests: [{ ...stack.pullRequests[0], state: GithubItemStateEnum.Merged, mergeable: PullRequestMergeability.Unknown }, stack.pullRequests[1]],
+				};
+				repo.queryProvider.expectOctokitRequest(['request'], [`PUT ${route}`, requestParams(model)], {
+					status: 'merged',
+					details: { message: 'Merged', sha: 'merge-sha' },
+				});
+
+				assert.strictEqual(await model.mergeStack(new MockRepository(), withMergedBelow, 'squash', 'direct_merge'), 'merged');
+			});
 		});
 
 		it('returns no stack when the pull request is not stacked', async function () {

webviews/components/merge.tsx

@@ -24,6 +24,7 @@ import { groupBy } from '../../src/common/utils';
 import {
 	CheckState,
 	GithubItemStateEnum,
+	isStackMergeable,
 	MergeMethod,
 	PullRequestCheckStatus,
 	PullRequestMergeability,
@@ -424,8 +425,9 @@ export const PrActions = ({ pr, isSimple }: { pr: PullRequest; isSimple: boolean
 	if (pr.stackMergeStatus === 'enqueued') {
 		return <div className="status-item">Pull request stack added to the merge queue.</div>;
 	}
-	if (pr.stack && hasWritePermission && !pr.mergeQueueEntry) {
-		return <MergeStack pr={pr} />;
+	if (pr.stack) {
+		return hasWritePermission && !pr.mergeQueueEntry && mergeable === PullRequestMergeability.Mergeable
+			&& isStackMergeable(pr.stack, pr.number) ? <MergeStack pr={pr} /> : null;
 	}
 
 	if (!pr.stack && mergeable === PullRequestMergeability.Mergeable && hasWritePermission && !pr.mergeQueueEntry) {

webviews/editorWebview/test/overview.test.tsx

@@ -67,7 +67,7 @@ describe('Overview', function () {
 			base: 'main',
 			pullRequests: [
 				{ position: 1, number: 793, title: 'First Change', head: 'D1', url: 'https://example.com/793', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable },
-				{ position: 2, number: 794, title: 'Second Change', head: 'D2', url: 'https://example.com/794', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.NotMergeable },
+				{ position: 2, number: 794, title: 'Second Change', head: 'D2', url: 'https://example.com/794', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable },
 				{ position: 3, number: 795, title: 'Third Change', head: 'D3', url: 'https://example.com/795', state: GithubItemStateEnum.Open, isDraft: true, mergeable: PullRequestMergeability.Mergeable },
 			],
 		}).build();
@@ -95,12 +95,12 @@ describe('Overview', function () {
 		]);
 		assert.deepStrictEqual([...section.querySelectorAll('.stack-entry-readiness')].map(entry => [entry.classList[1], entry.getAttribute('aria-label')]), [
 			['waiting', 'Draft pull request cannot be merged'],
-			['blocked', 'Merge requirements not met'],
+			['ready', 'Ready to merge'],
 			['ready', 'Ready to merge'],
 		]);
 		assert.deepStrictEqual([...section.querySelectorAll('.stack-entry-readiness')].map(entry => entry.getAttribute('title')), [
 			'Draft pull request cannot be merged',
-			'Merge requirements not met',
+			'Ready to merge',
 			'Ready to merge',
 		]);
 		assert.strictEqual(section.querySelector('.stack-entry-state'), null);
@@ -184,6 +184,64 @@ describe('Overview', function () {
 			'Mergeability is being checked',
 			'Already merged',
 		]);
+		assert.strictEqual(out.container.querySelector('.stack-merge'), null);
+	});
+
+	it('hides stack merge when the current or an open downstack PR is not mergeable', function () {
+		const readyStack = {
+			position: 2,
+			size: 3,
+			base: 'main',
+			pullRequests: [
+				{ position: 1, number: 793, title: 'First Change', head: 'D1', url: 'https://example.com/793', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable },
+				{ position: 2, number: 794, title: 'Second Change', head: 'D2', url: 'https://example.com/794', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable },
+				{ position: 3, number: 795, title: 'Third Change', head: 'D3', url: 'https://example.com/795', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Conflict },
+			],
+		};
+		const blocked = [
+			new PullRequestBuilder().number(794).mergeable(PullRequestMergeability.Conflict).stack(readyStack).build(),
+			...[
+				{ mergeable: PullRequestMergeability.Conflict },
+				{ mergeable: PullRequestMergeability.NotMergeable },
+				{ mergeable: PullRequestMergeability.Behind },
+				{ mergeable: PullRequestMergeability.Unknown },
+				{ isDraft: true },
+				{ state: GithubItemStateEnum.Closed },
+			].map(change => new PullRequestBuilder().number(794).stack({
+				...readyStack,
+				pullRequests: [{ ...readyStack.pullRequests[0], ...change }, ...readyStack.pullRequests.slice(1)],
+			}).build()),
+		];
+		for (const pr of blocked) {
+			const out = render(
+				<PullRequestContext.Provider value={new PRContext(pr)}>
+					<Overview {...pr} />
+				</PullRequestContext.Provider>,
+			);
+			assert(out.container.querySelector('#pull-request-stack'));
+			assert.strictEqual(out.container.querySelector('.stack-merge'), null);
+			out.unmount();
+		}
+	});
+
+	it('ignores PRs above and already merged PRs below the current stack merge', function () {
+		const stack = {
+			position: 2,
+			size: 3,
+			base: 'main',
+			pullRequests: [
+				{ position: 1, number: 793, title: 'First Change', head: 'D1', url: 'https://example.com/793', state: GithubItemStateEnum.Merged, isDraft: false, mergeable: PullRequestMergeability.Unknown },
+				{ position: 2, number: 794, title: 'Second Change', head: 'D2', url: 'https://example.com/794', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable },
+				{ position: 3, number: 795, title: 'Third Change', head: 'D3', url: 'https://example.com/795', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Conflict },
+			],
+		};
+		const pr = new PullRequestBuilder().number(794).stack(stack).build();
+		const out = render(
+			<PullRequestContext.Provider value={new PRContext(pr)}>
+				<Overview {...pr} />
+			</PullRequestContext.Provider>,
+		);
+		assert(out.getByText('Merge stack (1 pull request)'));
 	});
 
 	it('does not offer to merge a stack without write permission', function () {