diff --git a/README.md b/README.md index 49ed541..fade332 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ A keyboard-driven dashboard for your GitHub pull requests, review requests and n There are currently two clients: - **TUI** (`packages/cli`) — terminal UI built on the new sync engine (`packages/sync`, SQLite cache + bulk GraphQL). Browse PRs/reviews/notifications, read descriptions and comments, review diffs and post line comments, toggle draft/auto-merge, approve. Press `?` for keyboard shortcuts. -- **Web app** (`packages/web` + `packages/server`, optionally wrapped in Electron via `packages/desktop`) — browser dashboard backed by its own Hono API server and cache. **Not yet migrated to the sync engine.** A pre-built macOS app is on the [Releases page](https://github.com/AntonNiklasson/github-dashboard/releases). +- **Web app** (`packages/web` + `packages/server`, optionally wrapped in Electron via `packages/desktop`) — browser dashboard backed by a Hono API adapter to the shared sync engine and SQLite cache. A pre-built macOS app is on the [Releases page](https://github.com/AntonNiklasson/github-dashboard/releases). ## Development @@ -57,3 +57,7 @@ TUI state lives in XDG dirs and is safe to delete: - `~/.cache/github-dashboard/cache.sqlite` — sync cache - `~/.local/state/github-dashboard/tui.json` — last view, instance and sorting + +### Desktop/web sync engine scope + +The desktop and web clients use `packages/sync` for all GitHub access, including authentication, list synchronization, comments and file diffs. Supported writes are approval, draft/ready toggling, auto-merge toggling and notification dismissal. Direct merge/close, title editing, CI reruns, posting conversation comments, closed-PR search, colleague browsing, and detailed commit/check lists are currently unavailable; use “Open in browser” for those operations. The local Hono HTTP API remains the renderer transport, but owns no GitHub client or live cache. diff --git a/packages/server/package.json b/packages/server/package.json index fc9bdf5..fb2ceb0 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -10,7 +10,6 @@ }, "dependencies": { "@hono/node-server": "^1.14.1", - "@octokit/rest": "^22.0.1", "hono": "^4.7.6", "yaml": "^2.8.3", "zod": "^4.4.3", diff --git a/packages/server/src/cache.test.ts b/packages/server/src/cache.test.ts index 83323f3..b366a39 100644 --- a/packages/server/src/cache.test.ts +++ b/packages/server/src/cache.test.ts @@ -1,120 +1,27 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { FsMock } from "./test-utils/fs-mock.js"; - -vi.mock("node:fs", async () => { - const { createFsMock } = await import("./test-utils/fs-mock.js"); - return createFsMock(); -}); - -async function freshCache() { - vi.resetModules(); - return import("./cache.js"); -} - -async function getFsMock(): Promise { - return (await import("node:fs")) as unknown as FsMock; -} - -beforeEach(async () => { - (await getFsMock()).__store.clear(); +import { afterEach, expect, it, vi } from "vitest"; +const fs = vi.hoisted(() => ({ + existsSync: vi.fn(() => true), + readFileSync: vi.fn(() => + JSON.stringify({ "github:prs": { data: [{ number: 1 }] } }), + ), +})); +vi.mock("node:fs", () => fs); +afterEach(() => { + vi.unstubAllEnvs(); vi.clearAllMocks(); }); - -describe("cache in-memory", () => { - it("returns null for missing key", async () => { - const { getCached } = await freshCache(); - expect(getCached("missing")).toBeNull(); - }); - - it("roundtrips data via set/get", async () => { - const { getCached, setCached } = await freshCache(); - setCached("k", { a: 1 }); - expect(getCached<{ a: number }>("k")).toEqual({ a: 1 }); - }); - - it("cacheAge is null when key is missing", async () => { - const { cacheAge } = await freshCache(); - expect(cacheAge("missing")).toBeNull(); - }); - - it("cacheAge is near-zero immediately after setCached", async () => { - const { setCached, cacheAge } = await freshCache(); - setCached("k", 1); - const age = cacheAge("k"); - expect(age).not.toBeNull(); - expect(age!).toBeGreaterThanOrEqual(0); - expect(age!).toBeLessThan(1000); - }); - - it("setCached overwrites prior value", async () => { - const { setCached, getCached } = await freshCache(); - setCached("k", "first"); - setCached("k", "second"); - expect(getCached("k")).toBe("second"); - }); -}); - -describe("cache persistence", () => { - it("setCached writes the store to disk as JSON", async () => { - const { setCached } = await freshCache(); - const fs = await getFsMock(); - setCached("k", { hello: "world" }); - expect(fs.writeFileSync).toHaveBeenCalled(); - const written = fs.writeFileSync.mock.calls.at(-1)![1]; - const parsed = JSON.parse(written); - expect(parsed.k.data).toEqual({ hello: "world" }); - expect(typeof parsed.k.updatedAt).toBe("string"); - }); - - it("loadCache reads prior entries from disk", async () => { - const fs = await getFsMock(); - // Discover the path cache.ts writes to, then seed it with known content. - const probe = await freshCache(); - probe.setCached("probe", 0); - const path = fs.writeFileSync.mock.calls.at(-1)![0]; - - fs.__store.clear(); - fs.__store.set( - path, - JSON.stringify({ - foo: { data: "bar", updatedAt: new Date().toISOString() }, - }), - ); - - const reloaded = await freshCache(); - reloaded.loadCache(); - expect(reloaded.getCached("foo")).toBe("bar"); - }); - - it("loadCache is a no-op when no cache file exists", async () => { - const fs = await getFsMock(); - const cache = await freshCache(); - expect(() => cache.loadCache()).not.toThrow(); - expect(fs.readFileSync).not.toHaveBeenCalled(); - }); - - it("loadCache tolerates malformed JSON without throwing", async () => { - const fs = await getFsMock(); - const probe = await freshCache(); - probe.setCached("probe", 0); - const path = fs.writeFileSync.mock.calls.at(-1)![0]; - - fs.__store.clear(); - fs.__store.set(path, "{ not json"); - - const reloaded = await freshCache(); - expect(() => reloaded.loadCache()).not.toThrow(); - expect(reloaded.getCached("anything")).toBeNull(); - }); +it("never loads the old JSON cache for live data", async () => { + vi.resetModules(); + vi.stubEnv("DEMO", "0"); + const cache = await import("./cache.js"); + cache.loadCache(); + expect(cache.getCached("github:prs")).toBeNull(); + expect(fs.readFileSync).not.toHaveBeenCalled(); }); - -describe("cache in DEMO mode", () => { - it("setCached skips writing to disk", async () => { - vi.stubEnv("DEMO", "1"); - const { setCached } = await freshCache(); - const fs = await getFsMock(); - setCached("k", "v"); - expect(fs.writeFileSync).not.toHaveBeenCalled(); - vi.unstubAllEnvs(); - }); +it("loads read-only demo fixtures", async () => { + vi.resetModules(); + vi.stubEnv("DEMO", "1"); + const cache = await import("./cache.js"); + cache.loadCache(); + expect(cache.getCached("github:prs")).toEqual([{ number: 1 }]); }); diff --git a/packages/server/src/cache.ts b/packages/server/src/cache.ts index 70471ab..f811267 100644 --- a/packages/server/src/cache.ts +++ b/packages/server/src/cache.ts @@ -1,75 +1,23 @@ -import { readFileSync, writeFileSync, mkdirSync, existsSync } from "node:fs"; +import { readFileSync, existsSync } from "node:fs"; import { resolve } from "node:path"; -const DEMO = process.env.DEMO === "1"; - -function resolveCacheDir(): string { - if (DEMO) return resolve(import.meta.dirname, "../../../.cache-demo"); - if (process.env.GHD_DATA_DIR) - return resolve(process.env.GHD_DATA_DIR, ".cache"); - return resolve(import.meta.dirname, "../../../.cache"); -} - -const CACHE_DIR = resolveCacheDir(); -const CACHE_FILE = resolve(CACHE_DIR, "data.json"); - -interface CacheEntry { - data: unknown; - updatedAt: string; -} - -const store = new Map(); - -// Load from disk on startup +// Read-only demo fixtures. Live data is owned exclusively by the sync engine. +const store = new Map(); export function loadCache() { + if (process.env.DEMO !== "1") return; + const path = resolve(import.meta.dirname, "../../../.cache-demo/data.json"); try { - if (existsSync(CACHE_FILE)) { - const raw = JSON.parse(readFileSync(CACHE_FILE, "utf-8")); - for (const [key, entry] of Object.entries(raw)) { - if (!DEMO && /:(prs|reviews|notifications)$/.test(key)) continue; - store.set(key, entry as CacheEntry); + if (existsSync(path)) { + const entries = JSON.parse(readFileSync(path, "utf-8")); + for (const [key, entry] of Object.entries(entries)) { + store.set(key, entry as { data: unknown }); } - console.log(`Cache loaded: ${store.size} entries`); } } catch { - console.log("No existing cache found, starting fresh"); - } -} - -function persistCache() { - if (DEMO) return; - try { - mkdirSync(CACHE_DIR, { recursive: true }); - const obj: Record = {}; - for (const [key, entry] of store) { - obj[key] = entry; - } - writeFileSync(CACHE_FILE, JSON.stringify(obj)); - } catch (err) { - console.error("Failed to persist cache:", err); + console.log("Could not load demo fixtures"); } } export function getCached(key: string): T | null { - const entry = store.get(key); - if (!entry) return null; - return entry.data as T; -} - -export function setCached(key: string, data: unknown) { - store.set(key, { data, updatedAt: new Date().toISOString() }); - persistCache(); -} - -export function cacheAge(key: string): number | null { - const entry = store.get(key); - if (!entry) return null; - return Date.now() - new Date(entry.updatedAt).getTime(); -} - -/** Ancillary REST caches only; persistent lists live in the shared engine. */ -export function clearCache() { - if (DEMO) return; - store.clear(); - persistCache(); + return (store.get(key)?.data as T) ?? null; } diff --git a/packages/server/src/config.test.ts b/packages/server/src/config.test.ts index fe2fa86..5279a44 100644 --- a/packages/server/src/config.test.ts +++ b/packages/server/src/config.test.ts @@ -13,12 +13,18 @@ const octokitHolder = vi.hoisted( }), ); -vi.mock("@octokit/rest", async () => { +vi.mock("sync", async () => { const { octokitStub } = await import("./test-utils/octokit-mock.js"); octokitHolder.ctor = octokitStub({ login: "alice" }); return { - get Octokit() { - return octokitHolder.ctor; + authenticate: async (instance: { token: string; baseUrl: string }) => { + const client = new octokitHolder.ctor({ + auth: instance.token, + baseUrl: instance.baseUrl, + }) as { + users: { getAuthenticated: () => Promise<{ data: { login: string } }> }; + }; + return (await client.users.getAuthenticated()).data.login; }, }; }); diff --git a/packages/server/src/config.ts b/packages/server/src/config.ts index aa543f0..9975768 100644 --- a/packages/server/src/config.ts +++ b/packages/server/src/config.ts @@ -2,7 +2,7 @@ import { spawn } from "node:child_process"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join } from "node:path"; -import { Octokit } from "@octokit/rest"; +import { authenticate } from "sync"; import { parse } from "yaml"; import { z } from "zod"; @@ -215,9 +215,13 @@ async function probeInstance( baseUrl: string, ): Promise { try { - const client = new Octokit({ auth: token, baseUrl }); - const { data } = await client.users.getAuthenticated(); - return { ok: true, username: data.login }; + const username = await authenticate({ + id: baseUrl, + label: baseUrl, + token, + baseUrl, + }); + return { ok: true, username }; } catch (err) { return classifyProbeError(err); } diff --git a/packages/server/src/fetchers.integration.test.ts b/packages/server/src/fetchers.integration.test.ts deleted file mode 100644 index 258d97e..0000000 --- a/packages/server/src/fetchers.integration.test.ts +++ /dev/null @@ -1,508 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const { mockOctokit, cacheStore } = vi.hoisted(() => ({ - mockOctokit: { - search: { issuesAndPullRequests: vi.fn() }, - pulls: { get: vi.fn(), listReviews: vi.fn() }, - issues: { listEventsForTimeline: vi.fn() }, - checks: { listForRef: vi.fn() }, - repos: { getCombinedStatusForRef: vi.fn(), get: vi.fn() }, - activity: { listNotificationsForAuthenticatedUser: vi.fn() }, - graphql: vi.fn(), - }, - cacheStore: new Map(), -})); - -vi.mock("./github-client.js", () => ({ - getClient: async () => mockOctokit, - getInstance: async (id: string) => ({ - id, - label: id, - baseUrl: "https://api.github.com", - token: "t", - username: "alice", - }), -})); - -const cacheTimes = new Map(); -vi.mock("./cache.js", () => ({ - getCached: (key: string) => cacheStore.get(key) ?? null, - setCached: (key: string, data: unknown) => { - cacheStore.set(key, data); - cacheTimes.set(key, Date.now()); - }, - cacheAge: (key: string) => { - const t = cacheTimes.get(key); - return t == null ? null : Date.now() - t; - }, -})); - -const { fetchPrs, fetchNotifications, fetchReviews, notificationHtmlUrl } = - await import("./fetchers.js"); - -beforeEach(() => { - vi.clearAllMocks(); - cacheStore.clear(); - // Safe defaults - mockOctokit.pulls.listReviews.mockResolvedValue({ data: [] }); - mockOctokit.issues.listEventsForTimeline.mockResolvedValue({ data: [] }); - mockOctokit.repos.get.mockResolvedValue({ - data: { allow_auto_merge: false }, - }); - mockOctokit.pulls.get.mockResolvedValue({ - data: { - body: "", - head: { ref: "feat" }, - base: { ref: "main" }, - additions: 0, - deletions: 0, - commits: 0, - comments: 0, - review_comments: 0, - mergeable: true, - requested_reviewers: [], - requested_teams: [], - }, - }); - mockOctokit.checks.listForRef.mockResolvedValue({ - data: { check_runs: [{ status: "completed", conclusion: "success" }] }, - }); - mockOctokit.repos.getCombinedStatusForRef.mockResolvedValue({ - data: { statuses: [] }, - }); - mockOctokit.graphql.mockResolvedValue({}); -}); - -function prSearchItem(over: Partial> = {}) { - return { - id: 1, - node_id: "PR_1", - number: 5, - title: "t", - html_url: "http://x", - repository_url: "https://api.github.com/repos/o/r", - created_at: "2026-01-01T00:00:00Z", - updated_at: "2026-01-02T00:00:00Z", - user: { login: "alice", avatar_url: "" }, - draft: false, - labels: [{ name: "bug" }], - ...over, - }; -} - -describe("fetchPrs", () => { - it("queries authored open PRs and returns shaped results", async () => { - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { items: [prSearchItem()] }, - }); - const prs = await fetchPrs("github"); - expect(mockOctokit.search.issuesAndPullRequests).toHaveBeenCalledWith( - expect.objectContaining({ - q: "author:alice type:pr state:open", - sort: "updated", - order: "desc", - }), - ); - expect(prs).toHaveLength(1); - expect(prs[0]).toMatchObject({ - number: 5, - repo: "o/r", - author: "alice", - ciStatus: "success", - labels: ["bug"], - }); - }); - - it("filters out PRs whose author doesn't match (defensive)", async () => { - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { - items: [ - prSearchItem({ user: { login: "alice", avatar_url: "" } }), - prSearchItem({ - id: 2, - node_id: "PR_2", - user: { login: "bot", avatar_url: "" }, - }), - ], - }, - }); - const prs = await fetchPrs("github"); - expect(prs.map((p) => p.author)).toEqual(["alice"]); - }); - - it("populates mergeQueue + autoMerge + reviewDecision + threads from GraphQL", async () => { - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { items: [prSearchItem()] }, - }); - mockOctokit.graphql.mockResolvedValue({ - pr0: { - id: "PR_1", - mergeQueueEntry: { id: "MQ_1" }, - autoMergeRequest: { enabledAt: "x" }, - reviewDecision: "APPROVED", - mergeStateStatus: "BLOCKED", - reviewThreads: { - nodes: [ - { isResolved: false }, - { isResolved: false }, - { isResolved: true }, - ], - }, - }, - }); - const prs = await fetchPrs("github"); - expect(prs[0]).toMatchObject({ - inMergeQueue: true, - autoMerge: true, - reviewDecision: "APPROVED", - mergeStateStatus: "BLOCKED", - unresolvedThreadCount: 2, - }); - }); - - it("swallows GraphQL errors and leaves merge-queue fields as defaults", async () => { - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { items: [prSearchItem()] }, - }); - mockOctokit.graphql.mockRejectedValue(new Error("graphql down")); - const prs = await fetchPrs("github"); - expect(prs[0]).toMatchObject({ - inMergeQueue: false, - autoMerge: false, - reviewDecision: null, - mergeStateStatus: null, - unresolvedThreadCount: 0, - }); - }); - - it("handles empty search results", async () => { - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { items: [] }, - }); - const prs = await fetchPrs("github"); - expect(prs).toEqual([]); - expect(mockOctokit.graphql).not.toHaveBeenCalled(); - }); - - it("attaches autoMergeAllowed from repo settings and caches per repo", async () => { - mockOctokit.repos.get.mockResolvedValue({ - data: { allow_auto_merge: true }, - }); - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { - items: [ - prSearchItem(), - prSearchItem({ id: 2, node_id: "PR_2", number: 6 }), - ], - }, - }); - const prs = await fetchPrs("github"); - expect(prs.map((p) => p.autoMergeAllowed)).toEqual([true, true]); - // Both PRs share the same repo — settings call should hit cache after the first. - expect(mockOctokit.repos.get).toHaveBeenCalledTimes(1); - }); - - it("defaults autoMergeAllowed to false when repos.get fails", async () => { - mockOctokit.repos.get.mockRejectedValue(new Error("403")); - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { items: [prSearchItem()] }, - }); - const prs = await fetchPrs("github"); - expect(prs[0].autoMergeAllowed).toBe(false); - }); -}); - -describe("fetchNotifications", () => { - // fetchNotifications fans out across multiple pages — first call returns - // the test data, subsequent pages are empty. - const setNotifications = (data: unknown[]) => { - mockOctokit.activity.listNotificationsForAuthenticatedUser - .mockResolvedValueOnce({ data }) - .mockResolvedValue({ data: [] }); - }; - - it("omits notifications already represented elsewhere in the dashboard", async () => { - setNotifications([ - { - id: "keep-mention", - subject: { title: "x", type: "PullRequest", url: "http://x" }, - reason: "mention", - repository: { full_name: "o/r" }, - updated_at: "2026-01-01T00:00:00Z", - unread: true, - }, - { - id: "keep-author-issue", - subject: { title: "issue I opened", type: "Issue", url: "http://i" }, - reason: "author", - repository: { full_name: "o/r" }, - updated_at: "2026-01-01T00:00:00Z", - unread: true, - }, - { - id: "drop-review-requested", - subject: { title: "y", type: "PullRequest", url: "http://y" }, - reason: "review_requested", - repository: { full_name: "o/r" }, - updated_at: "2026-01-02T00:00:00Z", - unread: true, - }, - { - id: "drop-ci", - subject: { title: "z", type: "CheckSuite", url: null }, - reason: "ci_activity", - repository: { full_name: "o/r" }, - updated_at: "2026-01-03T00:00:00Z", - unread: true, - }, - { - id: "drop-author-pr", - subject: { title: "my pr", type: "PullRequest", url: "http://p" }, - reason: "author", - repository: { full_name: "o/r" }, - updated_at: "2026-01-04T00:00:00Z", - unread: true, - }, - { - id: "drop-state-change-pr", - subject: { title: "merged pr", type: "PullRequest", url: "http://m" }, - reason: "state_change", - repository: { full_name: "o/r" }, - updated_at: "2026-01-05T00:00:00Z", - unread: true, - }, - { - id: "drop-subscribed", - subject: { title: "sub", type: "Issue", url: "http://s" }, - reason: "subscribed", - repository: { full_name: "o/r" }, - updated_at: "2026-01-06T00:00:00Z", - unread: true, - }, - ]); - const notifs = await fetchNotifications("github"); - expect(notifs.map((n) => n.id)).toEqual([ - "keep-mention", - "keep-author-issue", - ]); - }); - - it("shapes the payload for the UI and rewrites the subject URL to its HTML form", async () => { - setNotifications([ - { - id: "1", - subject: { - title: "hello", - type: "Issue", - url: "https://api.github.com/repos/o/r/issues/42", - }, - reason: "mention", - repository: { full_name: "o/r" }, - updated_at: "2026-01-01T00:00:00Z", - unread: false, - }, - ]); - const [n] = await fetchNotifications("github"); - expect(n).toEqual({ - id: "1", - title: "hello", - type: "Issue", - reason: "mention", - repo: "o/r", - updatedAt: "2026-01-01T00:00:00Z", - unread: false, - url: "https://github.com/o/r/issues/42", - }); - }); - - it("fans out across multiple pages and merges the results", async () => { - const make = (id: string) => ({ - id, - subject: { title: id, type: "Issue", url: `http://i/${id}` }, - reason: "mention", - repository: { full_name: "o/r" }, - updated_at: "2026-01-01T00:00:00Z", - unread: true, - }); - mockOctokit.activity.listNotificationsForAuthenticatedUser - .mockResolvedValueOnce({ data: [make("p1-a"), make("p1-b")] }) - .mockResolvedValueOnce({ data: [make("p2-a")] }) - .mockResolvedValueOnce({ data: [make("p3-a")] }); - - const notifs = await fetchNotifications("github"); - expect(notifs.map((n) => n.id)).toEqual(["p1-a", "p1-b", "p2-a", "p3-a"]); - expect( - mockOctokit.activity.listNotificationsForAuthenticatedUser, - ).toHaveBeenCalledTimes(3); - }); -}); - -describe("notificationHtmlUrl", () => { - const apiBase = "https://api.github.com"; - - it("rewrites issue API URLs to HTML form", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/issues/42", - "Issue", - "o/r", - apiBase, - ), - ).toBe("https://github.com/o/r/issues/42"); - }); - - it("rewrites pulls API URLs (pulls → pull) for PRs", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/pulls/123", - "PullRequest", - "o/r", - apiBase, - ), - ).toBe("https://github.com/o/r/pull/123"); - }); - - it("rewrites commits API URLs (commits → commit)", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/commits/abc123", - "Commit", - "o/r", - apiBase, - ), - ).toBe("https://github.com/o/r/commit/abc123"); - }); - - it("falls back to the repo's releases page for Release subjects (the ID isn't UI-routable)", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/releases/999", - "Release", - "o/r", - apiBase, - ), - ).toBe("https://github.com/o/r/releases"); - }); - - it("falls back to the repo's discussions page when subject.url is missing (Discussion)", () => { - expect(notificationHtmlUrl(null, "Discussion", "o/r", apiBase)).toBe( - "https://github.com/o/r/discussions", - ); - }); - - it("falls back to the repo page when subject.url is missing and type is unknown", () => { - expect(notificationHtmlUrl(null, "CheckSuite", "o/r", apiBase)).toBe( - "https://github.com/o/r", - ); - }); - - it("handles GHES URLs by stripping the /api/v3 prefix and keeping the host", () => { - expect( - notificationHtmlUrl( - "https://ghe.example.com/api/v3/repos/o/r/pulls/7", - "PullRequest", - "o/r", - "https://ghe.example.com/api/v3", - ), - ).toBe("https://ghe.example.com/o/r/pull/7"); - }); - - it("falls back to the repo page when subject.url is unparseable", () => { - expect(notificationHtmlUrl("not a url", "Issue", "o/r", apiBase)).toBe( - "https://github.com/o/r", - ); - }); - - it("appends an issuecomment fragment when latest_comment_url points at an issue comment", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/issues/42", - "Issue", - "o/r", - apiBase, - "https://api.github.com/repos/o/r/issues/comments/9001", - ), - ).toBe("https://github.com/o/r/issues/42#issuecomment-9001"); - }); - - it("appends a discussion_r fragment for PR review comments", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/pulls/7", - "PullRequest", - "o/r", - apiBase, - "https://api.github.com/repos/o/r/pulls/comments/555", - ), - ).toBe("https://github.com/o/r/pull/7#discussion_r555"); - }); - - it("appends a commitcomment fragment for commit comments", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/commits/abc", - "Commit", - "o/r", - apiBase, - "https://api.github.com/repos/o/r/comments/77", - ), - ).toBe("https://github.com/o/r/commit/abc#commitcomment-77"); - }); - - it("handles GHES comment URLs by stripping the /api/v3 prefix", () => { - expect( - notificationHtmlUrl( - "https://ghe.example.com/api/v3/repos/o/r/pulls/7", - "PullRequest", - "o/r", - "https://ghe.example.com/api/v3", - "https://ghe.example.com/api/v3/repos/o/r/issues/comments/12", - ), - ).toBe("https://ghe.example.com/o/r/pull/7#issuecomment-12"); - }); - - it("ignores latest_comment_url when it doesn't match a known shape", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/issues/42", - "Issue", - "o/r", - apiBase, - "https://api.github.com/repos/o/r/reviews/9001", - ), - ).toBe("https://github.com/o/r/issues/42"); - }); - - it("does not append a comment fragment when falling back to a list page (Release)", () => { - expect( - notificationHtmlUrl( - "https://api.github.com/repos/o/r/releases/999", - "Release", - "o/r", - apiBase, - "https://api.github.com/repos/o/r/issues/comments/9001", - ), - ).toBe("https://github.com/o/r/releases"); - }); -}); - -describe("fetchReviews", () => { - it("returns GitHub search results without classifying review requests", async () => { - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { - items: [prSearchItem({ user: { login: "bob", avatar_url: "" } })], - }, - }); - - const reviews = await fetchReviews("github"); - - expect(mockOctokit.search.issuesAndPullRequests).toHaveBeenCalledWith( - expect.objectContaining({ - q: "review-requested:alice type:pr state:open", - }), - ); - expect(reviews).toHaveLength(1); - expect(reviews[0]).not.toHaveProperty("autoAssigned"); - expect(mockOctokit.issues.listEventsForTimeline).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/server/src/fetchers.test.ts b/packages/server/src/fetchers.test.ts deleted file mode 100644 index 23afe2f..0000000 --- a/packages/server/src/fetchers.test.ts +++ /dev/null @@ -1,266 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; -import { - getCiStatus, - latestCheckRunsByName, - summarizeReviews, -} from "./fetchers.js"; - -describe("summarizeReviews", () => { - it("returns empty arrays when no reviews", () => { - expect(summarizeReviews([])).toEqual({ - approved: [], - changesRequested: [], - }); - }); - - it("collects approvers and change-requesters", () => { - const result = summarizeReviews([ - { state: "APPROVED", user: { login: "alice" } }, - { state: "CHANGES_REQUESTED", user: { login: "bob" } }, - ]); - expect(result.approved).toEqual(["alice"]); - expect(result.changesRequested).toEqual(["bob"]); - }); - - it("keeps only each reviewer's latest non-comment state", () => { - const result = summarizeReviews([ - { state: "CHANGES_REQUESTED", user: { login: "alice" } }, - { state: "APPROVED", user: { login: "alice" } }, - ]); - expect(result.approved).toEqual(["alice"]); - expect(result.changesRequested).toEqual([]); - }); - - it("ignores COMMENTED reviews so they don't overwrite approval", () => { - const result = summarizeReviews([ - { state: "APPROVED", user: { login: "alice" } }, - { state: "COMMENTED", user: { login: "alice" } }, - ]); - expect(result.approved).toEqual(["alice"]); - }); - - it("skips reviews with no user", () => { - const result = summarizeReviews([{ state: "APPROVED", user: null }]); - expect(result.approved).toEqual([]); - }); - - it("dismissed review removes approval", () => { - const result = summarizeReviews([ - { state: "APPROVED", user: { login: "alice" } }, - { state: "DISMISSED", user: { login: "alice" } }, - ]); - expect(result.approved).toEqual([]); - expect(result.changesRequested).toEqual([]); - }); -}); - -function mockClient(overrides: { - statuses?: { state: string }[]; - checks?: { - name?: string; - status: string; - conclusion: string | null; - completed_at?: string | null; - }[]; - statusesError?: boolean; - checksError?: boolean; -}) { - return { - repos: { - getCombinedStatusForRef: vi.fn(() => - overrides.statusesError - ? Promise.reject(new Error("boom")) - : Promise.resolve({ data: { statuses: overrides.statuses ?? [] } }), - ), - }, - checks: { - listForRef: vi.fn(() => - overrides.checksError - ? Promise.reject(new Error("boom")) - : Promise.resolve({ data: { check_runs: overrides.checks ?? [] } }), - ), - }, - // biome-ignore lint: test mock — we intentionally type it as any - } as any; -} - -describe("getCiStatus", () => { - it("returns 'unknown' when neither statuses nor checks exist", async () => { - const result = await getCiStatus(mockClient({}), "o", "r", "ref"); - expect(result).toBe("unknown"); - }); - - it("returns 'pending' when any check run is not completed", async () => { - const result = await getCiStatus( - mockClient({ - checks: [ - { name: "lint", status: "in_progress", conclusion: null }, - { name: "test", status: "completed", conclusion: "success" }, - ], - }), - "o", - "r", - "ref", - ); - expect(result).toBe("pending"); - }); - - it("ignores stale check attempts when a newer one passed (rerun)", async () => { - const result = await getCiStatus( - mockClient({ - checks: [ - { - name: "Validate Title", - status: "completed", - conclusion: "failure", - completed_at: "2026-04-28T10:00:00Z", - }, - { - name: "Validate Title", - status: "completed", - conclusion: "success", - completed_at: "2026-04-28T11:00:00Z", - }, - { - name: "Tests", - status: "completed", - conclusion: "success", - completed_at: "2026-04-28T11:00:00Z", - }, - ], - }), - "o", - "r", - "ref", - ); - expect(result).toBe("success"); - }); - - it("uses the latest attempt regardless of API ordering", async () => { - const result = await getCiStatus( - mockClient({ - checks: [ - { - name: "Build", - status: "completed", - conclusion: "success", - completed_at: "2026-04-28T10:00:00Z", - }, - { - name: "Build", - status: "completed", - conclusion: "failure", - completed_at: "2026-04-28T11:00:00Z", - }, - ], - }), - "o", - "r", - "ref", - ); - expect(result).toBe("failure"); - }); - - it("returns 'failure' on failed legacy status", async () => { - const result = await getCiStatus( - mockClient({ statuses: [{ state: "failure" }] }), - "o", - "r", - "ref", - ); - expect(result).toBe("failure"); - }); - - it("returns 'failure' on errored legacy status", async () => { - const result = await getCiStatus( - mockClient({ statuses: [{ state: "error" }] }), - "o", - "r", - "ref", - ); - expect(result).toBe("failure"); - }); - - it.each(["failure", "timed_out", "cancelled"])( - "returns 'failure' when a completed check has conclusion %s", - async (conclusion) => { - const result = await getCiStatus( - mockClient({ checks: [{ status: "completed", conclusion }] }), - "o", - "r", - "ref", - ); - expect(result).toBe("failure"); - }, - ); - - it("returns 'success' when everything completed successfully", async () => { - const result = await getCiStatus( - mockClient({ - statuses: [{ state: "success" }], - checks: [{ status: "completed", conclusion: "success" }], - }), - "o", - "r", - "ref", - ); - expect(result).toBe("success"); - }); - - it("tolerates both API calls failing (returns 'unknown')", async () => { - const result = await getCiStatus( - mockClient({ statusesError: true, checksError: true }), - "o", - "r", - "ref", - ); - expect(result).toBe("unknown"); - }); -}); - -describe("latestCheckRunsByName", () => { - it("keeps the most recent attempt per name by completed_at", () => { - const result = latestCheckRunsByName([ - { - name: "a", - status: "completed", - conclusion: "failure", - completed_at: "2026-04-28T10:00:00Z", - }, - { - name: "a", - status: "completed", - conclusion: "success", - completed_at: "2026-04-28T11:00:00Z", - }, - { - name: "b", - status: "completed", - conclusion: "success", - completed_at: "2026-04-28T09:00:00Z", - }, - ]); - expect(result).toHaveLength(2); - expect(result.find((r) => r.name === "a")?.conclusion).toBe("success"); - expect(result.find((r) => r.name === "b")?.conclusion).toBe("success"); - }); - - it("falls back to started_at when completed_at is missing", () => { - const result = latestCheckRunsByName([ - { - name: "a", - status: "in_progress", - conclusion: null, - started_at: "2026-04-28T11:00:00Z", - }, - { - name: "a", - status: "completed", - conclusion: "failure", - completed_at: "2026-04-28T10:00:00Z", - }, - ]); - expect(result).toHaveLength(1); - expect(result[0].status).toBe("in_progress"); - }); -}); diff --git a/packages/server/src/fetchers.ts b/packages/server/src/fetchers.ts deleted file mode 100644 index 1f38f5a..0000000 --- a/packages/server/src/fetchers.ts +++ /dev/null @@ -1,482 +0,0 @@ -import type { Octokit } from "@octokit/rest"; -import { cacheAge, getCached, setCached } from "./cache.js"; -import { getClient, getInstance } from "./github-client.js"; - -interface RepoSettings { - autoMergeAllowed: boolean; -} - -const inFlightRepoSettings = new Map>(); -let repoSettingsGeneration = 0; -export function clearRepoSettings() { - repoSettingsGeneration++; - inFlightRepoSettings.clear(); -} - -const REPO_SETTINGS_TTL_MS = 2 * 60 * 1000; - -export async function getRepoSettings( - client: Octokit, - instanceId: string, - owner: string, - repo: string, -): Promise { - const key = `${instanceId}:repo-settings:${owner}/${repo}`; - const age = cacheAge(key); - const cached = - age !== null && age < REPO_SETTINGS_TTL_MS - ? getCached(key) - : null; - if (cached) return cached; - const existing = inFlightRepoSettings.get(key); - if (existing) return existing; - const generation = repoSettingsGeneration; - const promise = (async () => { - try { - const { data } = await client.repos.get({ owner, repo }); - const settings: RepoSettings = { - autoMergeAllowed: data.allow_auto_merge ?? false, - }; - if (generation === repoSettingsGeneration) setCached(key, settings); - return settings; - } catch { - return { autoMergeAllowed: false }; - } finally { - if (generation === repoSettingsGeneration) - inFlightRepoSettings.delete(key); - } - })(); - inFlightRepoSettings.set(key, promise); - return promise; -} - -type Review = { state: string; user: { login: string } | null }; - -interface MergeQueueInfo { - inMergeQueue: boolean; - autoMerge: boolean; - reviewDecision: string | null; - mergeStateStatus: string | null; - unresolvedThreadCount: number; -} - -export async function fetchMergeQueueStatus( - client: Octokit, - items: { node_id: string }[], -): Promise> { - const result = new Map(); - if (items.length === 0) return result; - - // Build a batched GraphQL query - const aliases = items.map((item, i) => { - return `pr${i}: node(id: "${item.node_id}") { ... on PullRequest { id mergeQueueEntry { id } autoMergeRequest { enabledAt } reviewDecision mergeStateStatus reviewThreads(first: 100) { nodes { isResolved } } } }`; - }); - - try { - const response = await client.graphql< - Record< - string, - { - id: string; - mergeQueueEntry: { id: string } | null; - autoMergeRequest: { enabledAt: string } | null; - reviewDecision: string | null; - mergeStateStatus: string | null; - reviewThreads: { nodes: { isResolved: boolean }[] } | null; - } | null - > - >(`query { ${aliases.join("\n")} }`); - - for (let i = 0; i < items.length; i++) { - const pr = response[`pr${i}`]; - const threads = pr?.reviewThreads?.nodes ?? []; - const unresolvedThreadCount = threads.filter((t) => !t.isResolved).length; - result.set(items[i].node_id, { - inMergeQueue: pr?.mergeQueueEntry != null, - autoMerge: pr?.autoMergeRequest != null, - reviewDecision: pr?.reviewDecision ?? null, - mergeStateStatus: pr?.mergeStateStatus ?? null, - unresolvedThreadCount, - }); - } - } catch { - // If GraphQL fails, return empty — don't break the whole fetch - } - - return result; -} - -type CheckRun = { - name: string; - status: string; - conclusion: string | null; - started_at?: string | null; - completed_at?: string | null; -}; - -// listForRef can return multiple attempts of the same named check (e.g. after -// a rerun). Keep only the most recent attempt per name so a stale failure -// doesn't outvote a later success. -export function latestCheckRunsByName(runs: T[]): T[] { - const latest = new Map(); - const ts = (r: T) => { - const t = r.completed_at ?? r.started_at; - return t ? new Date(t).getTime() : 0; - }; - for (const r of runs) { - const existing = latest.get(r.name); - if (!existing || ts(r) >= ts(existing)) { - latest.set(r.name, r); - } - } - return [...latest.values()]; -} - -export async function getCiStatus( - client: Octokit, - owner: string, - repo: string, - ref: string, -): Promise { - const [statusRes, checksRes] = await Promise.all([ - client.repos - .getCombinedStatusForRef({ owner, repo, ref }) - .catch(() => null), - client.checks - .listForRef({ owner, repo, ref, per_page: 100 }) - .catch(() => null), - ]); - - const statuses = statusRes?.data.statuses ?? []; - const checkRuns = latestCheckRunsByName(checksRes?.data.check_runs ?? []); - - // No CI configured at all - if (statuses.length === 0 && checkRuns.length === 0) return "unknown"; - - // Check if anything is still running - const hasRunning = checkRuns.some((cr) => cr.status !== "completed"); - if (hasRunning) return "pending"; - - // Check for failures - const hasStatusFailure = statuses.some( - (s) => s.state === "failure" || s.state === "error", - ); - const hasCheckFailure = checkRuns.some( - (cr) => - cr.conclusion === "failure" || - cr.conclusion === "timed_out" || - cr.conclusion === "cancelled", - ); - if (hasStatusFailure || hasCheckFailure) return "failure"; - - // If everything completed successfully - return "success"; -} - -export function summarizeReviews(reviews: Review[]) { - const latest = new Map(); - for (const r of reviews) { - if (!r.user) continue; - if (r.state === "COMMENTED") continue; - latest.set(r.user.login, r.state); - } - - const approved: string[] = []; - const changesRequested: string[] = []; - - for (const [user, state] of latest) { - if (state === "APPROVED") approved.push(user); - if (state === "CHANGES_REQUESTED") changesRequested.push(user); - } - - return { approved, changesRequested }; -} - -export async function fetchPrs(instanceId: string) { - const client = await getClient(instanceId); - const { username } = await getInstance(instanceId); - - const { data } = await client.search.issuesAndPullRequests({ - q: `author:${username} type:pr state:open`, - sort: "updated", - order: "desc", - per_page: 30, - }); - - // Batch GraphQL query for merge queue + auto-merge status - const mergeQueueStatus = await fetchMergeQueueStatus(client, data.items); - - const prs = await Promise.all( - data.items.map(async (item) => { - const [owner, repo] = item.repository_url.split("/").slice(-2); - const prNumber = item.number; - - const [ciStatus, reviewsRes, prRes, repoSettings] = await Promise.all([ - getCiStatus(client, owner, repo, `pull/${prNumber}/head`), - client.pulls - .listReviews({ owner, repo, pull_number: prNumber }) - .catch(() => null), - client.pulls - .get({ owner, repo, pull_number: prNumber }) - .catch(() => null), - getRepoSettings(client, instanceId, owner, repo), - ]); - - const reviews = reviewsRes?.data ?? []; - const prData = prRes?.data; - const mqStatus = mergeQueueStatus.get(item.node_id); - - return { - id: item.id, - number: prNumber, - title: item.title, - body: prData?.body ?? "", - url: item.html_url, - repo: `${owner}/${repo}`, - createdAt: item.created_at, - updatedAt: item.updated_at, - author: item.user?.login ?? "unknown", - authorAvatar: item.user?.avatar_url ?? "", - draft: item.draft ?? false, - // Not applicable for open PRs - ciStatus, - inMergeQueue: mqStatus?.inMergeQueue ?? false, - autoMerge: mqStatus?.autoMerge ?? false, - autoMergeAllowed: repoSettings.autoMergeAllowed, - headBranch: prData?.head.ref ?? "", - baseBranch: prData?.base.ref ?? "main", - reviews: summarizeReviews(reviews), - reviewDecision: mqStatus?.reviewDecision ?? null, - mergeStateStatus: mqStatus?.mergeStateStatus ?? null, - unresolvedThreadCount: mqStatus?.unresolvedThreadCount ?? 0, - additions: prData?.additions ?? 0, - deletions: prData?.deletions ?? 0, - commits: prData?.commits ?? 0, - commentCount: (prData?.comments ?? 0) + (prData?.review_comments ?? 0), - labels: item.labels.map((l) => - typeof l === "string" ? l : (l.name ?? ""), - ), - mergeable: prData?.mergeable ?? null, - }; - }), - ); - - // Filter to only include PRs where the author matches the configured username - return prs.filter((pr) => pr.author === username); -} - -export async function fetchReviews(instanceId: string) { - const client = await getClient(instanceId); - const { username } = await getInstance(instanceId); - - const { data } = await client.search.issuesAndPullRequests({ - q: `review-requested:${username} type:pr state:open`, - sort: "updated", - order: "desc", - per_page: 30, - }); - - const mergeQueueStatus = await fetchMergeQueueStatus(client, data.items); - - return Promise.all( - data.items.map(async (item) => { - const [owner, repo] = item.repository_url.split("/").slice(-2); - const prNumber = item.number; - - const [ciStatus, reviewsRes, prRes] = await Promise.all([ - getCiStatus(client, owner, repo, `pull/${prNumber}/head`), - client.pulls - .listReviews({ owner, repo, pull_number: prNumber }) - .catch(() => null), - client.pulls - .get({ owner, repo, pull_number: prNumber }) - .catch(() => null), - ]); - - const reviews = reviewsRes?.data ?? []; - const prData = prRes?.data; - const mqStatus = mergeQueueStatus.get(item.node_id); - - return { - id: item.id, - number: prNumber, - title: item.title, - body: prData?.body ?? "", - url: item.html_url, - repo: `${owner}/${repo}`, - createdAt: item.created_at, - updatedAt: item.updated_at, - author: item.user?.login ?? "unknown", - authorAvatar: item.user?.avatar_url ?? "", - draft: item.draft ?? false, - // Not applicable for open PRs - ciStatus, - inMergeQueue: mqStatus?.inMergeQueue ?? false, - autoMerge: mqStatus?.autoMerge ?? false, - headBranch: prData?.head.ref ?? "", - baseBranch: prData?.base.ref ?? "main", - reviews: summarizeReviews(reviews), - reviewDecision: mqStatus?.reviewDecision ?? null, - mergeStateStatus: mqStatus?.mergeStateStatus ?? null, - unresolvedThreadCount: mqStatus?.unresolvedThreadCount ?? 0, - additions: prData?.additions ?? 0, - deletions: prData?.deletions ?? 0, - commits: prData?.commits ?? 0, - commentCount: (prData?.comments ?? 0) + (prData?.review_comments ?? 0), - mergeable: prData?.mergeable ?? null, - }; - }), - ); -} - -// Convert a GitHub *API* URL (e.g. https://api.github.com/repos/o/r/pulls/1) -// into its corresponding *HTML* URL (https://github.com/o/r/pull/1). -// The notifications API returns the subject's API URL on `subject.url` and -// doesn't include an `html_url`, so we have to derive it ourselves. -// -// When `latestCommentUrl` is set, we append the matching fragment so the -// link opens at the comment (e.g. #issuecomment-123) rather than the top -// of the thread. -// -// Handles github.com and GHES (where the API lives under `/api/v3`). -// For subjects that don't have a usable URL (e.g. Discussion) or types we -// can't address precisely (Release IDs aren't routable in the UI), we fall -// back to a sensible repo-level page. -export function notificationHtmlUrl( - apiUrl: string | null | undefined, - type: string | null | undefined, - repoFullName: string, - apiBaseUrl: string, - latestCommentUrl?: string | null, -): string { - const htmlBase = htmlBaseFromApiBase(apiBaseUrl); - const repoUrl = `${htmlBase}/${repoFullName}`; - - if (!apiUrl) { - if (type === "Discussion") return `${repoUrl}/discussions`; - if (type === "Release") return `${repoUrl}/releases`; - return repoUrl; - } - - let path: string; - try { - path = new URL(apiUrl).pathname; - } catch { - return repoUrl; - } - - path = path.replace(/^\/api\/v3\/repos\//, "/").replace(/^\/repos\//, "/"); - path = path - .replace(/^\/([^/]+\/[^/]+)\/pulls\//, "/$1/pull/") - .replace(/^\/([^/]+\/[^/]+)\/commits\//, "/$1/commit/"); - // Release notification subjects point at /releases/{id}, which the GitHub - // UI doesn't route. Fall back to the repo's releases list — the comment - // fragment isn't meaningful on that page either. - const releaseMatch = path.match(/^\/([^/]+\/[^/]+)\/releases\/\d+$/); - if (releaseMatch) return `${htmlBase}/${releaseMatch[1]}/releases`; - - const fragment = commentFragment(latestCommentUrl); - return `${htmlBase}${path}${fragment ?? ""}`; -} - -// Map a comment's API URL to the matching HTML page fragment so the link -// scrolls to the comment. Returns null for shapes we don't recognise — the -// caller then renders the bare thread URL. -function commentFragment( - commentApiUrl: string | null | undefined, -): string | null { - if (!commentApiUrl) return null; - let path: string; - try { - path = new URL(commentApiUrl).pathname; - } catch { - return null; - } - path = path.replace(/^\/api\/v3/, ""); - let m: RegExpMatchArray | null; - if ((m = path.match(/\/issues\/comments\/(\d+)$/))) - return `#issuecomment-${m[1]}`; - if ((m = path.match(/\/pulls\/comments\/(\d+)$/))) - return `#discussion_r${m[1]}`; - if ((m = path.match(/\/repos\/[^/]+\/[^/]+\/comments\/(\d+)$/))) - return `#commitcomment-${m[1]}`; - return null; -} - -function htmlBaseFromApiBase(apiBaseUrl: string): string { - try { - const u = new URL(apiBaseUrl); - if (u.hostname === "api.github.com") return "https://github.com"; - return `${u.protocol}//${u.host}`; - } catch { - return "https://github.com"; - } -} - -// Notifications already represented elsewhere in the dashboard (My work, -// Reviews, the PR itself) or that are pure noise. Dropping them at the -// fetcher keeps the Notifications column focused on things that aren't -// surfaced anywhere else. -type NotificationShape = { - reason: string; - subject: { type: string | null }; -}; - -const redundantRules: ReadonlyArray<(n: NotificationShape) => boolean> = [ - // shown in the Reviews column - (n) => n.reason === "review_requested", - // visible on the PR itself - (n) => n.reason === "ci_activity", - // your own PR, shown in My work - (n) => n.subject.type === "PullRequest" && n.reason === "author", - // your own PR, shown in My work / Reviews - (n) => n.subject.type === "PullRequest" && n.reason === "state_change", - // auto-subscription noise - (n) => n.reason === "subscribed", -]; - -function isRedundantNotification(n: NotificationShape): boolean { - return redundantRules.some((rule) => rule(n)); -} - -// We pull a few pages so the filter has enough raw material — many items -// are dropped as redundant (review_requested, ci_activity, …), and a single -// page often leaves the kept set sparse. Pages are fetched in parallel; the -// result is cached, so this only fires on cache miss / explicit refresh. -const NOTIFICATION_PAGES = 3; -const NOTIFICATIONS_PER_PAGE = 50; - -export async function fetchNotifications(instanceId: string) { - const client = await getClient(instanceId); - const { baseUrl } = await getInstance(instanceId); - - const pages = await Promise.all( - Array.from({ length: NOTIFICATION_PAGES }, (_, i) => - client.activity.listNotificationsForAuthenticatedUser({ - all: true, - per_page: NOTIFICATIONS_PER_PAGE, - page: i + 1, - }), - ), - ); - const data = pages.flatMap((r) => r.data); - - return data - .filter((n) => !isRedundantNotification(n)) - .map((n) => ({ - id: n.id, - title: n.subject.title, - type: n.subject.type, - reason: n.reason, - repo: n.repository.full_name, - updatedAt: n.updated_at, - unread: n.unread, - url: notificationHtmlUrl( - n.subject.url, - n.subject.type, - n.repository.full_name, - baseUrl, - n.subject.latest_comment_url, - ), - })); -} diff --git a/packages/server/src/github-client.ts b/packages/server/src/github-client.ts deleted file mode 100644 index b34b15c..0000000 --- a/packages/server/src/github-client.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { Octokit } from "@octokit/rest"; -import { type GitHubInstance, getInstances } from "./config.js"; - -let generation = 0; -const clients = new Map(); - -export async function getClient(instanceId: string): Promise { - const existing = clients.get(instanceId); - if (existing) return existing; - - const currentGeneration = generation; - const instance = await getInstance(instanceId); - const client = new Octokit({ - auth: instance.token, - baseUrl: instance.baseUrl, - }); - - if (currentGeneration === generation) clients.set(instanceId, client); - return client; -} - -export async function getInstance(instanceId: string): Promise { - const instances = await getInstances(); - const instance = instances.find((i) => i.id === instanceId); - if (!instance) throw new Error(`Unknown instance: ${instanceId}`); - return instance; -} - -export function clearClients(): void { - generation++; - clients.clear(); -} diff --git a/packages/server/src/routes.test.ts b/packages/server/src/routes.test.ts index e140548..c8189e7 100644 --- a/packages/server/src/routes.test.ts +++ b/packages/server/src/routes.test.ts @@ -1,65 +1,31 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -const { cacheStore, configStub, fetchersStub, mockOctokit, syncStub } = - vi.hoisted(() => { - return { - cacheStore: new Map(), - syncStub: { - listData: vi.fn(), - recordMutation: vi.fn(), - reloadSyncConfiguration: vi.fn().mockResolvedValue(undefined), - scheduleResync: vi.fn(), - waitForPendingResyncs: vi.fn(), - markNotificationDone: vi.fn(), - approvePullRequest: vi.fn(), - togglePullRequestAutoMerge: vi.fn(), - togglePullRequestDraft: vi.fn(), - }, - configStub: { - createConfigFromExample: vi.fn(), - exampleConfig: "port: 7100\n", - getConfigStatus: vi.fn(), - invalidateConfigStatus: vi.fn(), - openInDefaultApp: vi.fn(), - readConfig: vi.fn<() => Record | null>(() => null), - resolveConfigPath: vi.fn(() => "/test/config.yml"), - }, - fetchersStub: { - clearRepoSettings: vi.fn(), - fetchPrs: vi.fn(), - fetchReviews: vi.fn(), - fetchNotifications: vi.fn(), - latestCheckRunsByName: (runs: T[]): T[] => { - const m = new Map(); - for (const r of runs) m.set(r.name, r); - return [...m.values()]; - }, - }, - mockOctokit: { - users: { getAuthenticated: vi.fn() }, - activity: { markThreadAsDone: vi.fn() }, - pulls: { - createReview: vi.fn(), - get: vi.fn(), - update: vi.fn(), - merge: vi.fn(), - listFiles: vi.fn(), - listCommits: vi.fn(), - listReviewComments: vi.fn(), - }, - issues: { listComments: vi.fn(), createComment: vi.fn() }, - checks: { listForRef: vi.fn() }, - repos: { getCombinedStatusForRef: vi.fn() }, - actions: { - listWorkflowRunsForRepo: vi.fn(), - reRunWorkflow: vi.fn(), - reRunWorkflowFailedJobs: vi.fn(), - }, - search: { issuesAndPullRequests: vi.fn() }, - graphql: vi.fn(), - }, - }; - }); +const { cacheStore, configStub, syncStub } = vi.hoisted(() => { + return { + cacheStore: new Map(), + syncStub: { + listData: vi.fn(), + getPullRequestDiff: vi.fn(), + getPullRequestComments: vi.fn(), + reloadSyncConfiguration: vi.fn().mockResolvedValue(undefined), + scheduleResync: vi.fn(), + waitForPendingResyncs: vi.fn(), + markNotificationDone: vi.fn(), + approvePullRequest: vi.fn(), + togglePullRequestAutoMerge: vi.fn(), + togglePullRequestDraft: vi.fn(), + }, + configStub: { + createConfigFromExample: vi.fn(), + exampleConfig: "port: 7100\n", + getConfigStatus: vi.fn(), + invalidateConfigStatus: vi.fn(), + openInDefaultApp: vi.fn(), + readConfig: vi.fn<() => Record | null>(() => null), + resolveConfigPath: vi.fn(() => "/test/config.yml"), + }, + }; +}); vi.mock("./cache.js", () => ({ clearCache: () => cacheStore.clear(), @@ -71,20 +37,6 @@ vi.mock("./cache.js", () => ({ vi.mock("./sync.js", () => ({ ...syncStub, getSync: () => syncStub })); vi.mock("./config.js", () => configStub); -vi.mock("./fetchers.js", () => fetchersStub); - -vi.mock("./github-client.js", () => ({ - getClient: async () => mockOctokit, - getInstance: async (id: string) => ({ - id, - label: id, - baseUrl: "https://api.github.com", - token: "t", - username: "alice", - }), - clearClients: () => {}, -})); - // Import after mocks are registered const { api } = await import("./routes.js"); const { waitForPendingResyncs } = await import("./sync.js"); @@ -211,7 +163,7 @@ describe("POST /config/create", () => { }); describe("POST /config/reload", () => { - it("invalidates diagnostics, ancillary caches and reconciles even when auth fails", async () => { + it("invalidates diagnostics and reconciles even when auth fails", async () => { configStub.getConfigStatus.mockResolvedValue({ kind: "error", errors: [{ kind: "auth", domain: "github.com", message: "Rejected" }], @@ -220,8 +172,6 @@ describe("POST /config/reload", () => { const res = await call("/config/reload", { method: "POST" }); expect(res.status).toBe(200); expect(configStub.invalidateConfigStatus).toHaveBeenCalledOnce(); - expect(fetchersStub.clearRepoSettings).toHaveBeenCalledOnce(); - expect(cacheStore.size).toBe(0); expect(syncStub.reloadSyncConfiguration).toHaveBeenCalledOnce(); }); }); @@ -284,146 +234,6 @@ it("auto-merge preserves action result and the GUI's 422 error contract", async expect(await failed.json()).toEqual({ error: "auto_merge_not_allowed" }); }); -describe("POST /:instanceId/prs/:owner/:repo/:prNumber/merge", () => { - it("merges with squash and resyncs prs, reviews", async () => { - mockOctokit.pulls.merge.mockResolvedValue({}); - fetchersStub.fetchPrs.mockResolvedValue([]); - fetchersStub.fetchReviews.mockResolvedValue([]); - const res = await call("/github/prs/o/r/5/merge", { method: "POST" }); - expect(res.status).toBe(200); - expect(mockOctokit.pulls.merge).toHaveBeenCalledWith({ - owner: "o", - repo: "r", - pull_number: 5, - merge_method: "squash", - }); - await waitForPendingResyncs(); - expect(syncStub.scheduleResync).toHaveBeenCalledWith("github", [ - "prs", - "reviews", - ]); - expect(syncStub.recordMutation).toHaveBeenCalledWith( - "github", - { - kind: "removed", - repo: "o/r", - number: 5, - }, - expect.objectContaining({ - token: "t", - baseUrl: "https://api.github.com", - }), - ); - }); - - it("forwards GitHub's full error message (joining non-empty lines) when merge is rejected", async () => { - const ghErr = Object.assign(new Error("405"), { - status: 405, - response: { - data: { - message: - "Repository rule violations found\n\nA conversation must be resolved before this pull request can be merged.\n", - }, - }, - }); - mockOctokit.pulls.merge.mockRejectedValue(ghErr); - const res = await call("/github/prs/o/r/5/merge", { method: "POST" }); - expect(res.status).toBe(422); - expect(await res.json()).toEqual({ - error: "merge_rejected", - message: - "Repository rule violations found — A conversation must be resolved before this pull request can be merged.", - }); - }); -}); - -describe("POST /:instanceId/prs/:owner/:repo/:prNumber/close", () => { - it("sets state=closed and resyncs prs, reviews", async () => { - cacheStore.set("github:prs", [{ id: 1 }]); - cacheStore.set("github:reviews", [{ id: 2 }]); - fetchersStub.fetchPrs.mockResolvedValue([]); - fetchersStub.fetchReviews.mockResolvedValue([]); - mockOctokit.pulls.update.mockResolvedValue({}); - const res = await call("/github/prs/o/r/5/close", { method: "POST" }); - expect(res.status).toBe(200); - expect(mockOctokit.pulls.update).toHaveBeenCalledWith({ - owner: "o", - repo: "r", - pull_number: 5, - state: "closed", - }); - await waitForPendingResyncs(); - expect(syncStub.scheduleResync).toHaveBeenCalledWith("github", [ - "prs", - "reviews", - ]); - expect(syncStub.recordMutation).toHaveBeenCalledWith( - "github", - { - kind: "removed", - repo: "o/r", - number: 5, - }, - expect.objectContaining({ - token: "t", - baseUrl: "https://api.github.com", - }), - ); - }); -}); - -describe("PATCH /:instanceId/prs/:owner/:repo/:prNumber", () => { - it("400s when title is missing", async () => { - const res = await call("/github/prs/o/r/5", { method: "PATCH", body: {} }); - expect(res.status).toBe(400); - expect(mockOctokit.pulls.update).not.toHaveBeenCalled(); - }); - - it("400s on invalid JSON body", async () => { - const res = await api.request("/github/prs/o/r/5", { - method: "PATCH", - headers: { "content-type": "application/json" }, - body: "{", - }); - expect(res.status).toBe(400); - }); - - it("updates title and resyncs prs", async () => { - cacheStore.set("github:prs", [{ id: 1 }]); - fetchersStub.fetchPrs.mockResolvedValue([{ id: 1, title: "new title" }]); - mockOctokit.pulls.update.mockResolvedValue({}); - const res = await call("/github/prs/o/r/5", { - method: "PATCH", - body: { title: "new title" }, - }); - expect(res.status).toBe(200); - expect(mockOctokit.pulls.update).toHaveBeenCalledWith({ - owner: "o", - repo: "r", - pull_number: 5, - title: "new title", - }); - await waitForPendingResyncs(); - expect(syncStub.scheduleResync).toHaveBeenCalledWith("github", [ - "prs", - "reviews", - ]); - expect(syncStub.recordMutation).toHaveBeenCalledWith( - "github", - { - kind: "patch", - repo: "o/r", - number: 5, - patch: { title: "new title" }, - }, - expect.objectContaining({ - token: "t", - baseUrl: "https://api.github.com", - }), - ); - }); -}); - it("draft toggle returns shared action result and refreshes both lists", async () => { syncStub.togglePullRequestDraft.mockResolvedValueOnce({ draft: false }); const res = await call("/github/prs/o/r/5/toggle-draft", { method: "POST" }); @@ -439,235 +249,37 @@ it("draft toggle returns shared action result and refreshes both lists", async ( ]); }); -describe("POST /:instanceId/prs/:owner/:repo/:prNumber/rerun-ci", () => { - it("404s when no workflow runs are found", async () => { - mockOctokit.pulls.get.mockResolvedValue({ - data: { head: { sha: "abc" }, node_id: "PR_1" }, - }); - mockOctokit.actions.listWorkflowRunsForRepo.mockResolvedValue({ - data: { workflow_runs: [] }, - }); - const res = await call("/github/prs/o/r/5/rerun-ci", { method: "POST" }); - expect(res.status).toBe(404); - expect(mockOctokit.actions.reRunWorkflow).not.toHaveBeenCalled(); - expect(mockOctokit.actions.reRunWorkflowFailedJobs).not.toHaveBeenCalled(); - }); - - it("404s when no completed PR-triggered runs are found", async () => { - mockOctokit.pulls.get.mockResolvedValue({ - data: { head: { sha: "abc" }, node_id: "PR_1" }, - }); - mockOctokit.actions.listWorkflowRunsForRepo.mockResolvedValue({ - data: { - workflow_runs: [ - // Wrong event - { - id: 1, - workflow_id: 10, - event: "push", - status: "completed", - conclusion: "success", - }, - // Still running - { - id: 2, - workflow_id: 11, - event: "pull_request", - status: "in_progress", - conclusion: null, - }, - ], - }, - }); - const res = await call("/github/prs/o/r/5/rerun-ci", { method: "POST" }); - expect(res.status).toBe(404); - expect(mockOctokit.actions.reRunWorkflow).not.toHaveBeenCalled(); - }); - - it("reruns failed jobs for failed runs and full workflow for passing runs", async () => { - mockOctokit.pulls.get.mockResolvedValue({ - data: { head: { sha: "abc" }, node_id: "PR_1" }, - }); - mockOctokit.actions.listWorkflowRunsForRepo.mockResolvedValue({ - data: { - workflow_runs: [ - // Newer attempt of workflow 20 — should win - { - id: 200, - workflow_id: 20, - event: "pull_request", - status: "completed", - conclusion: "failure", - }, - // Older attempt of workflow 20 — should be ignored - { - id: 199, - workflow_id: 20, - event: "pull_request", - status: "completed", - conclusion: "success", - }, - // Different workflow, success - { - id: 300, - workflow_id: 30, - event: "pull_request", - status: "completed", - conclusion: "success", - }, - // Wrong event - { - id: 400, - workflow_id: 40, - event: "push", - status: "completed", - conclusion: "failure", - }, - ], - }, - }); - mockOctokit.actions.reRunWorkflow.mockResolvedValue({}); - mockOctokit.actions.reRunWorkflowFailedJobs.mockResolvedValue({}); - const res = await call("/github/prs/o/r/5/rerun-ci", { method: "POST" }); - expect(res.status).toBe(200); - expect(await res.json()).toEqual({ ok: true, triggered: 2 }); - expect(mockOctokit.actions.reRunWorkflowFailedJobs).toHaveBeenCalledWith({ - owner: "o", - repo: "r", - run_id: 200, - }); - expect(mockOctokit.actions.reRunWorkflow).toHaveBeenCalledWith({ - owner: "o", - repo: "r", - run_id: 300, - }); - expect(mockOctokit.actions.reRunWorkflow).not.toHaveBeenCalledWith( - expect.objectContaining({ run_id: 400 }), - ); - }); - - it("502s when every rerun call fails", async () => { - mockOctokit.pulls.get.mockResolvedValue({ - data: { head: { sha: "abc" }, node_id: "PR_1" }, - }); - mockOctokit.actions.listWorkflowRunsForRepo.mockResolvedValue({ - data: { - workflow_runs: [ - { - id: 500, - workflow_id: 50, - event: "pull_request", - status: "completed", - conclusion: "failure", - }, - ], - }, - }); - mockOctokit.actions.reRunWorkflowFailedJobs.mockRejectedValue( - new Error("boom"), - ); - const res = await call("/github/prs/o/r/5/rerun-ci", { method: "POST" }); - expect(res.status).toBe(502); - }); -}); - -describe("GET /:instanceId/prs/:owner/:repo/:prNumber/meta", () => { - it("merges legacy statuses + checks into a unified list", async () => { - mockOctokit.pulls.listFiles.mockResolvedValue({ - data: [ - { - filename: "a.ts", - additions: 1, - deletions: 2, - status: "modified", - patch: "+x", - }, - ], - }); - mockOctokit.pulls.listCommits.mockResolvedValue({ - data: [{ sha: "abcdef1234", commit: { message: "feat: x\n\nbody" } }], - }); - mockOctokit.checks.listForRef.mockResolvedValue({ - data: { - check_runs: [ - { name: "build", status: "completed", conclusion: "success" }, - ], - }, - }); - mockOctokit.repos.getCombinedStatusForRef.mockResolvedValue({ - data: { - statuses: [ - { context: "ci/old", state: "failure" }, - { context: "build", state: "success" }, - ], - }, - }); - const res = await call("/github/prs/o/r/5/meta"); - const body = await res.json(); - expect(body.files).toEqual([ - { - filename: "a.ts", - additions: 1, - deletions: 2, - status: "modified", - patch: "+x", - }, - ]); - expect(body.commits).toEqual([{ sha: "abcdef1", message: "feat: x" }]); - // check-run takes precedence over same-named status (Map overwrite order) - const buildCheck = body.checks.find( - (c: { name: string }) => c.name === "build", - ); - expect(buildCheck.conclusion).toBe("success"); - const oldCheck = body.checks.find( - (c: { name: string }) => c.name === "ci/old", - ); - expect(oldCheck.conclusion).toBe("failure"); - }); -}); - -describe("GET /:instanceId/prs/:owner/:repo/:prNumber/comments", () => { - it("merges issue + review comments sorted by createdAt", async () => { - mockOctokit.issues.listComments.mockResolvedValue({ - data: [ - { - id: 1, - user: { login: "alice" }, - body: "first", - created_at: "2026-01-01T00:00:00Z", - }, - ], - }); - mockOctokit.pulls.listReviewComments.mockResolvedValue({ - data: [ - { - id: 2, - user: { login: "bob" }, - body: "on file", - created_at: "2026-01-02T00:00:00Z", - path: "a.ts", - }, - ], +for (const [path, method] of [ + ["diff", "getPullRequestDiff"], + ["comments", "getPullRequestComments"], +] as const) { + it(`delegates ${path} to the shared engine`, async () => { + const result = + path === "diff" + ? { headSha: "abc", files: [] } + : [{ id: 1, body: "hello" }]; + syncStub[method].mockResolvedValue(result); + const res = await call(`/github/prs/o/r/5/${path}`); + expect(await res.json()).toEqual(result); + expect(syncStub[method]).toHaveBeenCalledWith({ + instanceId: "github", + repo: "o/r", + number: 5, }); - const res = await call("/github/prs/o/r/5/comments"); - const body = await res.json(); - expect(body).toHaveLength(2); - expect(body[0].id).toBe(1); - expect(body[0].path).toBeNull(); - expect(body[1].path).toBe("a.ts"); }); -}); +} -describe("GET /:instanceId/search/prs", () => { - it("scopes the search query to the authenticated user's closed PRs", async () => { - mockOctokit.search.issuesAndPullRequests.mockResolvedValue({ - data: { items: [] }, - }); - await call("/github/search/prs?q=feature"); - const call0 = mockOctokit.search.issuesAndPullRequests.mock.calls[0][0]; - expect(call0.q).toContain("author:alice"); - expect(call0.q).toContain("type:pr"); - expect(call0.q).toContain("state:closed"); - expect(call0.q).toContain("feature"); +for (const [method, path] of [ + ["POST", "/github/prs/o/r/5/merge"], + ["POST", "/github/prs/o/r/5/close"], + ["PATCH", "/github/prs/o/r/5"], + ["POST", "/github/prs/o/r/5/rerun-ci"], + ["POST", "/github/prs/o/r/5/comment"], + ["GET", "/github/prs/o/r/5/meta"], + ["GET", "/github/search/prs?q=test"], + ["GET", "/github/users/alice/prs"], +]) { + it(`removes unsupported endpoint ${method} ${path}`, async () => { + expect((await call(path, { method })).status).toBe(404); }); -}); +} diff --git a/packages/server/src/routes.ts b/packages/server/src/routes.ts index 4876d42..c11333d 100644 --- a/packages/server/src/routes.ts +++ b/packages/server/src/routes.ts @@ -1,5 +1,4 @@ import { Hono } from "hono"; -import { clearCache } from "./cache.js"; import { createConfigFromExample, exampleConfig, @@ -9,12 +8,9 @@ import { readConfig, resolveConfigPath, } from "./config.js"; -import { clearRepoSettings, latestCheckRunsByName } from "./fetchers.js"; -import { clearClients, getClient, getInstance } from "./github-client.js"; import { getSync, listData, - recordMutation, reloadSyncConfiguration, scheduleResync, } from "./sync.js"; @@ -56,9 +52,6 @@ api.get("/config", async (c) => { api.post("/config/reload", async (c) => { invalidateConfigStatus(); - clearClients(); - clearRepoSettings(); - clearCache(); const payload = await configPayload(); // Invalid YAML is non-destructive; valid config still reconciles when a // rotated token fails auth. Lists cannot be read while config is invalid. @@ -142,115 +135,6 @@ api.post("/:instanceId/prs/:owner/:repo/:prNumber/auto-merge", async (c) => { } }); -// Merge a PR directly (squash) -api.post("/:instanceId/prs/:owner/:repo/:prNumber/merge", async (c) => { - const { instanceId, owner, repo, prNumber } = c.req.param(); - const num = Number(prNumber); - const fullRepo = `${owner}/${repo}`; - const identity = await getInstance(instanceId); - const client = await getClient(instanceId); - - try { - await client.pulls.merge({ - owner, - repo, - pull_number: num, - merge_method: "squash", - }); - } catch (err) { - // GitHub's message is often multi-line: a generic header - // ("Repository rule violations found") followed by the actionable - // detail ("A conversation must be resolved…"). Join non-empty lines - // so the toast carries the reason, not just the header. - const e = err as { response?: { data?: { message?: string } } }; - const raw = e.response?.data?.message; - const message = raw - ? raw - .split("\n") - .map((l) => l.trim()) - .filter((l) => l.length > 0) - .join(" — ") - : "Failed to merge PR"; - return c.json({ error: "merge_rejected", message }, 422); - } - - await recordMutation( - instanceId, - { - kind: "removed", - repo: fullRepo, - number: num, - }, - identity, - ); - scheduleResync(instanceId, ["prs", "reviews"]); - - return c.json({ ok: true }); -}); - -// Close a PR -api.post("/:instanceId/prs/:owner/:repo/:prNumber/close", async (c) => { - const { instanceId, owner, repo, prNumber } = c.req.param(); - const num = Number(prNumber); - const fullRepo = `${owner}/${repo}`; - const identity = await getInstance(instanceId); - const client = await getClient(instanceId); - - await client.pulls.update({ - owner, - repo, - pull_number: num, - state: "closed", - }); - - await recordMutation( - instanceId, - { - kind: "removed", - repo: fullRepo, - number: num, - }, - identity, - ); - scheduleResync(instanceId, ["prs", "reviews"]); - - return c.json({ ok: true }); -}); - -// Update PR title -api.patch("/:instanceId/prs/:owner/:repo/:prNumber", async (c) => { - const { instanceId, owner, repo, prNumber } = c.req.param(); - const { title } = await c.req.json().catch(() => ({ title: null })); - - if (!title) { - return c.json({ error: "Title is required" }, 400); - } - - const identity = await getInstance(instanceId); - const client = await getClient(instanceId); - - await client.pulls.update({ - owner, - repo, - pull_number: Number(prNumber), - title, - }); - - await recordMutation( - instanceId, - { - kind: "patch", - repo: `${owner}/${repo}`, - number: Number(prNumber), - patch: { title }, - }, - identity, - ); - scheduleResync(instanceId, ["prs", "reviews"]); - - return c.json({ ok: true, title }); -}); - // Toggle draft status on a PR api.post("/:instanceId/prs/:owner/:repo/:prNumber/toggle-draft", async (c) => { const { instanceId, owner, repo, prNumber } = c.req.param(); @@ -263,281 +147,27 @@ api.post("/:instanceId/prs/:owner/:repo/:prNumber/toggle-draft", async (c) => { return c.json({ ok: true, ...result }); }); -// Rerun CI for a PR -api.post("/:instanceId/prs/:owner/:repo/:prNumber/rerun-ci", async (c) => { +// File diffs and comments use the same engine APIs as the TUI. +api.get("/:instanceId/prs/:owner/:repo/:prNumber/diff", async (c) => { const { instanceId, owner, repo, prNumber } = c.req.param(); - const identity = await getInstance(instanceId); - const client = await getClient(instanceId); - const num = Number(prNumber); - - const { data: pr } = await client.pulls.get({ - owner, - repo, - pull_number: num, - }); - - const runsRes = await client.actions - .listWorkflowRunsForRepo({ - owner, - repo, - head_sha: pr.head.sha, - per_page: 100, - }) - .catch(() => null); - - // Keep only the latest attempt per workflow_id, and only for PR-triggered events - const latestPerWorkflow = new Map< - number, - { id: number; status: string; conclusion: string | null } - >(); - for (const run of runsRes?.data.workflow_runs ?? []) { - if (run.event !== "pull_request" && run.event !== "pull_request_target") { - continue; - } - const existing = latestPerWorkflow.get(run.workflow_id); - if (!existing) { - latestPerWorkflow.set(run.workflow_id, { - id: run.id, - status: run.status ?? "", - conclusion: run.conclusion, - }); - } - } - - const candidates = [...latestPerWorkflow.values()].filter( - (r) => r.status === "completed", - ); - - if (candidates.length === 0) { - return c.json({ error: "No rerunnable workflow runs found" }, 404); - } - - const results = await Promise.allSettled( - candidates.map((run) => { - const failed = - run.conclusion === "failure" || - run.conclusion === "timed_out" || - run.conclusion === "cancelled"; - return failed - ? client.actions.reRunWorkflowFailedJobs({ - owner, - repo, - run_id: run.id, - }) - : client.actions.reRunWorkflow({ owner, repo, run_id: run.id }); - }), - ); - - const triggered = results.filter((r) => r.status === "fulfilled").length; - - if (triggered === 0) { - return c.json({ error: "Failed to rerun any workflow" }, 502); - } - - await recordMutation( - instanceId, - { - kind: "patch", + return c.json( + await getSync().getPullRequestDiff({ + instanceId, repo: `${owner}/${repo}`, - number: num, - patch: { ciStatus: "pending" }, - }, - identity, + number: Number(prNumber), + }), ); - scheduleResync(instanceId, ["prs", "reviews"]); - - return c.json({ ok: true, triggered }); }); -// Post a comment on a PR -api.post("/:instanceId/prs/:owner/:repo/:prNumber/comment", async (c) => { - const { instanceId, owner, repo, prNumber } = c.req.param(); - const { body } = await c.req.json<{ body: string }>(); - const client = await getClient(instanceId); - const num = Number(prNumber); - - await client.issues.createComment({ owner, repo, issue_number: num, body }); - scheduleResync(instanceId, ["prs", "reviews"]); - - return c.json({ ok: true }); -}); - -// PR metadata for panel + copy menu -api.get("/:instanceId/prs/:owner/:repo/:prNumber/meta", async (c) => { - const { instanceId, owner, repo, prNumber } = c.req.param(); - const client = await getClient(instanceId); - const num = Number(prNumber); - - const [filesRes, commitsRes, checksRes, statusesRes] = await Promise.all([ - client.pulls.listFiles({ owner, repo, pull_number: num, per_page: 100 }), - client.pulls.listCommits({ owner, repo, pull_number: num, per_page: 100 }), - client.checks - .listForRef({ owner, repo, ref: `pull/${num}/head`, per_page: 100 }) - .catch(() => null), - client.repos - .getCombinedStatusForRef({ owner, repo, ref: `pull/${num}/head` }) - .catch(() => null), - ]); - - const checksByName = new Map< - string, - { name: string; status: string; conclusion: string | null } - >(); - - for (const s of statusesRes?.data.statuses ?? []) { - checksByName.set(s.context, { - name: s.context, - status: "completed", - conclusion: - s.state === "success" - ? "success" - : s.state === "failure" || s.state === "error" - ? "failure" - : null, - }); - } - - for (const cr of latestCheckRunsByName(checksRes?.data.check_runs ?? [])) { - checksByName.set(cr.name, { - name: cr.name, - status: cr.status, - conclusion: cr.conclusion, - }); - } - - return c.json({ - files: filesRes.data.map((f) => ({ - filename: f.filename, - additions: f.additions, - deletions: f.deletions, - status: f.status, - patch: f.patch ?? "", - })), - commits: commitsRes.data.map((c) => ({ - sha: c.sha.slice(0, 7), - message: c.commit.message.split("\n")[0], - })), - checks: [...checksByName.values()], - }); -}); - -// PR comments api.get("/:instanceId/prs/:owner/:repo/:prNumber/comments", async (c) => { const { instanceId, owner, repo, prNumber } = c.req.param(); - const client = await getClient(instanceId); - const num = Number(prNumber); - - const [issueComments, reviewComments] = await Promise.all([ - client.issues.listComments({ - owner, - repo, - issue_number: num, - per_page: 100, - }), - client.pulls.listReviewComments({ - owner, - repo, - pull_number: num, - per_page: 100, + return c.json( + await getSync().getPullRequestComments({ + instanceId, + repo: `${owner}/${repo}`, + number: Number(prNumber), }), - ]); - - const comments = [ - ...issueComments.data.map((c) => ({ - id: c.id, - author: c.user?.login ?? "unknown", - body: c.body ?? "", - createdAt: c.created_at, - path: null as string | null, - inReplyToId: null as number | null, - })), - ...reviewComments.data.map((c) => ({ - id: c.id, - author: c.user?.login ?? "unknown", - body: c.body, - createdAt: c.created_at, - path: c.path, - inReplyToId: c.in_reply_to_id ?? null, - })), - ].sort( - (a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime(), ); - - return c.json(comments); -}); - -// Search own closed PR history -api.get("/:instanceId/search/prs", async (c) => { - const { instanceId } = c.req.param(); - const client = await getClient(instanceId); - const { username } = await getInstance(instanceId); - const q = c.req.query("q") ?? ""; - - const { data } = await client.search.issuesAndPullRequests({ - q: `author:${username} type:pr state:closed ${q}`, - sort: "updated", - order: "desc", - per_page: 20, - }); - - const prs = data.items.map((item) => { - const [owner, repo] = item.repository_url.split("/").slice(-2); - return { - id: item.id, - number: item.number, - title: item.title, - url: item.html_url, - repo: `${owner}/${repo}`, - updatedAt: item.updated_at, - state: item.state, - }; - }); - - return c.json(prs); -}); - -// Colleague's open PRs -api.get("/:instanceId/users/:username/prs", async (c) => { - const { instanceId, username } = c.req.param(); - const client = await getClient(instanceId); - - const { data } = await client.search.issuesAndPullRequests({ - q: `author:${username} type:pr state:open`, - sort: "updated", - order: "desc", - per_page: 20, - }); - - const prs = data.items.map((item) => { - const [owner, repo] = (item.repository_url ?? "").split("/").slice(-2); - return { - id: item.id, - number: item.number, - title: item.title ?? "", - body: item.body ?? "", - url: item.html_url ?? "", - repo: `${owner}/${repo}`, - updatedAt: item.updated_at ?? new Date().toISOString(), - author: item.user?.login ?? "unknown", - authorAvatar: item.user?.avatar_url ?? "", - draft: (item as unknown as { draft?: boolean }).draft ?? false, - ciStatus: "unknown", - inMergeQueue: false, - autoMerge: false, - headBranch: - (item as unknown as { head?: { ref?: string } }).head?.ref ?? "", - baseBranch: - (item as unknown as { base?: { ref?: string } }).base?.ref ?? "", - reviews: { approved: [], changesRequested: [] }, - additions: 0, - deletions: 0, - commits: 0, - commentCount: 0, - labels: [], - }; - }); - - return c.json(prs); }); export { api }; diff --git a/packages/server/src/sync.integration.test.ts b/packages/server/src/sync.integration.test.ts index 3113946..b74d4a2 100644 --- a/packages/server/src/sync.integration.test.ts +++ b/packages/server/src/sync.integration.test.ts @@ -11,7 +11,6 @@ let domain: string; let id: string; let requests: string[]; let fail: boolean; -let holdUpdate: (() => Promise) | undefined; let api: (typeof import("./routes.js"))["api"]; let runtime: typeof import("./sync.js"); const node = { @@ -62,7 +61,6 @@ beforeEach(async () => { mkdirSync(join(root, "config/github-dashboard"), { recursive: true }); requests = []; fail = false; - holdUpdate = undefined; github = createServer(async (req, res) => { requests.push(req.url!); let body = ""; @@ -94,7 +92,6 @@ beforeEach(async () => { }), ); } else if (req.url?.endsWith("/pulls/5")) { - if (req.method === "PATCH") await holdUpdate?.(); res.end(JSON.stringify({ node_id: "PR_node", draft: true })); } else if (req.url?.includes("/notifications")) { res.setHeader("etag", '"v1"'); @@ -220,7 +217,7 @@ it("serves cached lists while an unrelated instance is blocked authenticating", } }); -it("shared actions and REST mutations cannot be undone by stale fresh/background syncs", async () => { +it("shared actions cannot be undone by stale fresh/background syncs", async () => { await api.request(`/${id}/prs`); await api.request(`/${id}/reviews`); const toggled = await api.request(`/${id}/prs/o/r/5/toggle-draft`, { @@ -233,15 +230,6 @@ it("shared actions and REST mutations cannot be undone by stale fresh/background (await (await api.request(`/${id}/${kind}?fresh=1`)).json())[0].draft, ).toBe(false); } - const closed = await api.request(`/${id}/prs/o/r/5/close`, { - method: "POST", - }); - expect(closed.status).toBe(200); - await runtime.waitForPendingResyncs(); - expect(await (await api.request(`/${id}/prs?fresh=1`)).json()).toEqual([]); - expect(await (await api.request(`/${id}/reviews?fresh=1`)).json()).toEqual( - [], - ); }); it("reload purges rotated identity even when the new token fails authentication", async () => { @@ -268,31 +256,3 @@ it("invalid YAML is non-destructive but blocks list reads; valid empty config re await api.request("/config/reload", { method: "POST" }); expect(runtime.getSync().listInstances()).toEqual([]); }); - -it("an in-flight REST mutation cannot patch a reloaded credential's snapshots", async () => { - await api.request(`/${id}/prs`); - let started!: () => void; - const began = new Promise((resolve) => { - started = resolve; - }); - let release!: () => void; - const gate = new Promise((resolve) => { - release = resolve; - }); - holdUpdate = async () => { - started(); - await gate; - }; - const closing = api.request(`/${id}/prs/o/r/5/close`, { method: "POST" }); - await began; - try { - config("new-token"); - await api.request("/config/reload", { method: "POST" }); - expect((await api.request(`/${id}/prs?fresh=1`)).status).toBe(200); - } finally { - release(); - } - expect((await closing).status).toBe(200); - await runtime.waitForPendingResyncs(); - expect(await (await api.request(`/${id}/prs`)).json()).toHaveLength(1); -}); diff --git a/packages/server/src/sync.ts b/packages/server/src/sync.ts index 4564d87..18ee72c 100644 --- a/packages/server/src/sync.ts +++ b/packages/server/src/sync.ts @@ -1,6 +1,6 @@ -import { createSync, type CacheMutation, type SyncKind } from "sync"; +import { createSync, type SyncKind } from "sync"; import { getCached } from "./cache.js"; -import { loadSyncInstances, type GitHubInstance } from "./config.js"; +import { loadSyncInstances } from "./config.js"; const SYNC_INTERVAL = 30_000; export type ResyncKey = SyncKind; @@ -52,14 +52,6 @@ export async function reloadSyncConfiguration(): Promise { await getSync().reconcileConfiguration(); } -export async function recordMutation( - instanceId: string, - mutation: CacheMutation, - expectedIdentity?: Pick, -) { - await getSync().recordMutation(instanceId, mutation, expectedIdentity); -} - export async function resyncInstance( instanceId: string, keys: ResyncKey[] = ALL_KEYS, diff --git a/packages/sync/src/index.ts b/packages/sync/src/index.ts index 5e07735..8852cd6 100644 --- a/packages/sync/src/index.ts +++ b/packages/sync/src/index.ts @@ -18,3 +18,5 @@ export { } from "./providers/github/reviewDiff.js"; export type { CacheMutation, PullRequestPatch } from "./cache/store.js"; + +export { authenticate } from "./config.js"; diff --git a/packages/web/src/App.tsx b/packages/web/src/App.tsx index 2fc8366..7411ee6 100644 --- a/packages/web/src/App.tsx +++ b/packages/web/src/App.tsx @@ -6,19 +6,10 @@ import { atomWithStorage } from "jotai/utils"; import { ArrowUpCircle, Loader2 } from "lucide-react"; import { useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; -import { AutoMergeNotAllowedError, api, type ConfigResponse } from "./api"; -import { - STALE_CACHE_RECOVERY_PROMPT, - decideMergeAction, - mergeActionLabel, -} from "./merge-decision"; +import { api, type ConfigResponse } from "./api"; import { dismissKey, dismissedReviewsAtom, isDismissed } from "./dismissed"; import { useChords } from "./use-chords"; -import { - type Action, - ActionMenu, - CommentDialog, -} from "./components/ActionMenu"; +import { type Action, ActionMenu } from "./components/ActionMenu"; import { type CopyTarget, CopyMenu } from "./components/CopyMenu"; import { PrPanel } from "./components/PrPanel"; import { Text } from "./components/Text"; @@ -49,7 +40,6 @@ import { useAllNotifications, useAllReviewRequests, useAuthoredPrs, - useColleaguePrs, useKeyboardNav, useNotifications, useReviewRequests, @@ -169,8 +159,6 @@ export function App() { }; }, []); - const urlParams = new URLSearchParams(window.location.search); - const filterParam = urlParams.get("filter"); const showWelcome = configRes?.status.kind === "error"; const instanceTabs = instances.map((i) => ({ id: i.id, label: i.label })); @@ -308,7 +296,6 @@ export function App() { ? instances[0] : instances.find((i) => i.id === activeTab)) ?? instances[0] } - authorFilter={filterParam ?? undefined} /> ) ) : null} @@ -342,64 +329,20 @@ interface FocusedItem { ciStatus?: string; } -interface CommentingPr { - instanceId: string; - repo: string; - number: number; -} - -function confirmAndMerge( +function toggleAutoMerge( queryClient: ReturnType, item: FocusedItem & { instanceId: string; repo: string; number: number }, - message: string, ): void { - if (!confirm(message)) return; + if (!confirm(item.autoMerge ? "Disable auto-merge?" : "Enable auto-merge?")) + return; mutations - .mergePr(queryClient, { - instanceId: item.instanceId, - repo: item.repo, - number: item.number, - }) + .toggleAutoMerge(queryClient, item, !!item.autoMerge) .catch(() => {}); } -function autoMergeOrFallback( - queryClient: ReturnType, - item: FocusedItem & { instanceId: string; repo: string; number: number }, -): void { - const decision = decideMergeAction(item); - const target = { - instanceId: item.instanceId, - repo: item.repo, - number: item.number, - }; - - if (decision.kind === "disable_auto_merge") { - mutations.toggleAutoMerge(queryClient, target, true).catch(() => {}); - return; - } - - if (decision.kind === "merge") { - confirmAndMerge(queryClient, item, decision.prompt); - return; - } - - // arm_auto_merge - if (!confirm(decision.prompt)) return; - mutations.toggleAutoMerge(queryClient, target, false).catch((err) => { - // Cache said allow_auto_merge=true but server disagrees — fall through - // and merge directly (user already expressed merge intent). - if (err instanceof AutoMergeNotAllowedError) { - confirmAndMerge(queryClient, item, STALE_CACHE_RECOVERY_PROMPT); - } - }); -} - function getActionsForItem( item: FocusedItem, queryClient: ReturnType, - setEditingPrNumber?: (prNumber: number) => void, - setCommentingPr?: (pr: CommentingPr) => void, ): Action[] { const actions: Action[] = [ { @@ -446,10 +389,10 @@ function getActionsForItem( // GitHub rejects enablePullRequestAutoMerge on draft PRs. if (item.autoMerge || !item.draft) { actions.push({ - label: mergeActionLabel(decideMergeAction(item)), + label: item.autoMerge ? "Disable auto-merge" : "Enable auto-merge", key: "m", onSelect: () => { - autoMergeOrFallback(queryClient, { + toggleAutoMerge(queryClient, { ...item, instanceId: item.instanceId!, repo: item.repo!, @@ -458,46 +401,6 @@ function getActionsForItem( }, }); } - - if (setEditingPrNumber) { - actions.push({ - label: "Edit title", - key: "t", - onSelect: () => { - setEditingPrNumber(item.number!); - }, - }); - } - - if (setCommentingPr) { - actions.push({ - label: "Post comment", - key: "c", - onSelect: () => { - setCommentingPr({ - instanceId: item.instanceId!, - repo: item.repo!, - number: item.number!, - }); - }, - }); - } - - if (item.ciStatus === "failure") { - actions.push({ - label: "Rerun failed jobs", - key: "i", - onSelect: () => { - mutations - .rerunCi(queryClient, { - instanceId: item.instanceId!, - repo: item.repo!, - number: item.number!, - }) - .catch(() => {}); - }, - }); - } } if ( @@ -521,20 +424,6 @@ function getActionsForItem( .catch(() => {}); }, }); - actions.push({ - label: "Close", - key: "x", - confirm: "Are you sure you want to close this PR?", - onSelect: () => { - mutations - .closePr(queryClient, { - instanceId: item.instanceId!, - repo: item.repo!, - number: item.number!, - }) - .catch(() => {}); - }, - }); } return actions; @@ -590,16 +479,8 @@ function MultiInstanceDashboard({ instances }: { instances: Instance[] }) { return ; } -function SingleInstanceDashboard({ - instance, - authorFilter, -}: { - instance: Instance; - authorFilter?: string; -}) { - const prs = authorFilter - ? useColleaguePrs(instance.id, authorFilter) - : useAuthoredPrs(instance.id); +function SingleInstanceDashboard({ instance }: { instance: Instance }) { + const prs = useAuthoredPrs(instance.id); const reviews = useReviewRequests(instance.id); const notifications = useNotifications(instance.id); const source: DashboardSource = { @@ -667,28 +548,14 @@ function Dashboard({ source }: { source: DashboardSource }) { const [copyMenu, setCopyMenu] = useState(null); const [panelItem, setPanelItem] = useState(null); const [togglingDraftId, setTogglingDraftId] = useState(null); - const [editingPrNumber, setEditingPrNumber] = useState(null); - const [commentingPr, setCommentingPr] = useState(null); - const [commentLoading, setCommentLoading] = useState(false); const actionMenuTimerRef = useRef(null); const overlayOpenRef = useRef(false); const updateOverlay = () => { - overlayOpenRef.current = !!( - actionMenu || - copyMenu || - panelItem || - editingPrNumber || - commentingPr - ); + overlayOpenRef.current = !!(actionMenu || copyMenu || panelItem); }; - useEffect(updateOverlay, [ - actionMenu, - copyMenu, - panelItem, - editingPrNumber, - commentingPr, - ]); + + useEffect(updateOverlay, [actionMenu, copyMenu, panelItem]); const openActionMenu = (item: FocusedItem) => { nav.setPaused(true); @@ -723,8 +590,8 @@ function Dashboard({ source }: { source: DashboardSource }) { setPanelItem(item); const { instanceId, repo, number } = item; queryClient.prefetchQuery({ - queryKey: ["pr-meta", instanceId, repo, number], - queryFn: () => api.prMeta(instanceId, repo, number), + queryKey: ["pr-diff", instanceId, repo, number], + queryFn: () => api.prDiff(instanceId, repo, number), }); queryClient.prefetchQuery({ queryKey: ["pr-comments", instanceId, repo, number], @@ -859,7 +726,7 @@ function Dashboard({ source }: { source: DashboardSource }) { toast.error("Mark the PR as ready before merging"); return; } - autoMergeOrFallback(queryClient, { + toggleAutoMerge(queryClient, { ...item, instanceId: item.instanceId, repo: item.repo, @@ -886,26 +753,6 @@ function Dashboard({ source }: { source: DashboardSource }) { }) .catch(() => {}); } - } else if ( - e.key === "c" && - (activeSection === "prs" || activeSection === "reviews") - ) { - if ( - item?.repo && - item.number && - item.instanceId && - !item.readOnly && - confirm("Close this PR?") - ) { - e.preventDefault(); - mutations - .closePr(queryClient, { - instanceId: item.instanceId, - repo: item.repo, - number: item.number, - }) - .catch(() => {}); - } } else if (e.key === "d" && activeSection === "prs") { const pr = prsRef.current[focusIndex]; if ( @@ -1030,21 +877,6 @@ function Dashboard({ source }: { source: DashboardSource }) { focusIndex={nav.focusIndex} isFocusedSection={nav.activeSection === "prs"} togglingDraftId={togglingDraftId ?? undefined} - editingPrNumber={editingPrNumber ?? undefined} - onSaveTitle={async (prNumber, title) => { - const pr = sortedPrs.find((p) => p.number === prNumber); - if (!pr) return; - const instanceId = pr.instanceId ?? instances[0]?.id; - if (!instanceId) return; - await mutations - .updatePrTitle( - queryClient, - { instanceId, repo: pr.repo, number: prNumber }, - title, - ) - .catch(() => {}); - setEditingPrNumber(null); - }} /> )} @@ -1113,18 +945,7 @@ function Dashboard({ source }: { source: DashboardSource }) { {actionMenu && ( { - setEditingPrNumber(num); - if (panelItem) closePanel(); - }, - (pr) => { - setCommentingPr(pr); - if (panelItem) closePanel(); - }, - )} + actions={getActionsForItem(actionMenu, queryClient)} onClose={closeActionMenu} /> )} @@ -1159,34 +980,6 @@ function Dashboard({ source }: { source: DashboardSource }) { /> )} - {commentingPr && ( - { - setCommentLoading(true); - try { - await api.postComment( - commentingPr.instanceId, - commentingPr.repo, - commentingPr.number, - body, - ); - toast("Comment posted"); - setCommentingPr(null); - if (!panelItem) nav.setPaused(false); - } catch { - toast("Failed to post comment"); - } finally { - setCommentLoading(false); - } - }} - onClose={() => { - setCommentingPr(null); - if (!panelItem) nav.setPaused(false); - }} - /> - )} - {chords.showPopup && chords.activeGroup && ( )} diff --git a/packages/web/src/api.ts b/packages/web/src/api.ts index 41d1e48..40ebbce 100644 --- a/packages/web/src/api.ts +++ b/packages/web/src/api.ts @@ -1,10 +1,4 @@ -import type { - Instance, - Notification, - PR, - ReviewRequest, - SearchPR, -} from "./types"; +import type { Instance, Notification, PR, ReviewRequest } from "./types"; async function fetchJson(url: string): Promise { const res = await fetch(url); @@ -61,14 +55,6 @@ export const api = { fetchJson( `/api/${instanceId}/notifications${fresh ? "?fresh=1" : ""}`, ), - searchPrs: (instanceId: string, q: string) => - fetchJson( - `/api/${instanceId}/search/prs?q=${encodeURIComponent(q)}`, - ), - userPrs: (instanceId: string, username: string) => - fetchJson( - `/api/${instanceId}/users/${encodeURIComponent(username)}/prs`, - ), dismissNotification: async (instanceId: string, threadId: string) => { const res = await fetch(`/api/${instanceId}/notifications/${threadId}`, { method: "DELETE", @@ -87,19 +73,16 @@ export const api = { if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); return res.json() as Promise<{ ok: boolean; draft: boolean }>; }, - prMeta: (instanceId: string, repo: string, prNumber: number) => { + prDiff: (instanceId: string, repo: string, prNumber: number) => { const [owner, name] = repo.split("/"); return fetchJson<{ + headSha: string; files: { - filename: string; - additions: number; - deletions: number; + path: string; status: string; - patch: string; + lines: { kind: "hunk" | "context" | "add" | "delete"; text: string }[]; }[]; - commits: { sha: string; message: string }[]; - checks: { name: string; status: string; conclusion: string | null }[]; - }>(`/api/${instanceId}/prs/${owner}/${name}/${prNumber}/meta`); + }>(`/api/${instanceId}/prs/${owner}/${name}/${prNumber}/diff`); }, toggleAutoMerge: async ( instanceId: string, @@ -122,49 +105,6 @@ export const api = { } return res.json() as Promise<{ ok: boolean; autoMerge: boolean }>; }, - mergePr: async (instanceId: string, repo: string, prNumber: number) => { - const [owner, name] = repo.split("/"); - const res = await fetch( - `/api/${instanceId}/prs/${owner}/${name}/${prNumber}/merge`, - { - method: "POST", - }, - ); - if (!res.ok) { - const body = await res.json().catch(() => null); - throw new Error(body?.message ?? `${res.status} ${res.statusText}`); - } - return res.json(); - }, - closePr: async (instanceId: string, repo: string, prNumber: number) => { - const [owner, name] = repo.split("/"); - const res = await fetch( - `/api/${instanceId}/prs/${owner}/${name}/${prNumber}/close`, - { - method: "POST", - }, - ); - if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); - return res.json(); - }, - updatePrTitle: async ( - instanceId: string, - repo: string, - prNumber: number, - title: string, - ) => { - const [owner, name] = repo.split("/"); - const res = await fetch( - `/api/${instanceId}/prs/${owner}/${name}/${prNumber}`, - { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ title }), - }, - ); - if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); - return res.json(); - }, prComments: (instanceId: string, repo: string, prNumber: number) => { const [owner, name] = repo.split("/"); return fetchJson< @@ -189,33 +129,4 @@ export const api = { if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); return res.json(); }, - rerunCi: async (instanceId: string, repo: string, prNumber: number) => { - const [owner, name] = repo.split("/"); - const res = await fetch( - `/api/${instanceId}/prs/${owner}/${name}/${prNumber}/rerun-ci`, - { - method: "POST", - }, - ); - if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); - return res.json(); - }, - postComment: async ( - instanceId: string, - repo: string, - prNumber: number, - body: string, - ) => { - const [owner, name] = repo.split("/"); - const res = await fetch( - `/api/${instanceId}/prs/${owner}/${name}/${prNumber}/comment`, - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ body }), - }, - ); - if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); - return res.json(); - }, }; diff --git a/packages/web/src/components/ActionMenu.tsx b/packages/web/src/components/ActionMenu.tsx index c26b8ee..bf79bdc 100644 --- a/packages/web/src/components/ActionMenu.tsx +++ b/packages/web/src/components/ActionMenu.tsx @@ -159,147 +159,3 @@ export function ActionMenu({ actions, onClose }: Props) { ); } - -interface CommentDialogProps { - onSubmit: (body: string) => void; - onClose: () => void; - loading?: boolean; -} - -export function CommentDialog({ - onSubmit, - onClose, - loading, -}: CommentDialogProps) { - const [value, setValue] = useState(""); - const textareaRef = useRef(null); - - useEffect(() => { - textareaRef.current?.focus(); - }, []); - - useEffect(() => { - const handler = (e: KeyboardEvent) => { - if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) { - e.preventDefault(); - if (value.trim()) onSubmit(value); - } else if (e.key === "Escape") { - e.preventDefault(); - onClose(); - } - }; - - window.addEventListener("keydown", handler); - return () => window.removeEventListener("keydown", handler); - }, [value, onSubmit, onClose]); - - return ( -
- e.stopPropagation()} - > -

Post comment

-