fix(github): re-evaluate auto-close on closed-without-merge events too

GPT-Boy review on #2470: gating only the `state == "merged"` branch left
one ordering hole. PR-A merges first → issue stays in_progress because
PR-B is open; PR-B later closes WITHOUT merging → no event ever re-runs
the auto-close check, so the issue is stuck in_progress.

Generalise the trigger to every terminal PR event (`merged` or `closed`)
and advance the issue only when:
- the issue is not already terminal (done / cancelled);
- no sibling PR is still in flight (open / draft);
- at least one linked PR — current or sibling — actually merged.

Rule (3) preserves "user closed every PR without merging → leave the
issue alone": if no work was delivered, the user decides what to do.

Replace `CountOpenSiblingPullRequestsForIssue` with
`GetSiblingPullRequestStateCountsForIssue`, which returns both the
in-flight count and the merged count in a single roundtrip.

Adds `TestWebhook_ClosedSiblingAfterMerge` (the regression GPT-Boy
flagged) and `TestWebhook_AllClosedWithoutMerge` (the negative case
guarding rule 3). Refactors the multi-PR webhook helper out of the
existing two-merge test so all three multi-PR scenarios share it.

Co-authored-by: multica-agent <github@multica.ai>

server/internal/handler/github.go

@@ -578,11 +578,28 @@ func (h *Handler) handlePullRequestEvent(ctx context.Context, body []byte) {
 		}
 		linkedIssueIDs = append(linkedIssueIDs, uuidToString(issue.ID))
 
-		// PR merged → move issue to done if it isn't already terminal.
-		// We deliberately avoid clobbering 'cancelled' since that signals
-		// the user explicitly abandoned the work.
-		if state == "merged" && issue.Status != "done" && issue.Status != "cancelled" {
-			h.advanceIssueToDone(ctx, issue, workspaceID)
+		// A terminal PR event (`merged` or `closed`) may be the moment the
+		// last in-flight sibling resolves, so we re-evaluate the issue on
+		// both. We advance the issue to done when:
+		//   1. the issue isn't already terminal (`done` / `cancelled`);
+		//   2. no sibling PR is still `open` / `draft`;
+		//   3. at least one linked PR (this one or a sibling) is `merged`.
+		// Rule (3) prevents an "all closed-without-merge" sequence from
+		// silently auto-closing the issue — if nothing was ever delivered,
+		// the user should decide what to do manually.
+		if (state == "merged" || state == "closed") && issue.Status != "done" && issue.Status != "cancelled" {
+			counts, err := h.Queries.GetSiblingPullRequestStateCountsForIssue(ctx, db.GetSiblingPullRequestStateCountsForIssueParams{
+				IssueID: issue.ID,
+				ID:      pr.ID,
+			})
+			if err != nil {
+				slog.Warn("github: count sibling pr states failed", "err", err, "issue_id", uuidToString(issue.ID))
+				continue
+			}
+			anyMerged := state == "merged" || counts.MergedCount > 0
+			if counts.OpenCount == 0 && anyMerged {
+				h.advanceIssueToDone(ctx, issue, workspaceID)
+			}
 		}
 	}
 

server/internal/handler/github_test.go

@@ -369,3 +369,304 @@ func TestWebhook_UninstallReturnsWorkspaceForBroadcast(t *testing.T) {
 		t.Error("expected ErrNoRows on second delete, got nil")
 	}
 }
+
+// TestWebhook_MergedPR_WaitsForOpenSibling guards the multi-PR case: when an
+// issue is linked to two PRs and only one is merged, the issue must stay in
+// its current status. Only the merge that resolves the LAST in-flight PR
+// closes the issue.
+func TestWebhook_MergedPR_WaitsForOpenSibling(t *testing.T) {
+	if testHandler == nil {
+		t.Skip("handler test fixture not initialized (no DB?)")
+	}
+	ctx := context.Background()
+	secret := "multi-pr-test-secret"
+	t.Setenv("GITHUB_WEBHOOK_SECRET", secret)
+
+	w := httptest.NewRecorder()
+	req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
+		"title":  "Multi-PR auto-merge test",
+		"status": "in_progress",
+	})
+	testHandler.CreateIssue(w, req)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("CreateIssue: %d %s", w.Code, w.Body.String())
+	}
+	var created IssueResponse
+	json.NewDecoder(w.Body).Decode(&created)
+
+	t.Cleanup(func() {
+		testPool.Exec(ctx, `DELETE FROM issue_pull_request WHERE issue_id = $1`, created.ID)
+		testPool.Exec(ctx, `DELETE FROM github_pull_request WHERE workspace_id = $1`, testWorkspaceID)
+		testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
+		testPool.Exec(ctx, `DELETE FROM activity_log WHERE issue_id = $1`, created.ID)
+		testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, created.ID)
+	})
+
+	const installationID int64 = 55667788
+	if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
+		WorkspaceID:    parseUUID(testWorkspaceID),
+		InstallationID: installationID,
+		AccountLogin:   "multi-pr-acct",
+		AccountType:    "User",
+	}); err != nil {
+		t.Fatalf("CreateGitHubInstallation: %v", err)
+	}
+
+	// Helper to fire one pull_request webhook.
+	fire := func(t *testing.T, repo string, prNumber int32, merged bool) {
+		t.Helper()
+		state := "open"
+		if merged {
+			state = "closed"
+		}
+		payload := map[string]any{
+			"action": state,
+			"pull_request": map[string]any{
+				"number":     prNumber,
+				"html_url":   "https://github.com/acme/" + repo + "/pull/1",
+				"title":      "Fix " + created.Identifier,
+				"body":       "",
+				"state":      state,
+				"draft":      false,
+				"merged":     merged,
+				"merged_at":  "2026-04-29T00:00:00Z",
+				"closed_at":  "2026-04-29T00:00:00Z",
+				"created_at": "2026-04-28T00:00:00Z",
+				"updated_at": "2026-04-29T00:00:00Z",
+				"head":       map[string]any{"ref": "fix/multi"},
+				"user":       map[string]any{"login": "octocat"},
+			},
+			"repository": map[string]any{
+				"name":  repo,
+				"owner": map[string]any{"login": "acme"},
+			},
+			"installation": map[string]any{"id": installationID},
+		}
+		raw, _ := json.Marshal(payload)
+		mac := hmac.New(sha256.New, []byte(secret))
+		mac.Write(raw)
+		sig := "sha256=" + hex.EncodeToString(mac.Sum(nil))
+
+		rec := httptest.NewRecorder()
+		hookReq := httptest.NewRequest("POST", "/api/webhooks/github", bytes.NewReader(raw))
+		hookReq.Header.Set("X-GitHub-Event", "pull_request")
+		hookReq.Header.Set("X-Hub-Signature-256", sig)
+		testHandler.HandleGitHubWebhook(rec, hookReq)
+		if rec.Code != http.StatusAccepted {
+			t.Fatalf("webhook: expected 202, got %d (%s)", rec.Code, rec.Body.String())
+		}
+	}
+
+	// Open PR A and PR B against two repos so the (workspace, owner, repo,
+	// number) uniqueness on github_pull_request leaves room for both.
+	fire(t, "repo-a", 1, false)
+	fire(t, "repo-b", 2, false)
+
+	// Sanity: both linked.
+	linked, err := testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID))
+	if err != nil {
+		t.Fatalf("ListPullRequestsByIssue: %v", err)
+	}
+	if len(linked) != 2 {
+		t.Fatalf("expected 2 linked PRs, got %d", len(linked))
+	}
+
+	// Merge PR A. Issue must stay in_progress because PR B is still open.
+	fire(t, "repo-a", 1, true)
+	issueAfterA, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
+	if err != nil {
+		t.Fatalf("GetIssue: %v", err)
+	}
+	if issueAfterA.Status != "in_progress" {
+		t.Errorf("issue should stay in_progress while sibling PR is open, got %q", issueAfterA.Status)
+	}
+
+	// Now merge PR B. Issue should advance to done — last sibling resolved.
+	fire(t, "repo-b", 2, true)
+	issueAfterB, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
+	if err != nil {
+		t.Fatalf("GetIssue: %v", err)
+	}
+	if issueAfterB.Status != "done" {
+		t.Errorf("expected issue 'done' after every linked PR merged, got %q", issueAfterB.Status)
+	}
+}
+
+// firePullRequestWebhook is a shared helper for the multi-PR tests below: it
+// fires one pull_request webhook for a given repo/number with a target state
+// of open / closed / merged and asserts the handler accepts it. Centralizing
+// here keeps the per-scenario tests focused on assertions.
+func firePullRequestWebhook(t *testing.T, secret, identifier string, installationID int64, repo string, prNumber int32, prState string) {
+	t.Helper()
+	state := "open"
+	merged := false
+	switch prState {
+	case "merged":
+		state = "closed"
+		merged = true
+	case "closed":
+		state = "closed"
+	}
+	payload := map[string]any{
+		"action": state,
+		"pull_request": map[string]any{
+			"number":     prNumber,
+			"html_url":   "https://github.com/acme/" + repo + "/pull/1",
+			"title":      "Fix " + identifier,
+			"body":       "",
+			"state":      state,
+			"draft":      false,
+			"merged":     merged,
+			"merged_at":  "2026-04-29T00:00:00Z",
+			"closed_at":  "2026-04-29T00:00:00Z",
+			"created_at": "2026-04-28T00:00:00Z",
+			"updated_at": "2026-04-29T00:00:00Z",
+			"head":       map[string]any{"ref": "fix/multi"},
+			"user":       map[string]any{"login": "octocat"},
+		},
+		"repository": map[string]any{
+			"name":  repo,
+			"owner": map[string]any{"login": "acme"},
+		},
+		"installation": map[string]any{"id": installationID},
+	}
+	raw, _ := json.Marshal(payload)
+	mac := hmac.New(sha256.New, []byte(secret))
+	mac.Write(raw)
+	sig := "sha256=" + hex.EncodeToString(mac.Sum(nil))
+
+	rec := httptest.NewRecorder()
+	hookReq := httptest.NewRequest("POST", "/api/webhooks/github", bytes.NewReader(raw))
+	hookReq.Header.Set("X-GitHub-Event", "pull_request")
+	hookReq.Header.Set("X-Hub-Signature-256", sig)
+	testHandler.HandleGitHubWebhook(rec, hookReq)
+	if rec.Code != http.StatusAccepted {
+		t.Fatalf("webhook %s pr=%d state=%s: expected 202, got %d (%s)",
+			repo, prNumber, prState, rec.Code, rec.Body.String())
+	}
+}
+
+// TestWebhook_ClosedSiblingAfterMerge guards the ordering bug GPT-Boy flagged
+// on PR #2470: PR-A merges first (issue stays in_progress because PR-B is
+// open), then PR-B closes WITHOUT merging. Because PR-A already delivered the
+// work, that close event must re-evaluate the issue and advance it to done.
+func TestWebhook_ClosedSiblingAfterMerge(t *testing.T) {
+	if testHandler == nil {
+		t.Skip("handler test fixture not initialized (no DB?)")
+	}
+	ctx := context.Background()
+	secret := "closed-sibling-secret"
+	t.Setenv("GITHUB_WEBHOOK_SECRET", secret)
+
+	w := httptest.NewRecorder()
+	req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
+		"title":  "Closed sibling after merge",
+		"status": "in_progress",
+	})
+	testHandler.CreateIssue(w, req)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("CreateIssue: %d %s", w.Code, w.Body.String())
+	}
+	var created IssueResponse
+	json.NewDecoder(w.Body).Decode(&created)
+
+	t.Cleanup(func() {
+		testPool.Exec(ctx, `DELETE FROM issue_pull_request WHERE issue_id = $1`, created.ID)
+		testPool.Exec(ctx, `DELETE FROM github_pull_request WHERE workspace_id = $1`, testWorkspaceID)
+		testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
+		testPool.Exec(ctx, `DELETE FROM activity_log WHERE issue_id = $1`, created.ID)
+		testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, created.ID)
+	})
+
+	const installationID int64 = 66778899
+	if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
+		WorkspaceID:    parseUUID(testWorkspaceID),
+		InstallationID: installationID,
+		AccountLogin:   "closed-sibling-acct",
+		AccountType:    "User",
+	}); err != nil {
+		t.Fatalf("CreateGitHubInstallation: %v", err)
+	}
+
+	// Open both PRs.
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-a", 1, "open")
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-b", 2, "open")
+
+	// Merge PR A — issue must stay in_progress because PR B is still open.
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-a", 1, "merged")
+	intermediate, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
+	if err != nil {
+		t.Fatalf("GetIssue: %v", err)
+	}
+	if intermediate.Status != "in_progress" {
+		t.Fatalf("issue should stay in_progress while sibling PR open, got %q", intermediate.Status)
+	}
+
+	// Close PR B WITHOUT merging — issue should now advance to done because
+	// PR-A's merge already delivered the work.
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-b", 2, "closed")
+	final, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
+	if err != nil {
+		t.Fatalf("GetIssue: %v", err)
+	}
+	if final.Status != "done" {
+		t.Errorf("expected issue 'done' after sibling closed-without-merge follows a prior merge, got %q", final.Status)
+	}
+}
+
+// TestWebhook_AllClosedWithoutMerge guards the "nothing was delivered" path:
+// two PRs both close without merging. We must NOT auto-close the issue —
+// closed-without-merge alone is not evidence the work landed, and the user
+// should decide what to do.
+func TestWebhook_AllClosedWithoutMerge(t *testing.T) {
+	if testHandler == nil {
+		t.Skip("handler test fixture not initialized (no DB?)")
+	}
+	ctx := context.Background()
+	secret := "all-closed-secret"
+	t.Setenv("GITHUB_WEBHOOK_SECRET", secret)
+
+	w := httptest.NewRecorder()
+	req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
+		"title":  "All closed no merge",
+		"status": "in_progress",
+	})
+	testHandler.CreateIssue(w, req)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("CreateIssue: %d %s", w.Code, w.Body.String())
+	}
+	var created IssueResponse
+	json.NewDecoder(w.Body).Decode(&created)
+
+	t.Cleanup(func() {
+		testPool.Exec(ctx, `DELETE FROM issue_pull_request WHERE issue_id = $1`, created.ID)
+		testPool.Exec(ctx, `DELETE FROM github_pull_request WHERE workspace_id = $1`, testWorkspaceID)
+		testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
+		testPool.Exec(ctx, `DELETE FROM activity_log WHERE issue_id = $1`, created.ID)
+		testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, created.ID)
+	})
+
+	const installationID int64 = 77889900
+	if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
+		WorkspaceID:    parseUUID(testWorkspaceID),
+		InstallationID: installationID,
+		AccountLogin:   "all-closed-acct",
+		AccountType:    "User",
+	}); err != nil {
+		t.Fatalf("CreateGitHubInstallation: %v", err)
+	}
+
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-a", 1, "open")
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-b", 2, "open")
+
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-a", 1, "closed")
+	firePullRequestWebhook(t, secret, created.Identifier, installationID, "repo-b", 2, "closed")
+
+	final, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
+	if err != nil {
+		t.Fatalf("GetIssue: %v", err)
+	}
+	if final.Status != "in_progress" {
+		t.Errorf("issue must stay in_progress when no linked PR ever merged, got %q", final.Status)
+	}
+}

server/pkg/db/generated/github.sql.go

@@ -178,6 +178,39 @@ func (q *Queries) GetGitHubPullRequest(ctx context.Context, arg GetGitHubPullReq
 	return i, err
 }
 
+const getSiblingPullRequestStateCountsForIssue = `-- name: GetSiblingPullRequestStateCountsForIssue :one
+SELECT
+    COALESCE(SUM(CASE WHEN pr.state IN ('open', 'draft') THEN 1 ELSE 0 END), 0)::bigint AS open_count,
+    COALESCE(SUM(CASE WHEN pr.state = 'merged' THEN 1 ELSE 0 END), 0)::bigint AS merged_count
+FROM github_pull_request pr
+JOIN issue_pull_request ipr ON ipr.pull_request_id = pr.id
+WHERE ipr.issue_id = $1
+  AND pr.id <> $2
+`
+
+type GetSiblingPullRequestStateCountsForIssueParams struct {
+	IssueID pgtype.UUID `json:"issue_id"`
+	ID      pgtype.UUID `json:"id"`
+}
+
+type GetSiblingPullRequestStateCountsForIssueRow struct {
+	OpenCount   int64 `json:"open_count"`
+	MergedCount int64 `json:"merged_count"`
+}
+
+// Returns, for the PRs linked to an issue excluding one PR by id (the PR
+// currently being processed by the webhook handler), how many are still in
+// flight (open or draft) and how many have already merged. The webhook
+// handler combines these with the current event's state to decide whether
+// to auto-advance the issue: the issue moves to done only when there is no
+// in-flight sibling AND at least one linked PR (current or sibling) merged.
+func (q *Queries) GetSiblingPullRequestStateCountsForIssue(ctx context.Context, arg GetSiblingPullRequestStateCountsForIssueParams) (GetSiblingPullRequestStateCountsForIssueRow, error) {
+	row := q.db.QueryRow(ctx, getSiblingPullRequestStateCountsForIssue, arg.IssueID, arg.ID)
+	var i GetSiblingPullRequestStateCountsForIssueRow
+	err := row.Scan(&i.OpenCount, &i.MergedCount)
+	return i, err
+}
+
 const linkIssueToPullRequest = `-- name: LinkIssueToPullRequest :exec
 
 INSERT INTO issue_pull_request (

server/pkg/db/queries/github.sql

@@ -80,6 +80,21 @@ ORDER BY pr.pr_created_at DESC;
 SELECT issue_id FROM issue_pull_request
 WHERE pull_request_id = $1;
 
+-- name: GetSiblingPullRequestStateCountsForIssue :one
+-- Returns, for the PRs linked to an issue excluding one PR by id (the PR
+-- currently being processed by the webhook handler), how many are still in
+-- flight (open or draft) and how many have already merged. The webhook
+-- handler combines these with the current event's state to decide whether
+-- to auto-advance the issue: the issue moves to done only when there is no
+-- in-flight sibling AND at least one linked PR (current or sibling) merged.
+SELECT
+    COALESCE(SUM(CASE WHEN pr.state IN ('open', 'draft') THEN 1 ELSE 0 END), 0)::bigint AS open_count,
+    COALESCE(SUM(CASE WHEN pr.state = 'merged' THEN 1 ELSE 0 END), 0)::bigint AS merged_count
+FROM github_pull_request pr
+JOIN issue_pull_request ipr ON ipr.pull_request_id = pr.id
+WHERE ipr.issue_id = $1
+  AND pr.id <> $2;
+
 -- =====================
 -- Issue ↔ Pull Request link
 -- =====================