From 014e55e829577b0deb2ec3db0d6f3dfdd07c9c7d Mon Sep 17 00:00:00 2001 From: Prateek Date: Fri, 13 Feb 2026 17:58:02 +0530 Subject: [PATCH] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20Grap?= =?UTF-8?q?hQL=20injection,=20N+1=20queries,=20timeouts,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tracker-linear: use GraphQL variables in listIssues to prevent injection - tracker-linear: add 30s HTTP timeout to linearQuery - tracker-linear: fix issueUrl to use workspace slug from project config - tracker-linear: add labels/assignee support to createIssue - scm-github: rewrite getPendingComments to use GraphQL reviewThreads with real isResolved status instead of hardcoding false - scm-github: simplify getAutomatedComments to single API call (N+1 fix) - scm-github: add 30s timeout to gh CLI calls - scm-github: fix parseDate to return epoch instead of fabricating dates - scm-github: add repo format validation in detectPR - scm-github: fix getCISummary to not count all-skipped as passing - tracker-github: add 30s timeout to gh CLI calls - tracker-github: fix createIssue to not use unsupported --json flag - Add comprehensive vitest tests for scm-github and tracker-github Co-Authored-By: Claude Opus 4.6 --- packages/plugins/scm-github/package.json | 4 +- packages/plugins/scm-github/src/index.ts | 227 +++---- .../plugins/scm-github/test/index.test.ts | 610 ++++++++++++++++++ packages/plugins/tracker-github/package.json | 4 +- packages/plugins/tracker-github/src/index.ts | 32 +- .../plugins/tracker-github/test/index.test.ts | 395 ++++++++++++ packages/plugins/tracker-linear/package.json | 4 +- packages/plugins/tracker-linear/src/index.ts | 155 +++-- pnpm-lock.yaml | 9 + 9 files changed, 1267 insertions(+), 173 deletions(-) create mode 100644 packages/plugins/scm-github/test/index.test.ts create mode 100644 packages/plugins/tracker-github/test/index.test.ts diff --git a/packages/plugins/scm-github/package.json b/packages/plugins/scm-github/package.json index b12ea7669..00a010271 100644 --- a/packages/plugins/scm-github/package.json +++ b/packages/plugins/scm-github/package.json @@ -14,6 +14,7 @@ "scripts": { "build": "tsc", "typecheck": "tsc --noEmit", + "test": "vitest run", "clean": "rm -rf dist" }, "dependencies": { @@ -21,6 +22,7 @@ }, "devDependencies": { "@types/node": "^25.2.3", - "typescript": "^5.7.0" + "typescript": "^5.7.0", + "vitest": "^3.0.0" } } diff --git a/packages/plugins/scm-github/src/index.ts b/packages/plugins/scm-github/src/index.ts index f661d12c6..75d7ad017 100644 --- a/packages/plugins/scm-github/src/index.ts +++ b/packages/plugins/scm-github/src/index.ts @@ -46,6 +46,7 @@ const BOT_AUTHORS = new Set([ async function gh(args: string[]): Promise { const { stdout } = await execFileAsync("gh", args, { maxBuffer: 10 * 1024 * 1024, + timeout: 30_000, }); return stdout.trim(); } @@ -55,7 +56,9 @@ function repoFlag(pr: PRInfo): string { } function parseDate(val: string | undefined | null): Date { - return val ? new Date(val) : new Date(); + if (!val) return new Date(0); + const d = new Date(val); + return isNaN(d.getTime()) ? new Date(0) : d; } // --------------------------------------------------------------------------- @@ -72,7 +75,11 @@ function createGitHubSCM(): SCM { ): Promise { if (!session.branch) return null; - const [owner, repo] = project.repo.split("/"); + const parts = project.repo.split("/"); + if (parts.length !== 2 || !parts[0] || !parts[1]) { + throw new Error(`Invalid repo format "${project.repo}", expected "owner/repo"`); + } + const [owner, repo] = parts; try { const raw = await gh([ "pr", @@ -226,6 +233,11 @@ function createGitHubSCM(): SCM { ); if (hasPending) return "pending"; + // Only report passing if at least one check actually passed + // (not all skipped) + const hasPassing = checks.some((c) => c.status === "passed"); + if (!hasPassing) return "none"; + return "passing"; }, @@ -287,39 +299,81 @@ function createGitHubSCM(): SCM { async getPendingComments(pr: PRInfo): Promise { try { + // Use GraphQL to get review threads with actual isResolved status const raw = await gh([ "api", - `repos/${repoFlag(pr)}/pulls/${pr.number}/comments`, + "graphql", + "-f", + `query=query { + repository(owner: "${pr.owner}", name: "${pr.repo}") { + pullRequest(number: ${pr.number}) { + reviewThreads(first: 100) { + nodes { + isResolved + comments(first: 1) { + nodes { + id + author { login } + body + path + line + url + createdAt + } + } + } + } + } + } + }`, ]); - const comments: Array<{ - id: number; - user: { login: string }; - body: string; - path: string; - line: number | null; - original_line: number | null; - in_reply_to_id?: number; - created_at: string; - html_url: string; - }> = JSON.parse(raw); + const data: { + data: { + repository: { + pullRequest: { + reviewThreads: { + nodes: Array<{ + isResolved: boolean; + comments: { + nodes: Array<{ + id: string; + author: { login: string } | null; + body: string; + path: string | null; + line: number | null; + url: string; + createdAt: string; + }>; + }; + }>; + }; + }; + }; + }; + } = JSON.parse(raw); - // Top-level comments only (not replies) that aren't from bots - return comments - .filter( - (c) => - !c.in_reply_to_id && !BOT_AUTHORS.has(c.user?.login ?? ""), - ) - .map((c) => ({ - id: String(c.id), - author: c.user?.login ?? "unknown", - body: c.body, - path: c.path || undefined, - line: c.line ?? c.original_line ?? undefined, - isResolved: false, - createdAt: parseDate(c.created_at), - url: c.html_url, - })); + const threads = + data.data.repository.pullRequest.reviewThreads.nodes; + + return threads + .filter((t) => { + const author = t.comments.nodes[0]?.author?.login ?? ""; + return !BOT_AUTHORS.has(author); + }) + .map((t) => { + const c = t.comments.nodes[0]; + return { + id: c.id, + author: c.author?.login ?? "unknown", + body: c.body, + path: c.path || undefined, + line: c.line ?? undefined, + isResolved: t.isResolved, + createdAt: parseDate(c.createdAt), + url: c.url, + }; + }); } catch { return []; } @@ -327,7 +381,7 @@ function createGitHubSCM(): SCM { async getAutomatedComments(pr: PRInfo): Promise { try { - // Get review comments from bots + // Single API call gets all review comments (including bot review comments) const raw = await gh([ "api", `repos/${repoFlag(pr)}/pulls/${pr.number}/comments`, @@ -344,89 +398,38 @@ function createGitHubSCM(): SCM { html_url: string; }> = JSON.parse(raw); - // Also get reviews from bots (like cursor[bot] which leaves reviews) - let botReviewComments: Array<{ - id: number; - user: { login: string }; - body: string; - path: string; - line: number | null; - original_line: number | null; - created_at: string; - html_url: string; - }> = []; - - try { - const reviewsRaw = await gh([ - "api", - `repos/${repoFlag(pr)}/pulls/${pr.number}/reviews`, - ]); - const reviews: Array<{ - id: number; - user: { login: string }; - }> = JSON.parse(reviewsRaw); - - // For each bot review, get the review comments - for (const review of reviews) { - if (!BOT_AUTHORS.has(review.user?.login ?? "")) continue; - try { - const reviewCommentsRaw = await gh([ - "api", - `repos/${repoFlag(pr)}/pulls/${pr.number}/reviews/${review.id}/comments`, - ]); - const rc: typeof botReviewComments = JSON.parse(reviewCommentsRaw); - botReviewComments = botReviewComments.concat(rc); - } catch { - // Skip if we can't fetch review comments + return comments + .filter((c) => BOT_AUTHORS.has(c.user?.login ?? "")) + .map((c) => { + // Determine severity from body content + let severity: AutomatedComment["severity"] = "info"; + const bodyLower = c.body.toLowerCase(); + if ( + bodyLower.includes("error") || + bodyLower.includes("bug") || + bodyLower.includes("critical") || + bodyLower.includes("potential issue") + ) { + severity = "error"; + } else if ( + bodyLower.includes("warning") || + bodyLower.includes("suggest") || + bodyLower.includes("consider") + ) { + severity = "warning"; } - } - } catch { - // Skip if we can't fetch reviews - } - const allBotComments = [ - ...comments.filter((c) => BOT_AUTHORS.has(c.user?.login ?? "")), - ...botReviewComments, - ]; - - // Deduplicate by id - const seen = new Set(); - const unique = allBotComments.filter((c) => { - if (seen.has(c.id)) return false; - seen.add(c.id); - return true; - }); - - return unique.map((c) => { - // Determine severity from body content - let severity: AutomatedComment["severity"] = "info"; - const bodyLower = c.body.toLowerCase(); - if ( - bodyLower.includes("error") || - bodyLower.includes("bug") || - bodyLower.includes("critical") || - bodyLower.includes("potential issue") - ) { - severity = "error"; - } else if ( - bodyLower.includes("warning") || - bodyLower.includes("suggest") || - bodyLower.includes("consider") - ) { - severity = "warning"; - } - - return { - id: String(c.id), - botName: c.user?.login ?? "unknown", - body: c.body, - path: c.path || undefined, - line: c.line ?? c.original_line ?? undefined, - severity, - createdAt: parseDate(c.created_at), - url: c.html_url, - }; - }); + return { + id: String(c.id), + botName: c.user?.login ?? "unknown", + body: c.body, + path: c.path || undefined, + line: c.line ?? c.original_line ?? undefined, + severity, + createdAt: parseDate(c.created_at), + url: c.html_url, + }; + }); } catch { return []; } diff --git a/packages/plugins/scm-github/test/index.test.ts b/packages/plugins/scm-github/test/index.test.ts new file mode 100644 index 000000000..a17e03d2c --- /dev/null +++ b/packages/plugins/scm-github/test/index.test.ts @@ -0,0 +1,610 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; + +// --------------------------------------------------------------------------- +// Mock node:child_process — gh CLI calls go through execFileAsync = promisify(execFile) +// vi.hoisted ensures the mock fn is available when vi.mock factory runs (hoisted above imports) +// --------------------------------------------------------------------------- +const { ghMock } = vi.hoisted(() => ({ ghMock: vi.fn() })); + +vi.mock("node:child_process", () => { + // Attach the custom promisify symbol so `promisify(execFile)` returns ghMock + const execFile = Object.assign(vi.fn(), { + [Symbol.for("nodejs.util.promisify.custom")]: ghMock, + }); + return { execFile }; +}); + +import { create, manifest } from "../src/index.js"; +import type { PRInfo, Session, ProjectConfig } from "@agent-orchestrator/core"; + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +const pr: PRInfo = { + number: 42, + url: "https://github.com/acme/repo/pull/42", + title: "feat: add feature", + owner: "acme", + repo: "repo", + branch: "feat/my-feature", + baseBranch: "main", + isDraft: false, +}; + +const project: ProjectConfig = { + name: "test", + repo: "acme/repo", + path: "/tmp/repo", + defaultBranch: "main", + sessionPrefix: "test", +}; + +function makeSession(overrides: Partial = {}): Session { + return { + id: "test-1", + projectId: "test", + status: "working", + activity: "active", + branch: "feat/my-feature", + issueId: null, + pr: null, + workspacePath: "/tmp/repo", + runtimeHandle: null, + agentInfo: null, + createdAt: new Date(), + lastActivityAt: new Date(), + metadata: {}, + ...overrides, + }; +} + +function mockGh(result: unknown) { + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify(result) }); +} + +function mockGhRaw(stdout: string) { + ghMock.mockResolvedValueOnce({ stdout }); +} + +function mockGhError(msg = "Command failed") { + ghMock.mockRejectedValueOnce(new Error(msg)); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("scm-github plugin", () => { + let scm: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + scm = create(); + }); + + // ---- manifest ---------------------------------------------------------- + + describe("manifest", () => { + it("has correct metadata", () => { + expect(manifest.name).toBe("github"); + expect(manifest.slot).toBe("scm"); + expect(manifest.version).toBe("0.1.0"); + }); + }); + + // ---- create() ---------------------------------------------------------- + + describe("create()", () => { + it("returns an SCM with correct name", () => { + expect(scm.name).toBe("github"); + }); + }); + + // ---- detectPR ---------------------------------------------------------- + + describe("detectPR", () => { + it("returns PRInfo when a PR exists", async () => { + mockGh([ + { + number: 42, + url: "https://github.com/acme/repo/pull/42", + title: "feat: add feature", + headRefName: "feat/my-feature", + baseRefName: "main", + isDraft: false, + }, + ]); + + const result = await scm.detectPR(makeSession(), project); + expect(result).toEqual({ + number: 42, + url: "https://github.com/acme/repo/pull/42", + title: "feat: add feature", + owner: "acme", + repo: "repo", + branch: "feat/my-feature", + baseBranch: "main", + isDraft: false, + }); + }); + + it("returns null when no PR found", async () => { + mockGh([]); + const result = await scm.detectPR(makeSession(), project); + expect(result).toBeNull(); + }); + + it("returns null when session has no branch", async () => { + const result = await scm.detectPR(makeSession({ branch: null }), project); + expect(result).toBeNull(); + expect(ghMock).not.toHaveBeenCalled(); + }); + + it("returns null on gh CLI error", async () => { + mockGhError("gh: not found"); + const result = await scm.detectPR(makeSession(), project); + expect(result).toBeNull(); + }); + + it("throws on invalid repo format", async () => { + const badProject = { ...project, repo: "no-slash" }; + await expect(scm.detectPR(makeSession(), badProject)).rejects.toThrow("Invalid repo format"); + }); + + it("detects draft PRs", async () => { + mockGh([ + { + number: 99, + url: "https://github.com/acme/repo/pull/99", + title: "WIP: draft feature", + headRefName: "feat/my-feature", + baseRefName: "main", + isDraft: true, + }, + ]); + const result = await scm.detectPR(makeSession(), project); + expect(result?.isDraft).toBe(true); + }); + }); + + // ---- getPRState -------------------------------------------------------- + + describe("getPRState", () => { + it('returns "open" for open PR', async () => { + mockGh({ state: "OPEN" }); + expect(await scm.getPRState(pr)).toBe("open"); + }); + + it('returns "merged" for merged PR', async () => { + mockGh({ state: "MERGED" }); + expect(await scm.getPRState(pr)).toBe("merged"); + }); + + it('returns "closed" for closed PR', async () => { + mockGh({ state: "CLOSED" }); + expect(await scm.getPRState(pr)).toBe("closed"); + }); + + it("handles lowercase state strings", async () => { + mockGh({ state: "merged" }); + expect(await scm.getPRState(pr)).toBe("merged"); + }); + }); + + // ---- mergePR ----------------------------------------------------------- + + describe("mergePR", () => { + it("uses --squash by default", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await scm.mergePR(pr); + expect(ghMock).toHaveBeenCalledWith( + "gh", + ["pr", "merge", "42", "--repo", "acme/repo", "--squash", "--delete-branch"], + expect.any(Object), + ); + }); + + it("uses --merge when specified", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await scm.mergePR(pr, "merge"); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--merge"]), + expect.any(Object), + ); + }); + + it("uses --rebase when specified", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await scm.mergePR(pr, "rebase"); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--rebase"]), + expect.any(Object), + ); + }); + }); + + // ---- closePR ----------------------------------------------------------- + + describe("closePR", () => { + it("calls gh pr close", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await scm.closePR(pr); + expect(ghMock).toHaveBeenCalledWith( + "gh", + ["pr", "close", "42", "--repo", "acme/repo"], + expect.any(Object), + ); + }); + }); + + // ---- getCIChecks ------------------------------------------------------- + + describe("getCIChecks", () => { + it("maps various check states correctly", async () => { + mockGh([ + { name: "build", state: "COMPLETED", conclusion: "SUCCESS", detailsUrl: "https://ci/1", startedAt: "2025-01-01T00:00:00Z", completedAt: "2025-01-01T00:05:00Z" }, + { name: "lint", state: "COMPLETED", conclusion: "FAILURE", detailsUrl: "", startedAt: "", completedAt: "" }, + { name: "deploy", state: "PENDING", conclusion: "", detailsUrl: "", startedAt: "", completedAt: "" }, + { name: "e2e", state: "IN_PROGRESS", conclusion: "", detailsUrl: "", startedAt: "", completedAt: "" }, + { name: "optional", state: "COMPLETED", conclusion: "SKIPPED", detailsUrl: "", startedAt: "", completedAt: "" }, + { name: "neutral", state: "COMPLETED", conclusion: "NEUTRAL", detailsUrl: "", startedAt: "", completedAt: "" }, + { name: "timeout", state: "COMPLETED", conclusion: "TIMED_OUT", detailsUrl: "", startedAt: "", completedAt: "" }, + { name: "queued", state: "QUEUED", conclusion: "", detailsUrl: "", startedAt: "", completedAt: "" }, + ]); + + const checks = await scm.getCIChecks(pr); + expect(checks).toHaveLength(8); + expect(checks[0].status).toBe("passed"); + expect(checks[0].url).toBe("https://ci/1"); + expect(checks[1].status).toBe("failed"); + expect(checks[2].status).toBe("pending"); + expect(checks[3].status).toBe("running"); + expect(checks[4].status).toBe("skipped"); + expect(checks[5].status).toBe("skipped"); + expect(checks[6].status).toBe("failed"); + expect(checks[7].status).toBe("pending"); + }); + + it("returns empty array on error", async () => { + mockGhError("no checks"); + expect(await scm.getCIChecks(pr)).toEqual([]); + }); + + it("returns empty array for PR with no checks", async () => { + mockGh([]); + expect(await scm.getCIChecks(pr)).toEqual([]); + }); + + it("handles missing optional fields gracefully", async () => { + mockGh([{ name: "test", state: "COMPLETED", conclusion: "SUCCESS" }]); + const checks = await scm.getCIChecks(pr); + expect(checks[0].url).toBeUndefined(); + expect(checks[0].startedAt).toBeUndefined(); + expect(checks[0].completedAt).toBeUndefined(); + }); + }); + + // ---- getCISummary ------------------------------------------------------ + + describe("getCISummary", () => { + it('returns "failing" when any check failed', async () => { + mockGh([ + { name: "a", state: "COMPLETED", conclusion: "SUCCESS" }, + { name: "b", state: "COMPLETED", conclusion: "FAILURE" }, + ]); + expect(await scm.getCISummary(pr)).toBe("failing"); + }); + + it('returns "pending" when checks are running', async () => { + mockGh([ + { name: "a", state: "COMPLETED", conclusion: "SUCCESS" }, + { name: "b", state: "IN_PROGRESS", conclusion: "" }, + ]); + expect(await scm.getCISummary(pr)).toBe("pending"); + }); + + it('returns "passing" when all checks passed', async () => { + mockGh([ + { name: "a", state: "COMPLETED", conclusion: "SUCCESS" }, + { name: "b", state: "COMPLETED", conclusion: "SUCCESS" }, + ]); + expect(await scm.getCISummary(pr)).toBe("passing"); + }); + + it('returns "none" when no checks', async () => { + mockGh([]); + expect(await scm.getCISummary(pr)).toBe("none"); + }); + + it('returns "none" on error (getCIChecks returns [])', async () => { + mockGhError(); + expect(await scm.getCISummary(pr)).toBe("none"); + }); + + it('returns "none" when all checks are skipped', async () => { + mockGh([ + { name: "a", state: "COMPLETED", conclusion: "SKIPPED" }, + { name: "b", state: "COMPLETED", conclusion: "NEUTRAL" }, + ]); + expect(await scm.getCISummary(pr)).toBe("none"); + }); + }); + + // ---- getReviews -------------------------------------------------------- + + describe("getReviews", () => { + it("maps review states correctly", async () => { + mockGh({ + reviews: [ + { author: { login: "alice" }, state: "APPROVED", body: "LGTM", submittedAt: "2025-01-01T00:00:00Z" }, + { author: { login: "bob" }, state: "CHANGES_REQUESTED", body: "Fix this", submittedAt: "2025-01-02T00:00:00Z" }, + { author: { login: "charlie" }, state: "COMMENTED", body: "", submittedAt: "2025-01-03T00:00:00Z" }, + { author: { login: "eve" }, state: "DISMISSED", body: "", submittedAt: "2025-01-04T00:00:00Z" }, + { author: { login: "frank" }, state: "PENDING", body: "", submittedAt: null }, + ], + }); + + const reviews = await scm.getReviews(pr); + expect(reviews).toHaveLength(5); + expect(reviews[0]).toMatchObject({ author: "alice", state: "approved" }); + expect(reviews[1]).toMatchObject({ author: "bob", state: "changes_requested" }); + expect(reviews[2]).toMatchObject({ author: "charlie", state: "commented" }); + expect(reviews[3]).toMatchObject({ author: "eve", state: "dismissed" }); + expect(reviews[4]).toMatchObject({ author: "frank", state: "pending" }); + }); + + it("handles empty reviews", async () => { + mockGh({ reviews: [] }); + expect(await scm.getReviews(pr)).toEqual([]); + }); + + it('defaults to "unknown" author when missing', async () => { + mockGh({ + reviews: [{ author: null, state: "APPROVED", body: "", submittedAt: "2025-01-01T00:00:00Z" }], + }); + const reviews = await scm.getReviews(pr); + expect(reviews[0].author).toBe("unknown"); + }); + }); + + // ---- getReviewDecision ------------------------------------------------- + + describe("getReviewDecision", () => { + it.each([ + ["APPROVED", "approved"], + ["CHANGES_REQUESTED", "changes_requested"], + ["REVIEW_REQUIRED", "pending"], + ] as const)('maps %s to "%s"', async (input, expected) => { + mockGh({ reviewDecision: input }); + expect(await scm.getReviewDecision(pr)).toBe(expected); + }); + + it('returns "none" when reviewDecision is empty', async () => { + mockGh({ reviewDecision: "" }); + expect(await scm.getReviewDecision(pr)).toBe("none"); + }); + + it('returns "none" when reviewDecision is null', async () => { + mockGh({ reviewDecision: null }); + expect(await scm.getReviewDecision(pr)).toBe("none"); + }); + }); + + // ---- getPendingComments ------------------------------------------------ + + describe("getPendingComments", () => { + function makeGraphQLThreads( + threads: Array<{ + isResolved: boolean; + id: string; + author: string | null; + body: string; + path: string | null; + line: number | null; + url: string; + createdAt: string; + }>, + ) { + return { + data: { + repository: { + pullRequest: { + reviewThreads: { + nodes: threads.map((t) => ({ + isResolved: t.isResolved, + comments: { + nodes: [ + { + id: t.id, + author: t.author ? { login: t.author } : null, + body: t.body, + path: t.path, + line: t.line, + url: t.url, + createdAt: t.createdAt, + }, + ], + }, + })), + }, + }, + }, + }, + }; + } + + it("returns non-bot comments with isResolved from GraphQL", async () => { + mockGh( + makeGraphQLThreads([ + { isResolved: false, id: "C1", author: "alice", body: "Fix line 10", path: "src/foo.ts", line: 10, url: "https://github.com/c/1", createdAt: "2025-01-01T00:00:00Z" }, + { isResolved: true, id: "C2", author: "bob", body: "Resolved one", path: "src/bar.ts", line: 20, url: "https://github.com/c/2", createdAt: "2025-01-02T00:00:00Z" }, + ]), + ); + + const comments = await scm.getPendingComments(pr); + expect(comments).toHaveLength(2); + expect(comments[0]).toMatchObject({ id: "C1", author: "alice", isResolved: false }); + expect(comments[1]).toMatchObject({ id: "C2", author: "bob", isResolved: true }); + }); + + it("filters out bot comments", async () => { + mockGh( + makeGraphQLThreads([ + { isResolved: false, id: "C1", author: "alice", body: "Fix this", path: "a.ts", line: 1, url: "u", createdAt: "2025-01-01T00:00:00Z" }, + { isResolved: false, id: "C2", author: "cursor[bot]", body: "Bot says", path: "a.ts", line: 2, url: "u", createdAt: "2025-01-01T00:00:00Z" }, + { isResolved: false, id: "C3", author: "codecov[bot]", body: "Coverage", path: "a.ts", line: 3, url: "u", createdAt: "2025-01-01T00:00:00Z" }, + ]), + ); + + const comments = await scm.getPendingComments(pr); + expect(comments).toHaveLength(1); + expect(comments[0].author).toBe("alice"); + }); + + it("returns empty on error", async () => { + mockGhError("API rate limit"); + expect(await scm.getPendingComments(pr)).toEqual([]); + }); + + it("handles null path and line", async () => { + mockGh( + makeGraphQLThreads([ + { isResolved: false, id: "C1", author: "alice", body: "General comment", path: null, line: null, url: "u", createdAt: "2025-01-01T00:00:00Z" }, + ]), + ); + const comments = await scm.getPendingComments(pr); + expect(comments[0].path).toBeUndefined(); + expect(comments[0].line).toBeUndefined(); + }); + }); + + // ---- getAutomatedComments ---------------------------------------------- + + describe("getAutomatedComments", () => { + it("returns bot comments filtered from all PR comments", async () => { + mockGh([ + { id: 1, user: { login: "cursor[bot]" }, body: "Found a potential issue", path: "a.ts", line: 5, original_line: null, created_at: "2025-01-01T00:00:00Z", html_url: "u1" }, + { id: 2, user: { login: "alice" }, body: "Human comment", path: "a.ts", line: 1, original_line: null, created_at: "2025-01-01T00:00:00Z", html_url: "u2" }, + ]); + + const comments = await scm.getAutomatedComments(pr); + expect(comments).toHaveLength(1); + expect(comments[0].botName).toBe("cursor[bot]"); + expect(comments[0].severity).toBe("error"); // "potential issue" → error + }); + + it("classifies severity from body content", async () => { + mockGh([ + { id: 1, user: { login: "github-actions[bot]" }, body: "Error: build failed", path: "a.ts", line: 1, original_line: null, created_at: "2025-01-01T00:00:00Z", html_url: "u" }, + { id: 2, user: { login: "github-actions[bot]" }, body: "Warning: deprecated API", path: "a.ts", line: 2, original_line: null, created_at: "2025-01-01T00:00:00Z", html_url: "u" }, + { id: 3, user: { login: "github-actions[bot]" }, body: "Deployed to staging", path: "a.ts", line: 3, original_line: null, created_at: "2025-01-01T00:00:00Z", html_url: "u" }, + ]); + + const comments = await scm.getAutomatedComments(pr); + expect(comments).toHaveLength(3); + expect(comments[0].severity).toBe("error"); + expect(comments[1].severity).toBe("warning"); + expect(comments[2].severity).toBe("info"); + }); + + it("returns empty when no bot comments", async () => { + mockGh([ + { id: 1, user: { login: "alice" }, body: "Human comment", path: "a.ts", line: 1, original_line: null, created_at: "2025-01-01T00:00:00Z", html_url: "u" }, + ]); + + const comments = await scm.getAutomatedComments(pr); + expect(comments).toEqual([]); + }); + + it("returns empty on error", async () => { + mockGhError("network failure"); + expect(await scm.getAutomatedComments(pr)).toEqual([]); + }); + + it("uses original_line as fallback", async () => { + mockGh([ + { id: 1, user: { login: "dependabot[bot]" }, body: "Suggest update", path: "a.ts", line: null, original_line: 15, created_at: "2025-01-01T00:00:00Z", html_url: "u" }, + ]); + + const comments = await scm.getAutomatedComments(pr); + expect(comments[0].line).toBe(15); + }); + }); + + // ---- getMergeability --------------------------------------------------- + + describe("getMergeability", () => { + it("returns mergeable when everything is clear", async () => { + // PR view + mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "CLEAN", isDraft: false }); + // CI checks (called by getCISummary) + mockGh([{ name: "build", state: "COMPLETED", conclusion: "SUCCESS" }]); + + const result = await scm.getMergeability(pr); + expect(result).toEqual({ + mergeable: true, + ciPassing: true, + approved: true, + noConflicts: true, + blockers: [], + }); + }); + + it("reports CI failures as blockers", async () => { + mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "UNSTABLE", isDraft: false }); + mockGh([{ name: "build", state: "COMPLETED", conclusion: "FAILURE" }]); + + const result = await scm.getMergeability(pr); + expect(result.ciPassing).toBe(false); + expect(result.mergeable).toBe(false); + expect(result.blockers).toContain("CI is failing"); + }); + + it("reports changes requested as blockers", async () => { + mockGh({ mergeable: "MERGEABLE", reviewDecision: "CHANGES_REQUESTED", mergeStateStatus: "CLEAN", isDraft: false }); + mockGh([]); // no CI checks + + const result = await scm.getMergeability(pr); + expect(result.approved).toBe(false); + expect(result.blockers).toContain("Changes requested in review"); + }); + + it("reports review required as blocker", async () => { + mockGh({ mergeable: "MERGEABLE", reviewDecision: "REVIEW_REQUIRED", mergeStateStatus: "BLOCKED", isDraft: false }); + mockGh([]); + + const result = await scm.getMergeability(pr); + expect(result.blockers).toContain("Review required"); + }); + + it("reports merge conflicts as blockers", async () => { + mockGh({ mergeable: "CONFLICTING", reviewDecision: "APPROVED", mergeStateStatus: "DIRTY", isDraft: false }); + mockGh([]); + + const result = await scm.getMergeability(pr); + expect(result.noConflicts).toBe(false); + expect(result.blockers).toContain("Merge conflicts"); + }); + + it("reports draft status as blocker", async () => { + mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "DRAFT", isDraft: true }); + mockGh([{ name: "build", state: "COMPLETED", conclusion: "SUCCESS" }]); + + const result = await scm.getMergeability(pr); + expect(result.blockers).toContain("PR is still a draft"); + expect(result.mergeable).toBe(false); + }); + + it("reports multiple blockers simultaneously", async () => { + mockGh({ mergeable: "CONFLICTING", reviewDecision: "CHANGES_REQUESTED", mergeStateStatus: "DIRTY", isDraft: true }); + mockGh([{ name: "build", state: "COMPLETED", conclusion: "FAILURE" }]); + + const result = await scm.getMergeability(pr); + expect(result.blockers).toHaveLength(4); + expect(result.mergeable).toBe(false); + }); + }); +}); diff --git a/packages/plugins/tracker-github/package.json b/packages/plugins/tracker-github/package.json index 43d5a1003..520d33a33 100644 --- a/packages/plugins/tracker-github/package.json +++ b/packages/plugins/tracker-github/package.json @@ -14,6 +14,7 @@ "scripts": { "build": "tsc", "typecheck": "tsc --noEmit", + "test": "vitest run", "clean": "rm -rf dist" }, "dependencies": { @@ -21,6 +22,7 @@ }, "devDependencies": { "@types/node": "^25.2.3", - "typescript": "^5.7.0" + "typescript": "^5.7.0", + "vitest": "^3.0.0" } } diff --git a/packages/plugins/tracker-github/src/index.ts b/packages/plugins/tracker-github/src/index.ts index ecbf9b7f1..d7054d2bb 100644 --- a/packages/plugins/tracker-github/src/index.ts +++ b/packages/plugins/tracker-github/src/index.ts @@ -25,6 +25,7 @@ const execFileAsync = promisify(execFile); async function gh(args: string[]): Promise { const { stdout } = await execFileAsync("gh", args, { maxBuffer: 10 * 1024 * 1024, + timeout: 30_000, }); return stdout.trim(); } @@ -270,8 +271,6 @@ function createGitHubTracker(): Tracker { input.title, "--body", input.description, - "--json", - "number,title,body,url,state,labels,assignees", ]; if (input.labels && input.labels.length > 0) { @@ -282,26 +281,17 @@ function createGitHubTracker(): Tracker { args.push("--assignee", input.assignee); } - const raw = await gh(args); - const data: { - number: number; - title: string; - body: string; - url: string; - state: string; - labels: Array<{ name: string }>; - assignees: Array<{ login: string }>; - } = JSON.parse(raw); + // gh issue create outputs the URL of the new issue + const url = await gh(args); - return { - id: String(data.number), - title: data.title, - description: data.body ?? "", - url: data.url, - state: "open", - labels: (data.labels ?? []).map((l) => l.name), - assignee: data.assignees?.[0]?.login, - }; + // Extract issue number from URL and fetch full details + const match = url.match(/\/issues\/(\d+)/); + if (!match) { + throw new Error(`Failed to parse issue URL from gh output: ${url}`); + } + const number = match[1]; + + return this.getIssue(number, project); }, }; } diff --git a/packages/plugins/tracker-github/test/index.test.ts b/packages/plugins/tracker-github/test/index.test.ts new file mode 100644 index 000000000..0088b31e9 --- /dev/null +++ b/packages/plugins/tracker-github/test/index.test.ts @@ -0,0 +1,395 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; + +// --------------------------------------------------------------------------- +// Mock node:child_process +// --------------------------------------------------------------------------- +const { ghMock } = vi.hoisted(() => ({ ghMock: vi.fn() })); + +vi.mock("node:child_process", () => { + const execFile = Object.assign(vi.fn(), { + [Symbol.for("nodejs.util.promisify.custom")]: ghMock, + }); + return { execFile }; +}); + +import { create, manifest } from "../src/index.js"; +import type { ProjectConfig } from "@agent-orchestrator/core"; + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +const project: ProjectConfig = { + name: "test", + repo: "acme/repo", + path: "/tmp/repo", + defaultBranch: "main", + sessionPrefix: "test", +}; + +function mockGh(result: unknown) { + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify(result) }); +} + +function mockGhRaw(stdout: string) { + ghMock.mockResolvedValueOnce({ stdout }); +} + +function mockGhError(msg = "Command failed") { + ghMock.mockRejectedValueOnce(new Error(msg)); +} + +const sampleIssue = { + number: 123, + title: "Fix login bug", + body: "Users can't log in with SSO", + url: "https://github.com/acme/repo/issues/123", + state: "OPEN", + stateReason: null as string | null, + labels: [{ name: "bug" }, { name: "priority-high" }], + assignees: [{ login: "alice" }], +}; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("tracker-github plugin", () => { + let tracker: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + tracker = create(); + }); + + // ---- manifest ---------------------------------------------------------- + + describe("manifest", () => { + it("has correct metadata", () => { + expect(manifest.name).toBe("github"); + expect(manifest.slot).toBe("tracker"); + expect(manifest.version).toBe("0.1.0"); + }); + }); + + describe("create()", () => { + it("returns a Tracker with correct name", () => { + expect(tracker.name).toBe("github"); + }); + }); + + // ---- getIssue ---------------------------------------------------------- + + describe("getIssue", () => { + it("returns Issue with correct fields", async () => { + mockGh(sampleIssue); + const issue = await tracker.getIssue("123", project); + expect(issue).toEqual({ + id: "123", + title: "Fix login bug", + description: "Users can't log in with SSO", + url: "https://github.com/acme/repo/issues/123", + state: "open", + labels: ["bug", "priority-high"], + assignee: "alice", + }); + }); + + it("maps CLOSED state to closed", async () => { + mockGh({ ...sampleIssue, state: "CLOSED", stateReason: "COMPLETED" }); + const issue = await tracker.getIssue("123", project); + expect(issue.state).toBe("closed"); + }); + + it("maps NOT_PLANNED close reason to cancelled", async () => { + mockGh({ ...sampleIssue, state: "CLOSED", stateReason: "NOT_PLANNED" }); + const issue = await tracker.getIssue("123", project); + expect(issue.state).toBe("cancelled"); + }); + + it("handles missing body gracefully", async () => { + mockGh({ ...sampleIssue, body: null }); + const issue = await tracker.getIssue("123", project); + expect(issue.description).toBe(""); + }); + + it("handles empty assignees", async () => { + mockGh({ ...sampleIssue, assignees: [] }); + const issue = await tracker.getIssue("123", project); + expect(issue.assignee).toBeUndefined(); + }); + + it("propagates gh CLI errors", async () => { + mockGhError("issue not found"); + await expect(tracker.getIssue("999", project)).rejects.toThrow("issue not found"); + }); + + it("throws on malformed JSON response", async () => { + ghMock.mockResolvedValueOnce({ stdout: "not json{" }); + await expect(tracker.getIssue("123", project)).rejects.toThrow(); + }); + }); + + // ---- isCompleted ------------------------------------------------------- + + describe("isCompleted", () => { + it("returns true for CLOSED issues", async () => { + mockGh({ state: "CLOSED" }); + expect(await tracker.isCompleted("123", project)).toBe(true); + }); + + it("returns false for OPEN issues", async () => { + mockGh({ state: "OPEN" }); + expect(await tracker.isCompleted("123", project)).toBe(false); + }); + + it("handles lowercase state", async () => { + mockGh({ state: "closed" }); + expect(await tracker.isCompleted("123", project)).toBe(true); + }); + }); + + // ---- issueUrl ---------------------------------------------------------- + + describe("issueUrl", () => { + it("generates correct URL", () => { + expect(tracker.issueUrl("42", project)).toBe("https://github.com/acme/repo/issues/42"); + }); + + it("strips # prefix from identifier", () => { + expect(tracker.issueUrl("#42", project)).toBe("https://github.com/acme/repo/issues/42"); + }); + }); + + // ---- branchName -------------------------------------------------------- + + describe("branchName", () => { + it("generates feat/issue-N format", () => { + expect(tracker.branchName("42", project)).toBe("feat/issue-42"); + }); + + it("strips # prefix", () => { + expect(tracker.branchName("#42", project)).toBe("feat/issue-42"); + }); + }); + + // ---- generatePrompt ---------------------------------------------------- + + describe("generatePrompt", () => { + it("includes title and URL", async () => { + mockGh(sampleIssue); + const prompt = await tracker.generatePrompt("123", project); + expect(prompt).toContain("Fix login bug"); + expect(prompt).toContain("https://github.com/acme/repo/issues/123"); + expect(prompt).toContain("GitHub issue #123"); + }); + + it("includes labels when present", async () => { + mockGh(sampleIssue); + const prompt = await tracker.generatePrompt("123", project); + expect(prompt).toContain("bug, priority-high"); + }); + + it("includes description", async () => { + mockGh(sampleIssue); + const prompt = await tracker.generatePrompt("123", project); + expect(prompt).toContain("Users can't log in with SSO"); + }); + + it("omits labels section when no labels", async () => { + mockGh({ ...sampleIssue, labels: [] }); + const prompt = await tracker.generatePrompt("123", project); + expect(prompt).not.toContain("Labels:"); + }); + + it("omits description section when body is empty", async () => { + mockGh({ ...sampleIssue, body: null }); + const prompt = await tracker.generatePrompt("123", project); + expect(prompt).not.toContain("## Description"); + }); + }); + + // ---- listIssues -------------------------------------------------------- + + describe("listIssues", () => { + it("returns mapped issues", async () => { + mockGh([sampleIssue, { ...sampleIssue, number: 456, title: "Another" }]); + const issues = await tracker.listIssues!({}, project); + expect(issues).toHaveLength(2); + expect(issues[0].id).toBe("123"); + expect(issues[1].id).toBe("456"); + }); + + it("passes state filter for closed issues", async () => { + mockGh([]); + await tracker.listIssues!({ state: "closed" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--state", "closed"]), + expect.any(Object), + ); + }); + + it("passes state filter for all issues", async () => { + mockGh([]); + await tracker.listIssues!({ state: "all" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--state", "all"]), + expect.any(Object), + ); + }); + + it("defaults to open state", async () => { + mockGh([]); + await tracker.listIssues!({}, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--state", "open"]), + expect.any(Object), + ); + }); + + it("passes label filter", async () => { + mockGh([]); + await tracker.listIssues!({ labels: ["bug", "urgent"] }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--label", "bug,urgent"]), + expect.any(Object), + ); + }); + + it("passes assignee filter", async () => { + mockGh([]); + await tracker.listIssues!({ assignee: "alice" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--assignee", "alice"]), + expect.any(Object), + ); + }); + + it("respects custom limit", async () => { + mockGh([]); + await tracker.listIssues!({ limit: 5 }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["--limit", "5"]), + expect.any(Object), + ); + }); + }); + + // ---- updateIssue ------------------------------------------------------- + + describe("updateIssue", () => { + it("closes an issue", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await tracker.updateIssue!("123", { state: "closed" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + ["issue", "close", "123", "--repo", "acme/repo"], + expect.any(Object), + ); + }); + + it("reopens an issue", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await tracker.updateIssue!("123", { state: "open" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + ["issue", "reopen", "123", "--repo", "acme/repo"], + expect.any(Object), + ); + }); + + it("adds labels", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await tracker.updateIssue!("123", { labels: ["bug", "urgent"] }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + ["issue", "edit", "123", "--repo", "acme/repo", "--add-label", "bug,urgent"], + expect.any(Object), + ); + }); + + it("adds assignee", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await tracker.updateIssue!("123", { assignee: "bob" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + ["issue", "edit", "123", "--repo", "acme/repo", "--add-assignee", "bob"], + expect.any(Object), + ); + }); + + it("adds comment", async () => { + ghMock.mockResolvedValueOnce({ stdout: "" }); + await tracker.updateIssue!("123", { comment: "Working on this" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + ["issue", "comment", "123", "--repo", "acme/repo", "--body", "Working on this"], + expect.any(Object), + ); + }); + + it("handles multiple updates in one call", async () => { + ghMock.mockResolvedValue({ stdout: "" }); + await tracker.updateIssue!("123", { state: "closed", labels: ["done"], comment: "Done!" }, project); + // Should have called gh 3 times: close + edit labels + comment + expect(ghMock).toHaveBeenCalledTimes(3); + }); + }); + + // ---- createIssue ------------------------------------------------------- + + describe("createIssue", () => { + it("creates an issue and fetches full details", async () => { + // First call: gh issue create returns URL + mockGhRaw("https://github.com/acme/repo/issues/999\n"); + // Second call: getIssue fetches the created issue + mockGh({ + number: 999, + title: "New issue", + body: "Description", + url: "https://github.com/acme/repo/issues/999", + state: "OPEN", + stateReason: null, + labels: [], + assignees: [], + }); + + const issue = await tracker.createIssue!({ title: "New issue", description: "Description" }, project); + expect(issue).toMatchObject({ id: "999", title: "New issue", state: "open" }); + }); + + it("passes labels and assignee to gh issue create", async () => { + mockGhRaw("https://github.com/acme/repo/issues/1000\n"); + mockGh({ + number: 1000, + title: "Bug", + body: "Crash", + url: "https://github.com/acme/repo/issues/1000", + state: "OPEN", + stateReason: null, + labels: [{ name: "bug" }], + assignees: [{ login: "alice" }], + }); + + await tracker.createIssue!({ title: "Bug", description: "Crash", labels: ["bug"], assignee: "alice" }, project); + expect(ghMock).toHaveBeenCalledWith( + "gh", + expect.arrayContaining(["issue", "create", "--label", "bug", "--assignee", "alice"]), + expect.any(Object), + ); + }); + + it("throws when URL cannot be parsed from gh output", async () => { + mockGhRaw("unexpected output"); + await expect( + tracker.createIssue!({ title: "Test", description: "" }, project), + ).rejects.toThrow("Failed to parse issue URL"); + }); + }); +}); diff --git a/packages/plugins/tracker-linear/package.json b/packages/plugins/tracker-linear/package.json index 3b90a2cc0..66476b322 100644 --- a/packages/plugins/tracker-linear/package.json +++ b/packages/plugins/tracker-linear/package.json @@ -14,6 +14,7 @@ "scripts": { "build": "tsc", "typecheck": "tsc --noEmit", + "test": "vitest run", "clean": "rm -rf dist" }, "dependencies": { @@ -21,6 +22,7 @@ }, "devDependencies": { "@types/node": "^25.2.3", - "typescript": "^5.7.0" + "typescript": "^5.7.0", + "vitest": "^3.0.0" } } diff --git a/packages/plugins/tracker-linear/src/index.ts b/packages/plugins/tracker-linear/src/index.ts index 3fa0cf91d..724494ea8 100644 --- a/packages/plugins/tracker-linear/src/index.ts +++ b/packages/plugins/tracker-linear/src/index.ts @@ -45,6 +45,14 @@ async function linearQuery( return new Promise((resolve, reject) => { const url = new URL(LINEAR_API_URL); + let settled = false; + const settle = (fn: () => void) => { + if (!settled) { + settled = true; + fn(); + } + }; + const req = request( { hostname: url.hostname, @@ -60,25 +68,35 @@ async function linearQuery( const chunks: Buffer[] = []; res.on("data", (chunk: Buffer) => chunks.push(chunk)); res.on("end", () => { - try { - const text = Buffer.concat(chunks).toString("utf-8"); - const json: LinearResponse = JSON.parse(text); - if (json.errors && json.errors.length > 0) { - reject(new Error(`Linear API error: ${json.errors[0].message}`)); - return; + settle(() => { + try { + const text = Buffer.concat(chunks).toString("utf-8"); + const json: LinearResponse = JSON.parse(text); + if (json.errors && json.errors.length > 0) { + reject(new Error(`Linear API error: ${json.errors[0].message}`)); + return; + } + if (!json.data) { + reject(new Error("Linear API returned no data")); + return; + } + resolve(json.data); + } catch (err) { + reject(err); } - if (!json.data) { - reject(new Error("Linear API returned no data")); - return; - } - resolve(json.data); - } catch (err) { - reject(err); - } + }); }); }, ); - req.on("error", reject); + + req.setTimeout(30_000, () => { + settle(() => { + req.destroy(); + reject(new Error("Linear API request timed out after 30s")); + }); + }); + + req.on("error", (err) => settle(() => reject(err))); req.write(body); req.end(); }); @@ -197,7 +215,13 @@ function createLinearTracker(): Tracker { return stateType === "completed" || stateType === "canceled"; }, - issueUrl(identifier: string, _project: ProjectConfig): string { + issueUrl(identifier: string, project: ProjectConfig): string { + const slug = project.tracker?.["workspaceSlug"] as string | undefined; + if (slug) { + return `https://linear.app/${slug}/issue/${identifier}`; + } + // Fallback: Linear also supports /issue/ URLs that redirect, + // but they require authentication return `https://linear.app/issue/${identifier}`; }, @@ -248,44 +272,44 @@ function createLinearTracker(): Tracker { filters: IssueFilters, project: ProjectConfig, ): Promise { - // Build filter object for Linear's GraphQL API - const filterParts: string[] = []; + // Build filter object using GraphQL variables to prevent injection + const filter: Record = {}; + const variables: Record = {}; if (filters.state === "open") { - filterParts.push(`state: { type: { nin: ["completed", "canceled"] } }`); + filter["state"] = { type: { nin: ["completed", "canceled"] } }; } else if (filters.state === "closed") { - filterParts.push(`state: { type: { in: ["completed", "canceled"] } }`); + filter["state"] = { type: { in: ["completed", "canceled"] } }; } if (filters.assignee) { - filterParts.push(`assignee: { displayName: { eq: "${filters.assignee}" } }`); + filter["assignee"] = { displayName: { eq: filters.assignee } }; } if (filters.labels && filters.labels.length > 0) { - filterParts.push(`labels: { name: { in: [${filters.labels.map((l) => `"${l}"`).join(", ")}] } }`); + filter["labels"] = { name: { in: filters.labels } }; } // Add team filter if available from project config - const teamId = (project.tracker as Record | undefined)?.["teamId"]; + const teamId = project.tracker?.["teamId"]; if (teamId) { - filterParts.push(`team: { id: { eq: "${teamId}" } }`); + filter["team"] = { id: { eq: teamId } }; } - const filterStr = - filterParts.length > 0 ? `filter: { ${filterParts.join(", ")} }` : ""; - - const limit = filters.limit ?? 30; + variables["filter"] = Object.keys(filter).length > 0 ? filter : undefined; + variables["first"] = filters.limit ?? 30; const data = await linearQuery<{ issues: { nodes: LinearIssueNode[] }; }>( - `query { - issues(${filterStr} first: ${limit}) { + `query($filter: IssueFilter, $first: Int!) { + issues(filter: $filter, first: $first) { nodes { ${ISSUE_FIELDS} } } }`, + variables, ); return data.issues.nodes.map((node) => ({ @@ -375,7 +399,7 @@ function createLinearTracker(): Tracker { input: CreateIssueInput, project: ProjectConfig, ): Promise { - const teamId = (project.tracker as Record | undefined)?.["teamId"]; + const teamId = project.tracker?.["teamId"]; if (!teamId) { throw new Error( "Linear tracker requires 'teamId' in project tracker config", @@ -392,18 +416,36 @@ function createLinearTracker(): Tracker { variables["priority"] = input.priority; } + if (input.assignee) { + variables["assigneeName"] = input.assignee; + } + + // Build label IDs variable parts + const varDecls = [ + "$title: String!", + "$description: String!", + "$teamId: String!", + "$priority: Int", + ...(input.assignee ? ["$assigneeName: String"] : []), + ]; + + const inputFields = [ + "title: $title", + "description: $description", + "teamId: $teamId", + "priority: $priority", + ...(input.assignee ? ["assigneeDisplayName: $assigneeName"] : []), + ]; + const data = await linearQuery<{ issueCreate: { success: boolean; issue: LinearIssueNode; }; }>( - `mutation($title: String!, $description: String!, $teamId: String!, $priority: Int) { + `mutation(${varDecls.join(", ")}) { issueCreate(input: { - title: $title, - description: $description, - teamId: $teamId, - priority: $priority + ${inputFields.join(",\n ")} }) { success issue { @@ -415,7 +457,7 @@ function createLinearTracker(): Tracker { ); const node = data.issueCreate.issue; - return { + const issue: Issue = { id: node.identifier, title: node.title, description: node.description ?? "", @@ -425,6 +467,45 @@ function createLinearTracker(): Tracker { assignee: node.assignee?.displayName ?? node.assignee?.name, priority: node.priority, }; + + // Add labels after creation (Linear's issueCreate doesn't accept label names directly) + if (input.labels && input.labels.length > 0) { + try { + // Look up label IDs by name for the team + const labelsData = await linearQuery<{ + issueLabels: { nodes: Array<{ id: string; name: string }> }; + }>( + `query($teamId: ID) { + issueLabels(filter: { team: { id: { eq: $teamId } } }) { + nodes { id name } + } + }`, + { teamId }, + ); + + const labelMap = new Map(labelsData.issueLabels.nodes.map((l) => [l.name, l.id])); + const labelIds = input.labels + .map((name) => labelMap.get(name)) + .filter((id): id is string => id !== undefined); + + if (labelIds.length > 0) { + await linearQuery( + `mutation($id: String!, $labelIds: [String!]!) { + issueUpdate(id: $id, input: { labelIds: $labelIds }) { + success + } + }`, + { id: node.id, labelIds }, + ); + // Reflect the labels we added + issue.labels = input.labels; + } + } catch { + // Labels are best-effort; don't fail the whole creation + } + } + + return issue; }, }; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ac6f04cc7..8698f1769 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -258,6 +258,9 @@ importers: typescript: specifier: ^5.7.0 version: 5.9.3 + vitest: + specifier: ^3.0.0 + version: 3.2.4(@types/node@25.2.3)(jiti@2.6.1)(jsdom@25.0.1)(lightningcss@1.30.2)(tsx@4.21.0)(yaml@2.8.2) packages/plugins/terminal-iterm2: dependencies: @@ -297,6 +300,9 @@ importers: typescript: specifier: ^5.7.0 version: 5.9.3 + vitest: + specifier: ^3.0.0 + version: 3.2.4(@types/node@25.2.3)(jiti@2.6.1)(jsdom@25.0.1)(lightningcss@1.30.2)(tsx@4.21.0)(yaml@2.8.2) packages/plugins/tracker-linear: dependencies: @@ -310,6 +316,9 @@ importers: typescript: specifier: ^5.7.0 version: 5.9.3 + vitest: + specifier: ^3.0.0 + version: 3.2.4(@types/node@25.2.3)(jiti@2.6.1)(jsdom@25.0.1)(lightningcss@1.30.2)(tsx@4.21.0)(yaml@2.8.2) packages/plugins/workspace-clone: dependencies: