diff --git a/src/db/issue-store.ts b/src/db/issue-store.ts index d4daa4fa..2c0dc607 100644 --- a/src/db/issue-store.ts +++ b/src/db/issue-store.ts @@ -158,11 +158,25 @@ export class IssueStore { return row ? mapIssueRow(row) : undefined; } + getIssueByProjectKey(projectId: string, issueKey: string): IssueRecord | undefined { + const row = this.connection + .prepare("SELECT * FROM issues WHERE project_id = ? AND issue_key = ? ORDER BY updated_at DESC LIMIT 1") + .get(projectId, issueKey) as Record | undefined; + return row ? mapIssueRow(row) : undefined; + } + getIssueByBranch(branchName: string): IssueRecord | undefined { const row = this.connection.prepare("SELECT * FROM issues WHERE branch_name = ?").get(branchName) as Record | undefined; return row ? mapIssueRow(row) : undefined; } + getIssueByProjectBranch(projectId: string, branchName: string): IssueRecord | undefined { + const row = this.connection + .prepare("SELECT * FROM issues WHERE project_id = ? AND branch_name = ? ORDER BY updated_at DESC LIMIT 1") + .get(projectId, branchName) as Record | undefined; + return row ? mapIssueRow(row) : undefined; + } + getIssueByPrNumber(prNumber: number): IssueRecord | undefined { const row = this.connection.prepare("SELECT * FROM issues WHERE pr_number = ?").get(prNumber) as Record | undefined; return row ? mapIssueRow(row) : undefined; diff --git a/src/github-webhook-issue-resolution.ts b/src/github-webhook-issue-resolution.ts index 33bb3821..75d6f52d 100644 --- a/src/github-webhook-issue-resolution.ts +++ b/src/github-webhook-issue-resolution.ts @@ -14,17 +14,26 @@ export function resolveGitHubWebhookIssue( event: NormalizedGitHubEvent, ): GitHubWebhookIssueResolution | undefined { if (event.prNumber !== undefined) { - const byPr = db.issues.getIssueByPrNumber(event.prNumber); - if (byPr && byPr.projectId === project.id) { + const byPr = db.issues.getIssueByProjectPrNumber(project.id, event.prNumber); + if (byPr) { return { issue: byPr, linkedBy: "pr" }; } } - const byBranch = db.issues.getIssueByBranch(event.branchName); - if (byBranch && byBranch.projectId === project.id) { + const byBranch = db.issues.getIssueByProjectBranch(project.id, event.branchName); + if (byBranch) { return { issue: byBranch, linkedBy: "branch" }; } + // An issue-key mention is discovery evidence, not PR ownership. Only the + // opening event may establish that association; later review, check, close, + // and merge events must resolve through the persisted repo-local PR or + // branch identity. Otherwise a foreign PR that merely references an issue + // can mutate or complete the referenced issue's workflow. + if (event.triggerEvent !== "pr_opened") { + return undefined; + } + const byIssueKey = resolveGitHubWebhookIssueByKey(db, project, event); if (byIssueKey) { return { issue: byIssueKey, linkedBy: "issue_key" }; @@ -61,11 +70,9 @@ export function resolveGitHubWebhookIssueByKey( if (!issueKey) { return undefined; } - const issue = db.issues.getIssueByKey(issueKey); - return issue?.projectId === project.id ? issue : undefined; + return db.issues.getIssueByProjectKey(project.id, issueKey); } function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } - diff --git a/test/github-runtime-contract.test.ts b/test/github-runtime-contract.test.ts index a6be142e..7b7411d6 100644 --- a/test/github-runtime-contract.test.ts +++ b/test/github-runtime-contract.test.ts @@ -340,6 +340,8 @@ function buildTerminalPrPayload(params: { prNumber: number; merged: boolean; prAuthorLogin?: string; + prTitle?: string; + prBody?: string; }): string { return JSON.stringify({ action: params.action, @@ -347,6 +349,8 @@ function buildTerminalPrPayload(params: { pull_request: { number: params.prNumber, html_url: `https://github.com/owner/repo/pull/${params.prNumber}`, + title: params.prTitle ?? `PR for #${params.prNumber}`, + body: params.prBody ?? "", state: "closed", merged: params.merged, user: { login: params.prAuthorLogin ?? "patchrelay[bot]" }, @@ -356,6 +360,53 @@ function buildTerminalPrPayload(params: { }); } +test("a merged foreign PR that mentions a tracked issue cannot complete it", async () => { + const baseDir = mkdtempSync(path.join(tmpdir(), "patchrelay-github-runtime-foreign-merge-")); + try { + const { db, enqueueCalls, handler } = createHandler(baseDir); + db.upsertIssue({ + projectId: "usertold", + linearIssueId: "issue-owned-pr", + issueKey: "USE-145", + branchName: "use/owned-pr", + prNumber: 194, + prState: "open", + prHeadSha: "owned-sha", + prReviewState: "changes_requested", + prAuthorLogin: "patchrelay[bot]", + workflowOutcome: undefined, + currentLinearState: "In Review", + currentLinearStateType: "started", + }); + + await handler.processGitHubWebhookEvent({ + eventType: "pull_request", + rawBody: buildTerminalPrPayload({ + action: "closed", + branch: "foreign/landing-page", + headSha: "foreign-sha", + prNumber: 192, + merged: true, + prBody: "Related follow-up: USE-145 should add the CLI command.", + }), + }); + + const issue = db.getIssue("usertold", "issue-owned-pr"); + assert.equal(issue?.workflowOutcome, undefined); + assert.equal(issue?.prNumber, 194); + assert.equal(issue?.prState, "open"); + assert.equal(issue?.currentLinearState, "In Review"); + assert.deepEqual(enqueueCalls, []); + assert.equal( + db.workflowObservations.listObservations("usertold", "issue-owned-pr") + .some((observation) => observation.type === "github.pr_merged"), + false, + ); + } finally { + rmSync(baseDir, { recursive: true, force: true }); + } +}); + function buildLabeledPrPayload(params: { branch: string; headSha: string; diff --git a/test/github-webhook-issue-resolution.test.ts b/test/github-webhook-issue-resolution.test.ts index 161395c3..6f953d3a 100644 --- a/test/github-webhook-issue-resolution.test.ts +++ b/test/github-webhook-issue-resolution.test.ts @@ -101,23 +101,29 @@ test("resolveGitHubWebhookIssue prefers PR, then branch, then issue key", () => assert.ok(project); const prResolved = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "review_approved", repoFullName: "owner/repo", branchName: "different", + headSha: "sha-pr", prNumber: 101, } as NormalizedGitHubEvent); assert.equal(prResolved?.issue.linearIssueId, "issue-pr"); assert.equal(prResolved?.linkedBy, "pr"); const branchResolved = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "check_passed", repoFullName: "owner/repo", branchName: "use/branch-match", + headSha: "sha-branch", } as NormalizedGitHubEvent); assert.equal(branchResolved?.issue.linearIssueId, "issue-branch"); assert.equal(branchResolved?.linkedBy, "branch"); const keyResolved = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "pr_opened", repoFullName: "owner/repo", branchName: "feature/USE-12-refactor", + headSha: "sha-key", prBody: "Fixes USE-12", } as NormalizedGitHubEvent); assert.equal(keyResolved?.issue.linearIssueId, "issue-key"); @@ -126,3 +132,134 @@ test("resolveGitHubWebhookIssue prefers PR, then branch, then issue key", () => rmSync(baseDir, { recursive: true, force: true }); } }); + +test("resolveGitHubWebhookIssue scopes equal PR numbers and branch names to the repository", () => { + const baseDir = mkdtempSync(path.join(tmpdir(), "patchrelay-issue-resolution-scope-")); + try { + const config = createConfig(baseDir); + const db = new PatchRelayDatabase(config.database.path, config.database.wal); + db.initializeSchema(); + db.upsertIssue({ + projectId: "another/repo", + linearIssueId: "other-pr", + issueKey: "USE-10", + branchName: "shared/branch", + prNumber: 101, + }); + db.upsertIssue({ + projectId: "owner/repo", + linearIssueId: "owner-pr", + issueKey: "USE-10", + branchName: "owner/pr", + prNumber: 101, + }); + db.upsertIssue({ + projectId: "owner/repo", + linearIssueId: "owner-branch", + issueKey: "USE-11", + branchName: "shared/branch", + }); + + const project = config.projects[0]; + assert.ok(project); + + const prResolved = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "review_approved", + repoFullName: "owner/repo", + branchName: "unrelated", + headSha: "sha-pr", + prNumber: 101, + }); + assert.equal(prResolved?.issue.linearIssueId, "owner-pr"); + assert.equal(prResolved?.linkedBy, "pr"); + + const branchResolved = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "check_passed", + repoFullName: "owner/repo", + branchName: "shared/branch", + headSha: "sha-branch", + }); + assert.equal(branchResolved?.issue.linearIssueId, "owner-branch"); + assert.equal(branchResolved?.linkedBy, "branch"); + + const keyResolved = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "pr_opened", + repoFullName: "owner/repo", + branchName: "feature/USE-10", + headSha: "sha-key", + prNumber: 102, + prBody: "Fixes USE-10", + }); + assert.equal(keyResolved?.issue.linearIssueId, "owner-pr"); + assert.equal(keyResolved?.linkedBy, "issue_key"); + } finally { + rmSync(baseDir, { recursive: true, force: true }); + } +}); + +test("only pr_opened may establish PR ownership from an issue-key mention", () => { + const baseDir = mkdtempSync(path.join(tmpdir(), "patchrelay-issue-resolution-ownership-")); + try { + const config = createConfig(baseDir); + const db = new PatchRelayDatabase(config.database.path, config.database.wal); + db.initializeSchema(); + db.upsertIssue({ + projectId: "owner/repo", + linearIssueId: "owned-issue", + issueKey: "USE-12", + branchName: "use/owned-pr", + prNumber: 194, + }); + db.upsertIssue({ + projectId: "owner/repo", + linearIssueId: "unlinked-issue", + issueKey: "USE-13", + }); + + const project = config.projects[0]; + assert.ok(project); + + for (const triggerEvent of [ + "pr_merged", + "pr_closed", + "pr_synchronize", + "review_approved", + "review_changes_requested", + "check_passed", + "check_failed", + ] as const) { + const resolved = resolveGitHubWebhookIssue(db, project, { + triggerEvent, + repoFullName: "owner/repo", + branchName: "foreign/branch", + headSha: "foreign-sha", + prNumber: 192, + prBody: "Related: USE-12 and implementation notes", + }); + assert.equal(resolved, undefined, `${triggerEvent} must not claim an issue by mention`); + } + + const terminalForUnlinkedIssue = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "pr_merged", + repoFullName: "owner/repo", + branchName: "foreign/unlinked", + headSha: "foreign-unlinked-sha", + prNumber: 193, + prBody: "Fixes USE-13", + }); + assert.equal(terminalForUnlinkedIssue, undefined); + + const opened = resolveGitHubWebhookIssue(db, project, { + triggerEvent: "pr_opened", + repoFullName: "owner/repo", + branchName: "feature/USE-13", + headSha: "new-sha", + prNumber: 195, + prBody: "Fixes USE-13", + }); + assert.equal(opened?.issue.linearIssueId, "unlinked-issue"); + assert.equal(opened?.linkedBy, "issue_key"); + } finally { + rmSync(baseDir, { recursive: true, force: true }); + } +});