diff --git a/.github/workflows/auto-close-duplicates.yml b/.github/workflows/auto-close-duplicates.yml index e8092299..ba0b11d8 100644 --- a/.github/workflows/auto-close-duplicates.yml +++ b/.github/workflows/auto-close-duplicates.yml @@ -22,6 +22,9 @@ jobs: with: bun-version: latest + - name: Run auto-close-duplicates tests + run: bun test scripts/auto-close-duplicates + - name: Auto-close duplicate issues run: bun run scripts/auto-close-duplicates.ts env: diff --git a/package.json b/package.json index 90070ec5..abddf802 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,8 @@ }, "license": "Apache-2.0", "scripts": { - "setup": "echo '⚠️ This npm setup is for the archived web application. For agent development, use: ./scripts/setup-python.sh' && bash scripts/setup.sh" + "setup": "echo '⚠️ This npm setup is for the archived web application. For agent development, use: ./scripts/setup-python.sh' && bash scripts/setup.sh", + "test:duplicates": "bun test scripts/auto-close-duplicates" }, "devDependencies": { "@types/node": "^20.10.0", diff --git a/scripts/AUTO_CLOSE_DUPLICATES_CROSS_VERIFY.md b/scripts/AUTO_CLOSE_DUPLICATES_CROSS_VERIFY.md new file mode 100644 index 00000000..b0d5c75d --- /dev/null +++ b/scripts/AUTO_CLOSE_DUPLICATES_CROSS_VERIFY.md @@ -0,0 +1,52 @@ +# Cross-verification: auto-close-duplicates changes + +## 1. Files changed (this PR / session) + +| File | Change | +|------|--------| +| `scripts/auto-close-duplicates.ts` | Circular-dup + self-ref prevention; extracted helpers (isDupeComment, isDupeCommentOldEnough, authorDisagreedWithDupe, getLastDupeComment, decideAutoClose); `if (import.meta.main)` guard | +| `scripts/auto-close-duplicates.test.ts` | New: 23 unit tests for all exported helpers | +| `package.json` | Added `"test:duplicates": "bun test scripts/auto-close-duplicates"` | +| `.github/workflows/auto-close-duplicates.yml` | Added step "Run auto-close-duplicates tests" before auto-close step | + +**Not changed:** `core/`, `tools/`, main CI (`.github/workflows/ci.yml`), triage workflow logic (only the script that consumes its comments). + +--- + +## 2. Dependencies / who uses what + +- **Only consumer of the script:** `.github/workflows/auto-close-duplicates.yml` runs `bun run scripts/auto-close-duplicates.ts`. No other code imports or runs it. +- **Only consumer of the script’s exports:** `scripts/auto-close-duplicates.test.ts` imports the exported helpers for unit tests. +- **Triage workflow contract:** `.github/workflows/claude-issue-triage.yml` instructs the bot to post: `"Found a possible duplicate of #: ..."`. The script still recognises this via `isDupeComment` (body includes `"possible duplicate"` and `comment.user.type === "Bot"`). **No change to that contract.** + +--- + +## 3. Backward compatibility + +- **Happy path unchanged:** Issue A has a bot comment “possible duplicate of #B”, B is open, comment > 12h old, no author thumbs-down → script still closes A as duplicate of B. +- **New safeguards only:** (1) If B is already closed → skip (no close). (2) If comment says “duplicate of #A” on issue A (self) → skip. (3) If target fetch fails or returns non-`open` state → skip. +- **Comment format:** Still `"possible duplicate"` + Bot; `extractDuplicateIssueNumber` still supports `#N` and GitHub issue URL. No change required in triage prompt. + +--- + +## 4. What was not touched + +- **Core / tools / agent_builder:** No edits to `core/`, `tools/`, or `agent_builder_server.py`. Grep hits for `agent_builder` / `SESSIONS_DIR` are in other docs or local issue/notes, not in this script. +- **Main CI:** `ci.yml` only runs Python (ruff + pytest). It does **not** run the TS script or `bun test`. So these changes do not affect main CI. +- **Triage workflow:** Only the **comment text** is consumed by the script. We did not change the triage YAML or the bot’s instructions; we only changed how the auto-close script interprets comments and when it skips closing. + +--- + +## 5. Verification performed + +- **Unit tests:** `bun test scripts/auto-close-duplicates` → 23 tests pass. +- **Script entry:** `bun run scripts/auto-close-duplicates.ts` with fake env runs and fails at first API call (401), so the main path still runs and `import.meta.main` guard works. +- **No other imports:** Only `scripts/auto-close-duplicates.test.ts` imports from `auto-close-duplicates.ts`. + +--- + +## 6. Conclusion + +- **Impact:** Limited to the auto-close-duplicates workflow and its tests. No impact on core, tools, main CI, or triage prompt text. +- **Behavior:** Same for the normal case; only adds skip conditions for circular duplicate, self-reference, and closed/unreachable target. +- **Contract:** Triage comment format and 12h + author-reaction behaviour unchanged; script remains compatible with existing triage. diff --git a/scripts/auto-close-duplicates.test.ts b/scripts/auto-close-duplicates.test.ts new file mode 100644 index 00000000..7422cc50 --- /dev/null +++ b/scripts/auto-close-duplicates.test.ts @@ -0,0 +1,261 @@ +/** + * Tests for auto-close-duplicates script: comment filter, 12h check, + * author reaction, extractDuplicateIssueNumber, and decideAutoClose + * (circular-dup and self-ref prevention). + */ +import { describe, expect, test } from "bun:test"; +import { + authorDisagreedWithDupe, + decideAutoClose, + extractDuplicateIssueNumber, + getLastDupeComment, + isDupeComment, + isDupeCommentOldEnough, + type GitHubComment, + type GitHubIssue, + type GitHubReaction, +} from "./auto-close-duplicates"; + +describe("extractDuplicateIssueNumber", () => { + test("extracts #123 format", () => { + expect( + extractDuplicateIssueNumber("Found a possible duplicate of #1275: ...") + ).toBe(1275); + expect(extractDuplicateIssueNumber("Duplicate of #1")).toBe(1); + expect(extractDuplicateIssueNumber("See #1000")).toBe(1000); + }); + + test("extracts first #N when multiple present", () => { + expect( + extractDuplicateIssueNumber("Duplicate of #1000 and also #1275") + ).toBe(1000); + }); + + test("extracts GitHub issue URL format", () => { + expect( + extractDuplicateIssueNumber( + "Duplicate of https://github.com/adenhq/hive/issues/42" + ) + ).toBe(42); + }); + + test("returns null when no issue number", () => { + expect(extractDuplicateIssueNumber("No number here")).toBe(null); + expect(extractDuplicateIssueNumber("")).toBe(null); + }); +}); + +describe("isDupeComment", () => { + test("true when body has 'possible duplicate' and user is Bot", () => { + expect( + isDupeComment({ + id: 1, + body: "Found a possible duplicate of #1000: same bug", + created_at: "", + user: { type: "Bot", id: 2 }, + }) + ).toBe(true); + expect( + isDupeComment({ + id: 1, + body: "Possible duplicate of #1275", + created_at: "", + user: { type: "Bot", id: 2 }, + }) + ).toBe(true); + }); + + test("false when body lacks 'possible duplicate'", () => { + expect( + isDupeComment({ + id: 1, + body: "Not a duplicate", + created_at: "", + user: { type: "Bot", id: 2 }, + }) + ).toBe(false); + }); + + test("false when user is not Bot", () => { + expect( + isDupeComment({ + id: 1, + body: "Found a possible duplicate of #1000", + created_at: "", + user: { type: "User", id: 2 }, + }) + ).toBe(false); + }); +}); + +describe("isDupeCommentOldEnough", () => { + test("true when comment date is before twelveHoursAgo", () => { + const twelveHoursAgo = new Date("2025-01-28T12:00:00Z"); + const oldComment = new Date("2025-01-28T00:00:00Z"); + expect(isDupeCommentOldEnough(oldComment, twelveHoursAgo)).toBe(true); + }); + + test("true when comment date equals twelveHoursAgo", () => { + const twelveHoursAgo = new Date("2025-01-28T12:00:00Z"); + expect(isDupeCommentOldEnough(twelveHoursAgo, twelveHoursAgo)).toBe(true); + }); + + test("false when comment is after twelveHoursAgo (too recent)", () => { + const twelveHoursAgo = new Date("2025-01-28T12:00:00Z"); + const recentComment = new Date("2025-01-28T18:00:00Z"); + expect(isDupeCommentOldEnough(recentComment, twelveHoursAgo)).toBe(false); + }); +}); + +describe("authorDisagreedWithDupe", () => { + test("true when issue author gave thumbs down", () => { + const issue = { number: 1275, title: "", state: "open", user: { id: 42 }, created_at: "" }; + const reactions: GitHubReaction[] = [ + { user: { id: 42 }, content: "-1" }, + ]; + expect(authorDisagreedWithDupe(reactions, issue)).toBe(true); + }); + + test("false when only other users reacted", () => { + const issue = { number: 1275, title: "", state: "open", user: { id: 42 }, created_at: "" }; + const reactions: GitHubReaction[] = [ + { user: { id: 99 }, content: "-1" }, + { user: { id: 1 }, content: "+1" }, + ]; + expect(authorDisagreedWithDupe(reactions, issue)).toBe(false); + }); + + test("false when author gave +1 or other reaction", () => { + const issue = { number: 1275, title: "", state: "open", user: { id: 42 }, created_at: "" }; + expect(authorDisagreedWithDupe([{ user: { id: 42 }, content: "+1" }], issue)).toBe(false); + expect(authorDisagreedWithDupe([{ user: { id: 42 }, content: "eyes" }], issue)).toBe(false); + }); +}); + +describe("getLastDupeComment", () => { + test("returns null when no dupe comments", () => { + expect( + getLastDupeComment([ + { id: 1, body: "Not a duplicate", created_at: "", user: { type: "User", id: 1 } }, + ]) + ).toBe(null); + }); + + test("returns the only dupe comment when one exists", () => { + const c: GitHubComment = { + id: 1, + body: "Found a possible duplicate of #1000", + created_at: "", + user: { type: "Bot", id: 2 }, + }; + expect(getLastDupeComment([c])).toBe(c); + }); + + test("returns the last dupe comment when multiple exist", () => { + const c1: GitHubComment = { + id: 1, + body: "Found a possible duplicate of #1000", + created_at: "", + user: { type: "Bot", id: 2 }, + }; + const c2: GitHubComment = { + id: 2, + body: "Found a possible duplicate of #1275", + created_at: "", + user: { type: "Bot", id: 2 }, + }; + const other: GitHubComment = { + id: 3, + body: "Some other comment", + created_at: "", + user: { type: "User", id: 3 }, + }; + expect(getLastDupeComment([other, c1, c2])).toBe(c2); + }); +}); + +function issue(num: number, state = "open"): GitHubIssue { + return { + number: num, + title: `Issue ${num}`, + state, + user: { id: 1 }, + created_at: new Date().toISOString(), + }; +} + +function comment(body: string): GitHubComment { + return { + id: 1, + body, + created_at: new Date().toISOString(), + user: { type: "Bot", id: 2 }, + }; +} + +describe("decideAutoClose", () => { + test("returns null when comment has no extractable issue number", async () => { + const result = await decideAutoClose( + issue(1275), + comment("Possible duplicate of something else"), + async () => ({ state: "open" }) + ); + expect(result).toBe(null); + }); + + test("returns null when duplicate target is self (same issue number)", async () => { + const result = await decideAutoClose( + issue(1275), + comment("Found a possible duplicate of #1275: same issue"), + async () => ({ state: "open" }) + ); + expect(result).toBe(null); + }); + + test("returns null when target issue is closed (avoids circular closure)", async () => { + const result = await decideAutoClose( + issue(1275), + comment("Found a possible duplicate of #1000"), + async (num) => (num === 1000 ? { state: "closed" } : { state: "open" }) + ); + expect(result).toBe(null); + }); + + test("returns null when getTargetIssue returns null", async () => { + const result = await decideAutoClose( + issue(1275), + comment("Found a possible duplicate of #1000"), + async () => null + ); + expect(result).toBe(null); + }); + + test("returns null when getTargetIssue throws", async () => { + const result = await decideAutoClose( + issue(1275), + comment("Found a possible duplicate of #1000"), + async () => { + throw new Error("API error"); + } + ); + expect(result).toBe(null); + }); + + test("returns duplicateOf number when target is open (should close)", async () => { + const result = await decideAutoClose( + issue(1275), + comment("Found a possible duplicate of #1000: same bug"), + async (num) => (num === 1000 ? { state: "open" } : { state: "closed" }) + ); + expect(result).toBe(1000); + }); + + test("returns null when target state is not exactly 'open' (e.g. uppercase)", async () => { + const result = await decideAutoClose( + issue(1275), + comment("Found a possible duplicate of #1000"), + async () => ({ state: "OPEN" } as { state: string }) + ); + expect(result).toBe(null); + }); +}); diff --git a/scripts/auto-close-duplicates.ts b/scripts/auto-close-duplicates.ts index ec46e16b..42461d70 100644 --- a/scripts/auto-close-duplicates.ts +++ b/scripts/auto-close-duplicates.ts @@ -6,21 +6,22 @@ declare global { }; } -interface GitHubIssue { +export interface GitHubIssue { number: number; title: string; + state: string; user: { id: number }; created_at: string; } -interface GitHubComment { +export interface GitHubComment { id: number; body: string; created_at: string; user: { type: string; id: number }; } -interface GitHubReaction { +export interface GitHubReaction { user: { id: number }; content: string; } @@ -57,7 +58,41 @@ async function githubRequest( return response.json(); } -function extractDuplicateIssueNumber(commentBody: string): number | null { +/** True if comment is a bot "possible duplicate" detection (used for filtering). */ +export function isDupeComment(comment: GitHubComment): boolean { + const bodyLower = comment.body.toLowerCase(); + return ( + bodyLower.includes("possible duplicate") && comment.user.type === "Bot" + ); +} + +/** True if the duplicate comment is old enough to auto-close (>= 12h). */ +export function isDupeCommentOldEnough( + dupeCommentDate: Date, + twelveHoursAgo: Date +): boolean { + return dupeCommentDate <= twelveHoursAgo; +} + +/** True if the issue author reacted with thumbs down to the duplicate comment. */ +export function authorDisagreedWithDupe( + reactions: GitHubReaction[], + issue: GitHubIssue +): boolean { + return reactions.some( + (r) => r.user.id === issue.user.id && r.content === "-1" + ); +} + +/** Returns the most recent duplicate-detection comment, or null if none. */ +export function getLastDupeComment( + comments: GitHubComment[] +): GitHubComment | null { + const dupeComments = comments.filter(isDupeComment); + return dupeComments.length > 0 ? dupeComments[dupeComments.length - 1]! : null; +} + +export function extractDuplicateIssueNumber(commentBody: string): number | null { // Try to match #123 format first let match = commentBody.match(/#(\d+)/); if (match) { @@ -73,6 +108,30 @@ function extractDuplicateIssueNumber(commentBody: string): number | null { return null; } +/** + * Decides whether to auto-close this issue as duplicate of another. + * Returns the target issue number to close as duplicate of, or null to skip. + * Used by the main loop and by tests. + */ +export async function decideAutoClose( + issue: GitHubIssue, + lastDupeComment: GitHubComment, + getTargetIssue: (issueNumber: number) => Promise<{ state: string } | null> +): Promise { + const duplicateIssueNumber = extractDuplicateIssueNumber(lastDupeComment.body); + if (duplicateIssueNumber === null) return null; + + if (duplicateIssueNumber === issue.number) return null; + + try { + const targetIssue = await getTargetIssue(duplicateIssueNumber); + if (!targetIssue || targetIssue.state !== "open") return null; + return duplicateIssueNumber; + } catch { + return null; + } +} + async function closeIssueAsDuplicate( owner: string, repo: string, @@ -173,25 +232,18 @@ async function autoCloseDuplicates(): Promise { `[DEBUG] Issue #${issue.number} has ${comments.length} comments` ); - const dupeComments = comments.filter((comment) => { - const bodyLower = comment.body.toLowerCase(); - return ( - bodyLower.includes("possible duplicate") && - comment.user.type === "Bot" - ); - }); + const lastDupeComment = getLastDupeComment(comments); + const dupeCount = comments.filter(isDupeComment).length; console.log( - `[DEBUG] Issue #${issue.number} has ${dupeComments.length} duplicate detection comments` + `[DEBUG] Issue #${issue.number} has ${dupeCount} duplicate detection comments` ); - if (dupeComments.length === 0) { + if (lastDupeComment === null) { console.log( `[DEBUG] Issue #${issue.number} - no duplicate comments found, skipping` ); continue; } - - const lastDupeComment = dupeComments[dupeComments.length - 1]; const dupeCommentDate = new Date(lastDupeComment.created_at); console.log( `[DEBUG] Issue #${ @@ -199,7 +251,7 @@ async function autoCloseDuplicates(): Promise { } - most recent duplicate comment from: ${dupeCommentDate.toISOString()}` ); - if (dupeCommentDate > twelveHoursAgo) { + if (!isDupeCommentOldEnough(dupeCommentDate, twelveHoursAgo)) { console.log( `[DEBUG] Issue #${issue.number} - duplicate comment is too recent, skipping` ); @@ -224,10 +276,7 @@ async function autoCloseDuplicates(): Promise { `[DEBUG] Issue #${issue.number} - duplicate comment has ${reactions.length} reactions` ); - const authorThumbsDown = reactions.some( - (reaction) => - reaction.user.id === issue.user.id && reaction.content === "-1" - ); + const authorThumbsDown = authorDisagreedWithDupe(reactions, issue); console.log( `[DEBUG] Issue #${issue.number} - author thumbs down reaction: ${authorThumbsDown}` ); @@ -239,12 +288,19 @@ async function autoCloseDuplicates(): Promise { continue; } - const duplicateIssueNumber = extractDuplicateIssueNumber( - lastDupeComment.body + const duplicateOf = await decideAutoClose( + issue, + lastDupeComment, + (issueNumber) => + githubRequest( + `/repos/${owner}/${repo}/issues/${issueNumber}`, + token + ).then((i) => ({ state: i.state })) ); - if (!duplicateIssueNumber) { + + if (duplicateOf === null) { console.log( - `[DEBUG] Issue #${issue.number} - could not extract duplicate issue number from comment, skipping` + `[DEBUG] Issue #${issue.number} - skipping (invalid/self/closed target or fetch error)` ); continue; } @@ -254,17 +310,17 @@ async function autoCloseDuplicates(): Promise { try { console.log( - `[INFO] Auto-closing issue #${issue.number} as duplicate of #${duplicateIssueNumber}: ${issueUrl}` + `[INFO] Auto-closing issue #${issue.number} as duplicate of #${duplicateOf}: ${issueUrl}` ); await closeIssueAsDuplicate( owner, repo, issue.number, - duplicateIssueNumber, + duplicateOf, token ); console.log( - `[SUCCESS] Successfully closed issue #${issue.number} as duplicate of #${duplicateIssueNumber}` + `[SUCCESS] Successfully closed issue #${issue.number} as duplicate of #${duplicateOf}` ); } catch (error) { console.error( @@ -278,6 +334,8 @@ async function autoCloseDuplicates(): Promise { ); } -autoCloseDuplicates().catch(console.error); +if (import.meta.main) { + autoCloseDuplicates().catch(console.error); +} export {};