mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
* fix(github): only auto-close issue when all linked PRs have resolved Previously, the webhook handler unconditionally moved an issue to `done` as soon as a single linked PR was merged. If a second PR was also linked to the same issue and still open / draft, the issue would close before the work was actually finished. Add `CountOpenSiblingPullRequestsForIssue` and gate the auto-status transition on it: a merged PR advances its linked issues only when no sibling PR linked to the same issue is still in flight. Issues stay put while siblings are open or draft, and the merge that resolves the last in-flight PR is the one that closes the issue. Adds an integration test that opens two PRs against the same issue, merges the first, asserts the issue stays in_progress, then merges the second and asserts the issue advances to done. Co-authored-by: multica-agent <github@multica.ai> * 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> --------- Co-authored-by: multica-agent <github@multica.ai>
673 lines
23 KiB
Go
673 lines
23 KiB
Go
package handler
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"crypto/hmac"
|
|
"crypto/sha256"
|
|
"encoding/hex"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"reflect"
|
|
"testing"
|
|
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
func TestExtractIdentifiers(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
in []string
|
|
want []string
|
|
}{
|
|
{
|
|
name: "branch_name",
|
|
in: []string{"", "", "mul-1510/fix-login"},
|
|
want: []string{"MUL-1510"},
|
|
},
|
|
{
|
|
name: "title_and_body",
|
|
in: []string{"Fix MUL-82", "Closes MUL-1510 and ABC-7", ""},
|
|
want: []string{"MUL-82", "MUL-1510", "ABC-7"},
|
|
},
|
|
{
|
|
name: "dedupe_across_fields",
|
|
in: []string{"MUL-1", "MUL-1 again", "mul-1/branch"},
|
|
want: []string{"MUL-1"},
|
|
},
|
|
{
|
|
name: "ignore_email_and_versions",
|
|
in: []string{"reply@user-1 v1.2-3 here", "", ""},
|
|
// Word-boundary regex still matches "user-1"; identifier prefix is
|
|
// any 2..10 letters/digits, so this is intentional. The downstream
|
|
// workspace prefix check in lookupIssueByIdentifier filters it.
|
|
want: []string{"USER-1"},
|
|
},
|
|
{
|
|
name: "no_match",
|
|
in: []string{"plain text", "no idents", ""},
|
|
want: []string{},
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := extractIdentifiers(tc.in...)
|
|
if len(got) == 0 && len(tc.want) == 0 {
|
|
return
|
|
}
|
|
if !reflect.DeepEqual(got, tc.want) {
|
|
t.Errorf("extractIdentifiers() = %v, want %v", got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestDerivePRState(t *testing.T) {
|
|
cases := []struct {
|
|
state string
|
|
draft bool
|
|
merged bool
|
|
want string
|
|
}{
|
|
{"open", false, false, "open"},
|
|
{"open", true, false, "draft"},
|
|
{"closed", false, false, "closed"},
|
|
{"closed", false, true, "merged"},
|
|
{"closed", true, true, "merged"}, // merged trumps draft
|
|
}
|
|
for _, tc := range cases {
|
|
got := derivePRState(tc.state, tc.draft, tc.merged)
|
|
if got != tc.want {
|
|
t.Errorf("derivePRState(%q, draft=%v, merged=%v) = %q, want %q",
|
|
tc.state, tc.draft, tc.merged, got, tc.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestVerifyWebhookSignature(t *testing.T) {
|
|
secret := "shared-secret"
|
|
body := []byte(`{"action":"opened"}`)
|
|
mac := hmac.New(sha256.New, []byte(secret))
|
|
mac.Write(body)
|
|
good := "sha256=" + hex.EncodeToString(mac.Sum(nil))
|
|
|
|
if !verifyWebhookSignature(secret, good, body) {
|
|
t.Error("expected valid signature to verify")
|
|
}
|
|
if verifyWebhookSignature(secret, "sha256=deadbeef", body) {
|
|
t.Error("expected bad hex to fail")
|
|
}
|
|
if verifyWebhookSignature(secret, "", body) {
|
|
t.Error("expected empty header to fail")
|
|
}
|
|
if verifyWebhookSignature(secret, "sha1=whatever", body) {
|
|
t.Error("expected non-sha256 prefix to fail")
|
|
}
|
|
if verifyWebhookSignature("other-secret", good, body) {
|
|
t.Error("expected wrong secret to fail")
|
|
}
|
|
}
|
|
|
|
func TestStateRoundTrip(t *testing.T) {
|
|
t.Setenv("GITHUB_WEBHOOK_SECRET", "test-secret-123")
|
|
wsID := "11111111-2222-3333-4444-555555555555"
|
|
|
|
tok, err := signState(wsID)
|
|
if err != nil {
|
|
t.Fatalf("signState: %v", err)
|
|
}
|
|
got, ok := verifyState(tok)
|
|
if !ok {
|
|
t.Fatal("verifyState rejected a freshly-signed token")
|
|
}
|
|
if got != wsID {
|
|
t.Errorf("verifyState() = %q, want %q", got, wsID)
|
|
}
|
|
|
|
// Tampering with the workspace portion must fail (signature is bound
|
|
// to it). Replace the leading UUID's first hex digit.
|
|
tampered := "01111111" + tok[8:]
|
|
if _, ok := verifyState(tampered); ok {
|
|
t.Error("tampered state token should fail to verify")
|
|
}
|
|
|
|
// Wrong secret rejects.
|
|
t.Setenv("GITHUB_WEBHOOK_SECRET", "different")
|
|
if _, ok := verifyState(tok); ok {
|
|
t.Error("token signed with old secret should fail under a new one")
|
|
}
|
|
}
|
|
|
|
func TestSignStateRequiresSecret(t *testing.T) {
|
|
t.Setenv("GITHUB_WEBHOOK_SECRET", "")
|
|
if _, err := signState("ws"); err == nil {
|
|
t.Error("signState should error when secret is unset")
|
|
}
|
|
}
|
|
|
|
// TestWebhook_MergedPR_AdvancesLinkedIssueToDone exercises the end-to-end
|
|
// auto-link + merge-sync path: install a workspace, fire a `pull_request`
|
|
// webhook with the issue identifier in the title, and verify (a) the PR row
|
|
// is upserted, (b) it is linked to the issue, (c) the issue transitions to
|
|
// 'done'. The system actor on that issue:updated event is what previously
|
|
// panicked the activity / notification listeners — having this test pass
|
|
// while listeners are wired up is the regression guard.
|
|
func TestWebhook_MergedPR_AdvancesLinkedIssueToDone(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("handler test fixture not initialized (no DB?)")
|
|
}
|
|
ctx := context.Background()
|
|
secret := "merge-sync-test-secret"
|
|
t.Setenv("GITHUB_WEBHOOK_SECRET", secret)
|
|
|
|
// Seed an issue we expect the webhook to close out.
|
|
w := httptest.NewRecorder()
|
|
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
|
|
"title": "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)
|
|
})
|
|
|
|
// Wire up an installation row for the webhook to attribute to.
|
|
const installationID int64 = 99887766
|
|
if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
|
|
WorkspaceID: parseUUID(testWorkspaceID),
|
|
InstallationID: installationID,
|
|
AccountLogin: "merge-sync-acct",
|
|
AccountType: "User",
|
|
}); err != nil {
|
|
t.Fatalf("CreateGitHubInstallation: %v", err)
|
|
}
|
|
|
|
// Build a minimal pull_request webhook payload referencing the issue.
|
|
body := map[string]any{
|
|
"action": "closed",
|
|
"pull_request": map[string]any{
|
|
"number": 1234,
|
|
"html_url": "https://github.com/acme/widget/pull/1234",
|
|
"title": "Fix login " + created.Identifier,
|
|
"body": "",
|
|
"state": "closed",
|
|
"draft": false,
|
|
"merged": true,
|
|
"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/login"},
|
|
"user": map[string]any{"login": "octocat", "avatar_url": ""},
|
|
},
|
|
"repository": map[string]any{
|
|
"name": "widget",
|
|
"owner": map[string]any{"login": "acme"},
|
|
},
|
|
"installation": map[string]any{"id": installationID},
|
|
}
|
|
raw, _ := json.Marshal(body)
|
|
mac := hmac.New(sha256.New, []byte(secret))
|
|
mac.Write(raw)
|
|
sig := "sha256=" + hex.EncodeToString(mac.Sum(nil))
|
|
|
|
w = httptest.NewRecorder()
|
|
req2 := httptest.NewRequest("POST", "/api/webhooks/github", bytes.NewReader(raw))
|
|
req2.Header.Set("X-GitHub-Event", "pull_request")
|
|
req2.Header.Set("X-Hub-Signature-256", sig)
|
|
testHandler.HandleGitHubWebhook(w, req2)
|
|
if w.Code != http.StatusAccepted {
|
|
t.Fatalf("webhook: expected 202, got %d (%s)", w.Code, w.Body.String())
|
|
}
|
|
|
|
// Verify PR row + link + issue status.
|
|
pr, err := testHandler.Queries.GetGitHubPullRequest(ctx, db.GetGitHubPullRequestParams{
|
|
WorkspaceID: parseUUID(testWorkspaceID),
|
|
RepoOwner: "acme",
|
|
RepoName: "widget",
|
|
PrNumber: 1234,
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("GetGitHubPullRequest: %v", err)
|
|
}
|
|
if pr.State != "merged" {
|
|
t.Errorf("expected pr state merged, got %q", pr.State)
|
|
}
|
|
|
|
linked, err := testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID))
|
|
if err != nil {
|
|
t.Fatalf("ListPullRequestsByIssue: %v", err)
|
|
}
|
|
if len(linked) != 1 {
|
|
t.Fatalf("expected 1 linked PR, got %d", len(linked))
|
|
}
|
|
|
|
updated, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
|
|
if err != nil {
|
|
t.Fatalf("GetIssue: %v", err)
|
|
}
|
|
if updated.Status != "done" {
|
|
t.Errorf("expected issue status 'done', got %q", updated.Status)
|
|
}
|
|
}
|
|
|
|
// TestWebhook_MergedPR_PreservesCancelled guards the "do not stomp cancelled"
|
|
// rule: cancelling an issue then merging a linked PR must leave the issue
|
|
// cancelled.
|
|
func TestWebhook_MergedPR_PreservesCancelled(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("handler test fixture not initialized (no DB?)")
|
|
}
|
|
ctx := context.Background()
|
|
secret := "cancelled-secret"
|
|
t.Setenv("GITHUB_WEBHOOK_SECRET", secret)
|
|
|
|
w := httptest.NewRecorder()
|
|
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
|
|
"title": "Already cancelled",
|
|
"status": "cancelled",
|
|
})
|
|
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 issue WHERE id = $1`, created.ID)
|
|
})
|
|
|
|
const installationID int64 = 11223344
|
|
if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
|
|
WorkspaceID: parseUUID(testWorkspaceID),
|
|
InstallationID: installationID,
|
|
AccountLogin: "cancelled-acct",
|
|
AccountType: "User",
|
|
}); err != nil {
|
|
t.Fatalf("CreateGitHubInstallation: %v", err)
|
|
}
|
|
|
|
body, _ := json.Marshal(map[string]any{
|
|
"action": "closed",
|
|
"pull_request": map[string]any{
|
|
"number": 7, "html_url": "https://x", "title": "Closes " + created.Identifier,
|
|
"state": "closed", "merged": true, "draft": false,
|
|
"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": "x"}, "user": map[string]any{"login": "u"},
|
|
},
|
|
"repository": map[string]any{"name": "r", "owner": map[string]any{"login": "o"}},
|
|
"installation": map[string]any{"id": installationID},
|
|
})
|
|
mac := hmac.New(sha256.New, []byte(secret))
|
|
mac.Write(body)
|
|
sig := "sha256=" + hex.EncodeToString(mac.Sum(nil))
|
|
|
|
w = httptest.NewRecorder()
|
|
req2 := httptest.NewRequest("POST", "/api/webhooks/github", bytes.NewReader(body))
|
|
req2.Header.Set("X-GitHub-Event", "pull_request")
|
|
req2.Header.Set("X-Hub-Signature-256", sig)
|
|
testHandler.HandleGitHubWebhook(w, req2)
|
|
|
|
updated, err := testHandler.Queries.GetIssue(ctx, parseUUID(created.ID))
|
|
if err != nil {
|
|
t.Fatalf("GetIssue: %v", err)
|
|
}
|
|
if updated.Status != "cancelled" {
|
|
t.Errorf("expected status to remain 'cancelled', got %q", updated.Status)
|
|
}
|
|
}
|
|
|
|
// TestWebhook_UninstallReturnsWorkspaceForBroadcast guards #4: the uninstall
|
|
// path must look up the workspace_id BEFORE deleting the row so the
|
|
// resulting `github_installation:deleted` event is broadcast scoped to that
|
|
// workspace (the realtime listener drops events with empty workspace_id).
|
|
func TestWebhook_UninstallReturnsWorkspaceForBroadcast(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("handler test fixture not initialized (no DB?)")
|
|
}
|
|
ctx := context.Background()
|
|
const installationID int64 = 55443322
|
|
|
|
if _, err := testHandler.Queries.CreateGitHubInstallation(ctx, db.CreateGitHubInstallationParams{
|
|
WorkspaceID: parseUUID(testWorkspaceID),
|
|
InstallationID: installationID,
|
|
AccountLogin: "uninstall-test",
|
|
AccountType: "User",
|
|
}); err != nil {
|
|
t.Fatalf("CreateGitHubInstallation: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(ctx, `DELETE FROM github_installation WHERE workspace_id = $1`, testWorkspaceID)
|
|
})
|
|
|
|
deleted, err := testHandler.Queries.DeleteGitHubInstallationByInstallationID(ctx, installationID)
|
|
if err != nil {
|
|
t.Fatalf("DeleteGitHubInstallationByInstallationID: %v", err)
|
|
}
|
|
if uuidToString(deleted.WorkspaceID) != testWorkspaceID {
|
|
t.Errorf("expected returned workspace_id %s, got %s", testWorkspaceID, uuidToString(deleted.WorkspaceID))
|
|
}
|
|
// Re-deleting must surface ErrNoRows so the handler can short-circuit
|
|
// the broadcast (and not panic).
|
|
if _, err := testHandler.Queries.DeleteGitHubInstallationByInstallationID(ctx, installationID); err == nil {
|
|
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)
|
|
}
|
|
}
|