diff --git a/src/services/github-webhook.ts b/src/services/github-webhook.ts index 1d5fbe9..be040d2 100644 --- a/src/services/github-webhook.ts +++ b/src/services/github-webhook.ts @@ -107,6 +107,29 @@ export function refFromText(text: unknown): string | null { return REF_RE.exec(text)?.[1] ?? null; } +/** + * EVERY ref the given strings name, deduped, first-seen order (SYD-274). + * + * `refFromText` deliberately takes only the first match, because exactly one + * ref can own the activity-feed line. But one PR routinely carries several + * issues' work — `0ae22a9` closed SYD-243 and SYD-244 under SYD-242's PR — and + * for the refs after the first, the first-match rule meant the board held + * *nothing at all*: no event, no link, no trace the PR existed. Their + * `done_without_merged_pr` warnings then had no evidence to reach, which is + * what left them lit with no path to resolution but archaeology. + * + * Same regex, same trust: what this feeds is the `references` suggestion, + * which gates nothing and proves nothing. + */ +export function refsFromText(texts: unknown[]): string[] { + const seen = new Set(); + for (const text of texts) { + if (typeof text !== "string") continue; + for (const [, ref] of text.matchAll(new RegExp(REF_RE, "g"))) seen.add(ref); + } + return [...seen]; +} + function resolveRef(branchCandidates: unknown[], textCandidates: unknown[] = []): string | null { for (const c of branchCandidates) { const ref = refFromBranch(c); @@ -398,6 +421,46 @@ function handlePullRequest(db: Db, rawPayload: unknown, repo: string | null): Gi } } + // SIBLING SUGGESTIONS (SYD-274). The display path above serves exactly one + // ref — the first the text names. Every other ref the PR names got nothing, + // so an issue whose work landed under a sibling's PR (SYD-243 and SYD-244 + // under SYD-242's, merged as 0ae22a9) showed no trace of that PR anywhere, + // and its done_without_merged_pr warning had no evidence a human could even + // navigate to. + // + // This mints the same inert `references` suggestion the primary ref has + // always had, for the rest of them. Deliberately NOT `delivers`, and + // deliberately not a gh_pr_merged event: recording either from PR prose is + // the false-clear hole SYD-280 closed, and it would let anyone silence a + // safety net by typing a ref. What clears the warning is unchanged — a + // human-confirmed `delivers` link, or SYD-262's human resolve. The point is + // to make the evidence reachable, not to decide on the human's behalf. + // + // Same project only. A ref from another project would carry its own repo + // binding, and minting a link across that boundary from free text is a + // guess this epic exists to delete. + if (issue !== undefined && resolvedRepo !== null) { + const primaryIssue = issue; + const actor = getOrCreateActor(db, GITHUB_ACTOR_NAME, "agent"); + for (const siblingRef of refsFromText([pr.title, pr.body])) { + if (siblingRef === primaryIssue.ref) continue; + let sibling; + try { + sibling = getIssue(db, siblingRef); + } catch { + continue; // names no real issue + } + if (sibling.projectId !== primaryIssue.projectId) continue; + recordIngestedPrLink(db, { + issueId: sibling.id, + repo: resolvedRepo, + prNumber, + role: "references", + actorId: actor.id, + }); + } + } + // The observation is the more meaningful answer when both halves ran (a // declared PR that also mentions some other issue), so it wins the single // return slot; the display write already happened either way. Falling diff --git a/tests/services/attention.test.ts b/tests/services/attention.test.ts index 8d2374c..6864d1f 100644 --- a/tests/services/attention.test.ts +++ b/tests/services/attention.test.ts @@ -8,7 +8,8 @@ import { recordEvent } from "../../src/services/events.js"; import { addGithubRepo } from "../../src/services/github-repos.js"; import { upsertPrState } from "../../src/services/pr-state.js"; import { getAttention, listAttentionByIssueId } from "../../src/services/attention.js"; -import { declarePrLink } from "../../src/services/pr-links.js"; +import { declarePrLink, listLiveLinks } from "../../src/services/pr-links.js"; +import { handleGithubWebhook } from "../../src/services/github-webhook.js"; import { resolveDeliveryFailure } from "../../src/services/triage-actions.js"; const REPO = "acme/widgets"; @@ -243,6 +244,42 @@ describe("getAttention — done_without_merged_pr (SYD-204)", () => { expect(getAttention(db, getIssue(db, "SYD-1").id)).toBeNull(); }); + // SYD-274 guard. Widening WHICH refs get a `references` suggestion must not + // widen what a suggestion is worth. This is the SYD-243/SYD-244 shape: their + // work landed under SYD-242's PR, so ingestion now links that PR to them — + // but a link minted from PR prose is exactly what SYD-280 stripped of + // clearing power, and it must stay inert here. A human confirming it (or + // SYD-262's resolve) is what clears the flag. + it("a sibling references link from PR text does not clear the flag (SYD-274)", () => { + const { db, human, agent } = setup(); + createIssue(db, human, { projectKey: "SYD", title: "landed under a sibling's PR" }); + updateIssue(db, human, "SYD-2", { status: "todo" }); + claimIssue(db, agent, "SYD-2"); + updateIssue(db, human, "SYD-2", { status: "in_review" }); + updateIssue(db, human, "SYD-2", { status: "done" }); + expect(getAttention(db, getIssue(db, "SYD-2").id)?.reason).toBe("done_without_merged_pr"); + + handleGithubWebhook(db, "pull_request", { + action: "closed", + repository: { full_name: REPO }, + pull_request: { + number: 206, + html_url: `https://github.com/${REPO}/pull/206`, + head: { ref: "feat/sibling-carrier", sha: "a".repeat(40) }, + updated_at: "2026-07-27T10:00:00Z", + merged: true, + merge_commit_sha: "0ae22a9".padEnd(40, "0"), + title: "feat: the carrier (SYD-1)", + body: "closes SYD-2", + }, + }); + + // The evidence is now reachable from SYD-2... + expect(listLiveLinks(db, getIssue(db, "SYD-2").id).map((l) => l.role)).toEqual(["references"]); + // ...but reachable is not vouched-for. Still lit. + expect(getAttention(db, getIssue(db, "SYD-2").id)?.reason).toBe("done_without_merged_pr"); + }); + // Guard: keyed on event id, so an earlier resolve can't mask a deviation // recorded after it. Dropping `r.id > latest.eventId` would break this. it("does not let an earlier deviation_resolved mask a later deviation", () => { diff --git a/tests/services/github-webhook.test.ts b/tests/services/github-webhook.test.ts index 0824953..0c73dad 100644 --- a/tests/services/github-webhook.test.ts +++ b/tests/services/github-webhook.test.ts @@ -11,8 +11,10 @@ import { handleGithubWebhook, refFromBranch, refFromText, + refsFromText, repositoryFullName, } from "../../src/services/github-webhook.js"; +import { listLiveLinks } from "../../src/services/pr-links.js"; function setup(boundRepos: string[] = []) { const db = openDb(":memory:"); @@ -40,6 +42,19 @@ describe("refFromBranch / refFromText", () => { expect(refFromText("no ref here")).toBeNull(); expect(refFromText(null)).toBeNull(); }); + + // SYD-274: refFromText stops at the first match because one ref owns the + // activity-feed line. refsFromText is for the rest of them. + it("refsFromText returns every ref across the given strings, deduped in order", () => { + expect(refsFromText(["feat: a thing (SYD-242)", "closes SYD-243, SYD-244"])).toEqual([ + "SYD-242", + "SYD-243", + "SYD-244", + ]); + expect(refsFromText(["SYD-1 and SYD-1 again", null, 42, "SYD-2"])).toEqual(["SYD-1", "SYD-2"]); + expect(refsFromText(["no ref here"])).toEqual([]); + expect(refsFromText([])).toEqual([]); + }); }); describe("handleGithubWebhook / pull_request", () => { @@ -796,3 +811,76 @@ describe("handleGithubWebhook / actor reuse", () => { expect(names).toEqual(["sean", "github", "github"]); }); }); + +// SYD-274: one PR routinely carries several issues' work — 0ae22a9 closed +// SYD-243 and SYD-244 under SYD-242's PR. Before this, only the first ref the +// text named got anything; the siblings held no event, no link, no trace the +// PR existed, so their done_without_merged_pr warnings had no evidence to +// reach and stayed lit with no path out but archaeology. +describe("handleGithubWebhook / sibling refs named in PR text (SYD-274)", () => { + function multiIssueSetup() { + const db = openDb(":memory:"); + const human = createActor(db, { name: "sean", type: "human" }).actor; + createProject(db, human, { key: "SYD", name: "Switchyard" }); + createProject(db, human, { key: "NOC", name: "Piano" }); + createIssue(db, human, { projectKey: "SYD", title: "parent" }); // SYD-1 + createIssue(db, human, { projectKey: "SYD", title: "sibling a" }); // SYD-2 + createIssue(db, human, { projectKey: "SYD", title: "sibling b" }); // SYD-3 + createIssue(db, human, { projectKey: "NOC", title: "other project" }); // NOC-1 + addGithubRepo(db, human, { fullName: "acme/widgets", projectKey: "SYD" }); + return { db, human }; + } + + const closingPr = (body: string) => ({ + action: "closed", + repository: { full_name: "acme/widgets" }, + pull_request: { + number: 206, + html_url: "https://github.com/acme/widgets/pull/206", + head: { ref: "feat/multi", sha: "a".repeat(40) }, + updated_at: "2026-07-27T10:00:00Z", + merged: true, + merge_commit_sha: "0ae22a9".padEnd(40, "0"), + title: "feat: a thing (SYD-1)", + body, + }, + }); + + it("mints a references link on every sibling the text names, not just the first", () => { + const { db } = multiIssueSetup(); + handleGithubWebhook(db, "pull_request", closingPr("closes SYD-2, SYD-3")); + for (const ref of ["SYD-1", "SYD-2", "SYD-3"]) { + const links = listLiveLinks(db, getIssue(db, ref).id); + expect( + links.map((l) => l.prNumber), + `${ref} should link PR 206`, + ).toEqual([206]); + } + }); + + // The load-bearing half. SYD-280 removed free-text clearing precisely so a + // passing mention could not silence a safety net; widening WHICH refs get a + // suggestion must not widen what a suggestion is worth. + it("mints them as inert references suggestions — never delivers, never confirmed", () => { + const { db } = multiIssueSetup(); + handleGithubWebhook(db, "pull_request", closingPr("closes SYD-2, SYD-3")); + for (const ref of ["SYD-2", "SYD-3"]) { + const [link] = listLiveLinks(db, getIssue(db, ref).id); + expect(link.role, `${ref} role`).toBe("references"); + expect(link.confirmedBy, `${ref} confirmedBy`).toBeNull(); + } + }); + + it("ignores refs from another project — a cross-project link would be a guess", () => { + const { db } = multiIssueSetup(); + handleGithubWebhook(db, "pull_request", closingPr("also mentions NOC-1")); + expect(listLiveLinks(db, getIssue(db, "NOC-1").id)).toEqual([]); + }); + + it("ignores refs naming no issue", () => { + const { db } = multiIssueSetup(); + const outcome = handleGithubWebhook(db, "pull_request", closingPr("closes SYD-2, SYD-999")); + expect(outcome.handled).toBe(true); + expect(listLiveLinks(db, getIssue(db, "SYD-2").id)).toHaveLength(1); + }); +});