diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index 5ce6cb89..431137c3 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -18,7 +18,7 @@ import { createMcpServer } from './mcp.js'; import { MAX_WAIT_SECONDS, type Live } from './live.js'; import type { Routine } from './routine.js'; import { uiApiRouter } from './ui-api.js'; -import { SECTIONS_SCRIPT, escapeHtml, page } from './html.js'; +import { SECTIONS_SCRIPT, SESSIONS_SCRIPT, escapeHtml, page, sessionsStatus } from './html.js'; import { randomToken } from './crypto.js'; import { SESSION_COOKIE, @@ -151,6 +151,10 @@ export function createApp(deps: AppDeps): express.Express { res.type('text/javascript').set('Cache-Control', 'no-cache').send(SECTIONS_SCRIPT); }); + app.get('/sessions.js', (_req, res) => { + res.type('text/javascript').set('Cache-Control', 'no-cache').send(SESSIONS_SCRIPT); + }); + if (deps.uiDir) { // Build artifacts, the same for everyone; nothing about a review is in them. app.use('/assets', express.static(join(deps.uiDir, 'assets'), { index: false, immutable: true, maxAge: '365d' })); @@ -583,25 +587,59 @@ export function createApp(deps: AppDeps): express.Express { const section = (heading: string, content: string, count?: number): string => `

${heading}${count === undefined ? '' : ` (${count})`}

${content}
`; + const draftBadge = 'Draft'; + + const plural = (count: number, word: string): string => `${count} ${word}${count === 1 ? '' : 's'}`; + + /** Who one of the user's own pull requests is waiting on. */ + const ownStatus = (request: ReviewRequestRow): string => { + const parts = request.draft ? [draftBadge] : []; + const reviewers = request.requestedReviewers ?? []; + const openThreads = request.openThreads ?? 0; + if (openThreads === 0 && request.moreThreads) { + parts.push('Open threads unknown ·'); + } + if (openThreads > 0) { + parts.push(`Waiting on: you · ${openThreads}${request.moreThreads ? '+' : ''} open thread${openThreads === 1 && !request.moreThreads ? '' : 's'}`); + } else if (reviewers.length > 0) { + const count = Math.max(request.requestedReviewerCount ?? 0, reviewers.length); + const shown = reviewers.slice(0, 2).join(', ') + (count > 2 ? ', …' : ''); + const all = `${plural(count, 'review')} requested: ${reviewers.join(', ')}${count > reviewers.length ? ', …' : ''}`; + parts.push(`Waiting on: ${escapeHtml(shown)}`); + } else if (request.reviewDecision === 'APPROVED') { + parts.push('Approved'); + } else if (request.reviewDecision === 'CHANGES_REQUESTED') { + parts.push('Changes requested'); + } else { + parts.push('No reviewer requested'); + } + return parts.join(' '); + }; + const pullsSection = async ( heading: string, connectFor: string, empty: string, - list: () => Promise<{ rows: ReviewRequestRow[]; hidden: number } | null>, + list: () => Promise<{ rows: ReviewRequestRow[]; hidden: number; fetchedAt: number } | null>, reachable: boolean, - ): Promise => { - let found: { rows: ReviewRequestRow[]; hidden: number } | null; + own = false, + ): Promise<{ html: string; fetchedAt: number | null; failed?: true }> => { + let found: { rows: ReviewRequestRow[]; hidden: number; fetchedAt: number } | null; try { found = await list(); } catch (err) { - return section(heading, `

GitHub could not be asked: ${escapeHtml(err instanceof Error ? err.message : String(err))}

`); + return { + html: section(heading, `

GitHub could not be asked: ${escapeHtml(err instanceof Error ? err.message : String(err))}

`), + fetchedAt: null, + failed: true, + }; } if (!found) { - return section(heading, `

Connect GitHub on Settings to see ${connectFor}.

`); + return { html: section(heading, `

Connect GitHub on Settings to see ${connectFor}.

`), fetchedAt: null }; } const hidden = found.hidden > 0 ? ` (${found.hidden} hidden by your filters)` : ''; if (found.rows.length === 0) { - return section(heading, `

${empty}${hidden}

`, 0); + return { html: section(heading, `

${empty}${hidden}

`, 0), fetchedAt: found.fetchedAt }; } const rows = found.rows.map(request => { const slug = `${request.owner}/${request.repo}`; @@ -619,35 +657,38 @@ export function createApp(deps: AppDeps): express.Express { return ` ${escapeHtml(`${slug}#${request.number}`)} - ${escapeHtml(request.title)}${request.draft ? ' draft' : ''} - ${escapeHtml(request.author)} + ${escapeHtml(request.title)}${!own && request.draft ? ` ${draftBadge}` : ''} + ${own ? ownStatus(request) : escapeHtml(request.author)} ${escapeHtml(request.updatedAt.slice(0, 16).replace('T', ' '))} ${action} `; }).join(''); - return section(heading, ` - ${rows}
Pull requestTitleAuthorUpdated
- ${hidden ? `

${hidden}

` : ''}`, found.rows.length); + return { + html: section(heading, ` + ${rows}
Pull requestTitle${own ? 'Status' : 'Author'}Updated
+ ${hidden ? `

${hidden}

` : ''}`, found.rows.length), + fetchedAt: found.fetchedAt, + }; }; - app.get('/', async (req, res) => { - const user = await userFor(req); - if (!user) { - notSignedIn(res, req.originalUrl); - return; - } + /** + * Everything on the sessions page below its status line. `fetchedAt` is when GitHub was last asked for the + * oldest list shown, or now when no list came from GitHub. + */ + const sessionsBody = async (user: User, { fresh = false } = {}): Promise<{ html: string; fetchedAt: number; failed: boolean }> => { + const now = Date.now(); const status = await live.userStatus(user.id); const listening = status.listening; const reachable = listening || (await routine.configured(user.id)); const [requests, own, sessions] = await Promise.all([ pullsSection('Awaiting your review', 'pull requests waiting for your review', 'Nothing is waiting for your review.', async () => { - const found = await service.reviewRequests(user.id); - return found && { rows: found.requests, hidden: found.hidden }; + const found = await service.reviewRequests(user.id, { now, fresh }); + return found && { rows: found.requests, hidden: found.hidden, fetchedAt: found.fetchedAt }; }, reachable), pullsSection('Your open pull requests', 'your open pull requests', 'You have no open pull requests.', async () => { - const rows = await service.ownPulls(user.id); - return rows && { rows, hidden: 0 }; - }, reachable), + const found = await service.ownPulls(user.id, { now, fresh }); + return found && { rows: found.requests, hidden: 0, fetchedAt: found.fetchedAt }; + }, reachable, true), reviews.listSessions(user.id), ]); const rows = sessions.map(session => ` @@ -661,10 +702,38 @@ export function createApp(deps: AppDeps): express.Express { : listening ? '

Agent listening · Create and review hands a pull request to it

' : ''; - sendPage(res, 200, 'Sessions', agent + requests + own + (sessions.length > 0 + const html = agent + requests.html + own.html + (sessions.length > 0 ? section('Your review sessions', `${rows}
RepositoryChangeCreated
`, sessions.length) : section('No review sessions yet', `

Ask your agent to create_session through the diffity MCP connector. - See settings for how to add it.

`)) + '', user, [], true); + See settings for how to add it.

`)); + const fetched = [requests.fetchedAt, own.fetchedAt].filter((at): at is number => at !== null); + return { html, fetchedAt: fetched.length > 0 ? Math.min(...fetched) : now, failed: !!(requests.failed || own.failed) }; + }; + + app.get('/', async (req, res) => { + const user = await userFor(req); + if (!user) { + notSignedIn(res, req.originalUrl); + return; + } + const body = await sessionsBody(user); + sendPage(res, 200, 'Sessions', sessionsStatus(body.fetchedAt, Date.now()) + + `
${body.html}
` + + '', user, [], true); + }); + + app.get('/sessions/body', async (req, res) => { + res.set('Cache-Control', 'no-store'); + const user = await userFor(req); + if (!user) { + res.status(401).json({ error: 'Sign in first' }); + return; + } + const body = await sessionsBody(user, { fresh: req.query.fresh === '1' }); + // A failed GitHub search is not swapped in over the lists the page already shows. + res.status(body.failed ? 502 : 200) + .set('Content-Security-Policy', pageContentSecurityPolicy()) + .set('X-Fetched-At', new Date(body.fetchedAt).toISOString()).type('html').send(body.html); }); app.post('/sessions', sameOrigin, forms, async (req, res) => { diff --git a/packages/server/src/github.ts b/packages/server/src/github.ts index 55c4564e..703014c0 100644 --- a/packages/server/src/github.ts +++ b/packages/server/src/github.ts @@ -73,6 +73,15 @@ export interface ReviewRequest { draft: boolean; updatedAt: string; url: string; + /** Unresolved review threads; known for the user's own pull requests only. */ + openThreads?: number; + /** More threads than one page holds, so `openThreads` counts only the first page. */ + moreThreads?: boolean; + /** Users and bots by login, and teams by name, whose review is still requested. */ + requestedReviewers?: string[]; + requestedReviewerCount?: number; + /** GitHub's APPROVED, CHANGES_REQUESTED or REVIEW_REQUIRED; null when the repository requires no review. */ + reviewDecision?: string | null; } interface RawSearchIssue { @@ -98,6 +107,39 @@ const MAX_PAGES = 30; const REVIEW_REQUESTS_QUERY = 'is:pr is:open archived:false review-requested:@me'; const OWN_PULLS_QUERY = 'is:pr is:open archived:false author:@me'; +const THREADS_PAGE = 100; + +const OWN_PULLS_GRAPHQL = `query($q:String!){ + search(type:ISSUE,query:$q,first:50){ + nodes{ + ... on PullRequest{ + number + title + url + updatedAt + isDraft + repository{owner{login} name} + author{login} + reviewDecision + reviewRequests(first:20){totalCount nodes{requestedReviewer{... on User{login} ... on Bot{login} ... on Team{name}}}} + reviewThreads(first:${THREADS_PAGE}){totalCount nodes{isResolved isOutdated}} + } + } + } +}`; + +interface RawOwnPull { + number?: number; + title?: string; + url?: string; + updatedAt?: string; + isDraft?: boolean; + repository?: { owner?: { login?: string }; name?: string }; + author?: { login?: string } | null; + reviewDecision?: string | null; + reviewRequests?: { totalCount?: number; nodes?: { requestedReviewer?: { login?: string; name?: string } | null }[] }; + reviewThreads?: { totalCount?: number; nodes?: { isResolved?: boolean }[] }; +} const REVIEW_THREADS_QUERY = `query($owner:String!,$repo:String!,$number:Int!){ repository(owner:$owner,name:$repo){ @@ -295,9 +337,54 @@ export class GitHubApi { return this.searchPulls(token, REVIEW_REQUESTS_QUERY); } - /** The token's own open pull requests, most recently updated first. */ - ownPulls(token: string): Promise { - return this.searchPulls(token, OWN_PULLS_QUERY); + /** + * The token's own open pull requests, most recently updated first, with who each is waiting on. + * Review threads and requested reviewers live on GraphQL only. + */ + async ownPulls(token: string): Promise { + let json: { data?: { search?: { nodes?: (RawOwnPull | null)[] } }; errors?: { message?: string }[] }; + try { + json = await this.json(token, '/graphql', { + method: 'POST', + body: { query: OWN_PULLS_GRAPHQL, variables: { q: `${OWN_PULLS_QUERY} sort:updated-desc` } }, + }); + } catch (err) { + if (err instanceof GitHubApiError && err.status === 422) { + console.warn(`Searching pull requests failed: ${err.message}`); + return []; + } + throw err; + } + // GraphQL answers a refused or partly failed query with 200 and errors, e.g. when rate limited. + if (!json.data?.search || (json.errors?.length ?? 0) > 0) { + throw new Error(`Searching pull requests failed: ${json.errors?.map(e => e.message).join('; ') || 'no data'}`); + } + return (json.data.search.nodes ?? []).flatMap(node => { + const owner = node?.repository?.owner?.login; + const repo = node?.repository?.name; + if (!node || !owner || !repo || node.number === undefined) { + return []; + } + const threads = node.reviewThreads?.nodes ?? []; + const reviewers = (node.reviewRequests?.nodes ?? []) + .map(request => request.requestedReviewer?.login ?? request.requestedReviewer?.name) + .filter((name): name is string => !!name); + return [{ + owner, + repo, + number: node.number, + title: node.title ?? '', + author: node.author?.login ?? '', + draft: !!node.isDraft, + updatedAt: node.updatedAt ?? '', + url: node.url ?? `https://github.com/${owner}/${repo}/pull/${node.number}`, + openThreads: threads.filter(thread => !thread.isResolved).length, + moreThreads: (node.reviewThreads?.totalCount ?? 0) > threads.length, + requestedReviewers: reviewers, + requestedReviewerCount: node.reviewRequests?.totalCount ?? reviewers.length, + reviewDecision: node.reviewDecision ?? null, + }]; + }); } private async searchPulls(token: string, q: string): Promise { @@ -306,8 +393,8 @@ export class GitHubApi { try { result = await this.json(token, `/search/issues?${query.toString()}`); } catch (err) { - // A search GitHub refuses (rate limit, an App without access) leaves the list empty, not the page broken. - if (err instanceof GitHubApiError && (err.status === 422 || err.status === 403)) { + // A query GitHub cannot validate, e.g. for an App without access, has no results. A rate limit's 403 is a failure. + if (err instanceof GitHubApiError && err.status === 422) { console.warn(`Searching pull requests failed: ${err.message}`); return []; } diff --git a/packages/server/src/html.ts b/packages/server/src/html.ts index 82e5e1ae..6179d079 100644 --- a/packages/server/src/html.ts +++ b/packages/server/src/html.ts @@ -27,18 +27,111 @@ const STYLE = ` summary > h2 { display: inline-block; } code, pre { font-family: ui-monospace, SFMono-Regular, Menlo, monospace; font-size: 12px; } pre { padding: 8px; border: 1px solid var(--line); overflow-x: auto; } + .badge { display: inline-block; padding: 0 7px; border: 1px solid var(--line); border-radius: 2em; font-size: 12px; font-weight: 500; line-height: 18px; color: var(--muted); white-space: nowrap; } + .status { display: flex; gap: 8px; align-items: center; } + .reload { display: inline-flex; padding: 4px; color: var(--muted); background: none; border: 1px solid var(--line); border-radius: 6px; } + .reload svg { width: 16px; height: 16px; } + .reload[aria-busy=true] svg { animation: spin 1s linear infinite; } + @keyframes spin { to { transform: rotate(360deg); } } + @media (prefers-reduced-motion: reduce) { .reload[aria-busy=true] svg { animation: none; } } `; -/** Keeps each collapsible section of a page open or closed as the user last left it. */ -export const SECTIONS_SCRIPT = `for (const details of document.querySelectorAll('details')) { - const key = 'diffity.section:' + details.dataset.section; - try { - if (localStorage.getItem(key) === 'closed') details.open = false; - } catch {} - details.addEventListener('toggle', () => { - try { localStorage.setItem(key, details.open ? 'open' : 'closed'); } catch {} - }); +/** + * Keeps each collapsible section of a page open or closed as the user last left it. `applySections` + * is global so a script that swaps in new sections can restore their state before showing them. + */ +export const SECTIONS_SCRIPT = `function applySections(root) { + for (const details of root.querySelectorAll('details[data-section]')) { + try { + if (localStorage.getItem('diffity.section:' + details.dataset.section) === 'closed') details.open = false; + } catch {} + } +} +applySections(document); +// toggle does not bubble, so one capturing listener covers every section, including ones swapped in later. +document.addEventListener('toggle', event => { + const details = event.target; + if (!(details instanceof HTMLDetailsElement) || !details.dataset.section) return; + try { localStorage.setItem('diffity.section:' + details.dataset.section, details.open ? 'open' : 'closed'); } catch {} +}, true); +`; + +/** Also runs in the browser, inside SESSIONS_SCRIPT, so it stays free of anything outside itself. */ +export function formatAge(ms: number): string { + const seconds = Math.max(0, Math.floor(ms / 1000)); + if (seconds < 10) return 'just now'; + if (seconds < 60) return `${seconds} s ago`; + if (seconds < 3600) return `${Math.floor(seconds / 60)} min ago`; + return `${Math.floor(seconds / 3600)} h ago`; +} + +const RELOAD_ICON = ``; + +/** The line above the sessions page's sections: how old its GitHub data is, and a button to ask again. */ +export function sessionsStatus(fetchedAt: number, now: number): string { + return `

Updated ${formatAge(now - fetchedAt)}` + + '' + + `

`; } + +/** + * Keeps the sessions page current without reloading it: swaps in `/sessions/body` every minute while the tab + * is visible, and from GitHub rather than the server's cache when the reload button asks. Needs SECTIONS_SCRIPT. + */ +export const SESSIONS_SCRIPT = `(() => { + const AUTO_MS = 60000; + const container = document.getElementById('sessions'); + const age = document.getElementById('sessions-age'); + const error = document.getElementById('sessions-error'); + const reload = document.getElementById('sessions-reload'); + if (!container || !age || !error || !reload) return; + let loadedAt = Date.now(); + let inFlight = false; + let failed = false; + + const formatAge = ${formatAge.toString()}; + const tick = () => { + const fetchedAt = Date.parse(container.dataset.fetchedAt || ''); + if (!Number.isNaN(fetchedAt)) age.textContent = 'Updated ' + formatAge(Date.now() - fetchedAt); + }; + + const refresh = async fresh => { + if (inFlight) return; + // Swapping the content would take focus from the control a keyboard user is on; the next tick tries again. + if (!fresh && container.contains(document.activeElement)) return; + inFlight = true; + reload.setAttribute('aria-busy', 'true'); + try { + const res = await fetch('/sessions/body' + (fresh ? '?fresh=1' : ''), { credentials: 'same-origin', cache: 'no-store' }); + if (!res.ok) throw new Error('HTTP ' + res.status); + const template = document.createElement('template'); + template.innerHTML = await res.text(); + applySections(template.content); + container.replaceChildren(template.content); + const fetchedAt = res.headers.get('X-Fetched-At'); + if (fetchedAt) container.dataset.fetchedAt = fetchedAt; + loadedAt = Date.now(); + failed = false; + } catch { + failed = true; + } finally { + inFlight = false; + reload.removeAttribute('aria-busy'); + error.hidden = !failed; + tick(); + } + }; + + const due = () => failed || Date.now() - loadedAt >= AUTO_MS; + reload.addEventListener('click', () => refresh(true)); + setInterval(() => { + tick(); + if (document.visibilityState === 'visible' && due()) refresh(false); + }, 15000); + document.addEventListener('visibilitychange', () => { + if (document.visibilityState === 'visible' && due()) refresh(false); + }); +})(); `; export function page(input: { title: string; body: string; user?: { email: string } | null }): string { diff --git a/packages/server/src/service.ts b/packages/server/src/service.ts index b432435f..2af2f1d1 100644 --- a/packages/server/src/service.ts +++ b/packages/server/src/service.ts @@ -63,11 +63,27 @@ export interface ReviewRequests { requests: ReviewRequestRow[]; /** How many the user's filters left out. */ hidden: number; + /** When GitHub was asked, in epoch milliseconds; the answer is cached for a minute. */ + fetchedAt: number; +} + +export interface OwnPulls { + requests: ReviewRequestRow[]; + /** When GitHub was asked, in epoch milliseconds; the answer is cached for a minute. */ + fetchedAt: number; +} + +export interface PullsQuery { + now?: number; + /** Asks GitHub again instead of answering from the cache. */ + fresh?: boolean; } const MAX_PATCH_BYTES = 10 * 1024 * 1024; const CACHE_ENTRIES = 256; const REVIEW_REQUESTS_TTL_MS = 60_000; +/** GitHub's search allows 30 requests a minute, and a refused one reads as an empty list. */ +const FRESH_MIN_AGE_MS = 10_000; export function filterReviewRequests(requests: ReviewRequest[], settings: UserSettings): ReviewRequest[] { const skip = settings.skipTitles.map(pattern => pattern.trim().toLowerCase()).filter(Boolean); @@ -167,27 +183,31 @@ export class ReviewService { } /** Null when the user has no GitHub access to ask with. */ - async reviewRequests(userId: string, now = Date.now()): Promise { - const all = await this.searchPulls(userId, 'review', token => this.github.reviewRequests(token), now); - if (!all) { + async reviewRequests(userId: string, query: PullsQuery = {}): Promise { + const found = await this.searchPulls(userId, 'review', token => this.github.reviewRequests(token), query); + if (!found) { return null; } + const all = await found.requests; const shown = filterReviewRequests(all, await this.users.settings(userId)); - return { requests: await this.withSessions(userId, shown), hidden: all.length - shown.length }; + return { requests: await this.withSessions(userId, shown), hidden: all.length - shown.length, fetchedAt: found.at }; } /** Null when the user has no GitHub access to ask with. */ - async ownPulls(userId: string, now = Date.now()): Promise { - const all = await this.searchPulls(userId, 'own', token => this.github.ownPulls(token), now); - return all && this.withSessions(userId, all); + async ownPulls(userId: string, query: PullsQuery = {}): Promise { + const found = await this.searchPulls(userId, 'own', token => this.github.ownPulls(token), query); + if (!found) { + return null; + } + return { requests: await this.withSessions(userId, await found.requests), fetchedAt: found.at }; } private async searchPulls( userId: string, kind: 'review' | 'own', search: (token: string) => Promise, - now: number, - ): Promise { + { now = Date.now(), fresh = false }: PullsQuery, + ): Promise<{ at: number; requests: Promise } | null> { const token = await this.access.tokenFor(userId); if (!token) { return null; @@ -199,7 +219,7 @@ export class ReviewService { } const key = `${kind}:${userId}`; let cached = this.reviewRequestCache.get(key); - if (!cached || cached.token !== token) { + if (!cached || cached.token !== token || (fresh && now - cached.at >= FRESH_MIN_AGE_MS)) { const entry = { token, at: now, requests: search(token) }; this.reviewRequestCache.set(key, entry); entry.requests.catch(() => { @@ -209,7 +229,7 @@ export class ReviewService { }); cached = entry; } - return cached.requests; + return cached; } private async withSessions(userId: string, requests: ReviewRequest[]): Promise { diff --git a/packages/server/src/web.ts b/packages/server/src/web.ts index 0b1497ee..b5aa6bb4 100644 --- a/packages/server/src/web.ts +++ b/packages/server/src/web.ts @@ -26,7 +26,7 @@ export function pageContentSecurityPolicy(extraFormTargets: string[] = [], selfS `form-action 'self'${extraFormTargets.map(target => ` ${target}`).join('')}`, "img-src 'self'", "style-src 'unsafe-inline'", - ...selfScripts ? ["script-src 'self'"] : [], + ...selfScripts ? ["script-src 'self'", "connect-src 'self'"] : [], ].join('; '); } diff --git a/packages/server/tests/helpers.ts b/packages/server/tests/helpers.ts index e88e1669..e5884a52 100644 --- a/packages/server/tests/helpers.ts +++ b/packages/server/tests/helpers.ts @@ -157,6 +157,14 @@ export interface FakeReviewRequest { author: string; draft?: boolean; updatedAt: string; + /** Own pull requests only: each review thread's isResolved. */ + threads?: boolean[]; + /** Own pull requests only: the threads' totalCount, when more than `threads` holds. */ + threadCount?: number; + /** Own pull requests only: requested users by login, or teams as `{ team }`. */ + reviewers?: (string | { team: string })[]; + reviewerCount?: number; + reviewDecision?: string | null; } export interface FakeGitHub { @@ -176,11 +184,13 @@ export interface FakeGitHub { failReviewWith: number | null; /** What `GET /search/issues` finds waiting for each token's review. */ reviewRequests: Record; - /** What `GET /search/issues` finds authored by each token's own account. */ + /** What the GraphQL search finds authored by each token's own account. */ ownPulls: Record; - /** The `q` of every search. */ + /** The `q` of every search, REST or GraphQL. */ searches: string[]; - /** Makes `GET /search/issues` answer this status instead. */ + /** Makes the GraphQL search answer 200 with these errors and no data. */ + graphqlSearchErrors: string[] | null; + /** Makes `GET /search/issues` and the GraphQL search answer this status instead. */ failSearchWith: number | null; /** The GitHub App's installation id per lowercased "owner/repo"; elsewhere it is not installed. */ installations: Record; @@ -212,6 +222,7 @@ export async function startFakeGitHub(repos: FakeRepo[]): Promise { ownPulls: {}, searches: [], failSearchWith: null, + graphqlSearchErrors: null, installations: {}, installationTokenRequests: [], installationTokenTtlMs: 60 * 60 * 1000, @@ -265,6 +276,40 @@ export async function startFakeGitHub(repos: FakeRepo[]): Promise { send(200, { total_count: items.length, incomplete_results: false, items }); return; } + if (url.pathname === '/graphql' && req.method === 'POST' && raw.includes('search(')) { + const { variables } = JSON.parse(raw) as { variables: { q: string } }; + fake.searches.push(variables.q); + if (fake.failSearchWith) { + send(fake.failSearchWith, { message: 'Forbidden' }); + return; + } + if (fake.graphqlSearchErrors) { + send(200, { errors: fake.graphqlSearchErrors.map(message => ({ message })) }); + return; + } + const nodes = (token ? fake.ownPulls[token] ?? [] : []).map(pr => ({ + number: pr.number, + title: pr.title, + url: `https://github.com/${pr.owner}/${pr.repo}/pull/${pr.number}`, + updatedAt: pr.updatedAt, + isDraft: pr.draft ?? false, + repository: { owner: { login: pr.owner }, name: pr.repo }, + author: { login: pr.author }, + reviewDecision: pr.reviewDecision ?? null, + reviewRequests: { + totalCount: pr.reviewerCount ?? pr.reviewers?.length ?? 0, + nodes: (pr.reviewers ?? []).map(reviewer => ({ + requestedReviewer: typeof reviewer === 'string' ? { login: reviewer } : { name: reviewer.team }, + })), + }, + reviewThreads: { + totalCount: pr.threadCount ?? pr.threads?.length ?? 0, + nodes: (pr.threads ?? []).map(isResolved => ({ isResolved, isOutdated: false })), + }, + })); + send(200, { data: { search: { nodes } } }); + return; + } if (url.pathname === '/graphql' && req.method === 'POST') { const { variables } = JSON.parse(raw) as { variables: { number: number } }; const roots = fake.comments.filter(c => c.pull === variables.number && !c.in_reply_to_id); diff --git a/packages/server/tests/http.test.ts b/packages/server/tests/http.test.ts index b6fff83f..b0604500 100644 --- a/packages/server/tests/http.test.ts +++ b/packages/server/tests/http.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import type { CommentThread, RepoInfoResponse, Tour } from '@diffity/api'; import { writeFileSync } from 'node:fs'; import { join } from 'node:path'; @@ -290,10 +290,15 @@ describe('pull requests awaiting review', () => { expect(review).not.toContain('acme/widgets#1'); expect(review).not.toContain('Work in progress'); expect(own).toContain('acme/widgets#1'); - expect(own).toContain('Work in progress draft'); + expect(own).toContain('Status'); + expect(own).not.toContain('Author'); + expect(own).not.toContain('alice'); + expect(own).toContain('Draft No reviewer requested'); + expect(review).toContain('Author'); + expect(review).toContain('carol'); expect(own).toContain(`open · ${fixture.head1.slice(0, 7)}`); expect(own).not.toContain('acme/widgets#2'); - expect(github.searches).toContain('is:pr is:open archived:false author:@me'); + expect(github.searches).toContain('is:pr is:open archived:false author:@me sort:updated-desc'); github.ownPulls['alice-token-own-none'] = []; await server.users.setGitHubToken(aliceId, 'alice-token-own-none'); @@ -306,6 +311,113 @@ describe('pull requests awaiting review', () => { expect((await api('/', { cookie: aliceCookie })).body).toContain('Connect GitHub on Settings to see your open pull requests.'); }); + it('answers a refresh with an error when GitHub fails, so the page keeps what it shows', async () => { + github.reviewRequests['alice-token-failing'] = []; + github.ownPulls['alice-token-failing'] = []; + await server.users.setGitHubToken(aliceId, 'alice-token-failing'); + github.failSearchWith = 500; + try { + const body = await api('/sessions/body', { cookie: aliceCookie }); + expect(body.status).toBe(502); + expect(body.body).toContain('GitHub could not be asked'); + expect((await api('/', { cookie: aliceCookie })).status).toBe(200); + } finally { + github.failSearchWith = null; + await server.users.setGitHubToken(aliceId, null); + } + }); + + it('says who each own pull request is waiting on', async () => { + github.ownPulls['alice-token-status'] = [ + { owner: 'acme', repo: 'widgets', number: 11, title: 'Threads', author: 'alice', draft: true, updatedAt: '2026-10-04T09:00:00Z', + threads: [false, true, false], reviewers: ['kirill'] }, + { owner: 'acme', repo: 'widgets', number: 12, title: 'One thread', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', threads: [false] }, + { owner: 'acme', repo: 'widgets', number: 13, title: 'Many threads', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', + threads: Array.from({ length: 100 }, () => false), threadCount: 150 }, + { owner: 'acme', repo: 'widgets', number: 14, title: 'Reviewers', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', + threads: [true], reviewers: ['kirill', '', { team: 'platform' }], reviewDecision: 'REVIEW_REQUIRED' }, + { owner: 'acme', repo: 'widgets', number: 15, title: 'Two reviewers', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', + reviewers: ['kirill', 'juraj'] }, + { owner: 'acme', repo: 'widgets', number: 16, title: 'Approved', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', reviewDecision: 'APPROVED' }, + { owner: 'acme', repo: 'widgets', number: 17, title: 'Changes', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', reviewDecision: 'CHANGES_REQUESTED' }, + { owner: 'acme', repo: 'widgets', number: 18, title: 'Nobody', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', reviewDecision: null }, + { owner: 'acme', repo: 'widgets', number: 19, title: 'Unread threads', author: 'alice', updatedAt: '2026-10-04T09:00:00Z', + threads: Array.from({ length: 100 }, () => true), threadCount: 150, reviewDecision: 'APPROVED' }, + ]; + await server.users.setGitHubToken(aliceId, 'alice-token-status'); + try { + const page = (await api('/', { cookie: aliceCookie })).body as string; + const own = page.slice(page.indexOf('Your open pull requests'), page.indexOf('Your review sessions')); + const status = (number: number) => { + const row = own.slice(own.indexOf(`acme/widgets#${number}<`)); + const cell = row.slice(0, row.indexOf('')).split('')[2]; + return cell.slice(0, cell.indexOf('')); + }; + expect(status(11)).toBe('Draft Waiting on: you · 2 open threads'); + expect(status(12)).toBe('Waiting on: you · 1 open thread'); + expect(status(13)).toBe('Waiting on: you · 100+ open threads'); + expect(status(14)).toBe('Waiting on: kirill, <juraj>, …'); + expect(status(15)).toBe('Waiting on: kirill, juraj'); + expect(status(16)).toBe('Approved'); + expect(status(17)).toBe('Changes requested'); + expect(status(18)).toBe('No reviewer requested'); + expect(status(19)).toBe('Open threads unknown · Approved'); + expect(own).not.toContain(''); + expect(own).toContain('Threads'); + } finally { + await server.users.setGitHubToken(aliceId, null); + } + }); + + it('serves the sections alone for the page to swap in, with when GitHub was asked', async () => { + expect(await api('/sessions/body')).toMatchObject({ status: 401 }); + github.reviewRequests['alice-token-body'] = [ + { owner: 'acme', repo: 'widgets', number: 2, title: 'Rename a file', author: 'carol', updatedAt: '2026-10-02T09:00:00Z' }, + ]; + github.ownPulls['alice-token-body'] = []; + await server.users.setGitHubToken(aliceId, 'alice-token-body'); + const fetchedAt = (page: string) => /
/.exec(page)![1]; + try { + const searches = github.searches.length; + const first = (await api('/', { cookie: aliceCookie })).body as string; + expect(first).toMatch(/

Updated just now<\/span>/); + expect(first).toContain('id="sessions-reload"'); + const at = fetchedAt(first); + expect(Date.parse(at)).not.toBeNaN(); + expect(github.searches.length).toBe(searches + 2); + + await new Promise(resolve => setTimeout(resolve, 5)); + const body = await api('/sessions/body', { cookie: aliceCookie }); + expect(body.status).toBe(200); + expect(body.headers.get('cache-control')).toBe('no-store'); + expect(body.headers.get('content-type')).toContain('text/html'); + expect(body.headers.get('x-fetched-at')).toBe(at); + expect(body.body).toContain('

Awaiting your review (1)

'); + expect(body.body).toContain('

Your open pull requests (0)

'); + expect(body.body).not.toContain(''); + expect(body.body).not.toContain('id="sessions"'); + expect(fetchedAt((await api('/', { cookie: aliceCookie })).body)).toBe(at); + expect(github.searches.length).toBe(searches + 2); + + const soon = await api('/sessions/body?fresh=1', { cookie: aliceCookie }); + expect(soon.headers.get('x-fetched-at')).toBe(at); + expect(github.searches.length).toBe(searches + 2); + + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(Date.parse(at) + 11_000); + const fresh = await api('/sessions/body?fresh=1', { cookie: aliceCookie }); + expect(github.searches.length).toBe(searches + 4); + const later = fresh.headers.get('x-fetched-at')!; + expect(Date.parse(later)).toBeGreaterThan(Date.parse(at)); + expect(fetchedAt((await api('/', { cookie: aliceCookie })).body)).toBe(later); + expect(github.searches.length).toBe(searches + 4); + } finally { + vi.useRealTimers(); + await server.users.setGitHubToken(aliceId, null); + } + }); + it('opens sessions in a new tab, folds each section, and allows only its own script', async () => { github.reviewRequests['alice-token-tabs'] = [ { owner: 'acme', repo: 'widgets', number: 2, title: 'Rename a file', author: 'carol', updatedAt: '2026-10-02T09:00:00Z' }, @@ -323,6 +435,12 @@ describe('pull requests awaiting review', () => { const csp = res.headers.get('content-security-policy')!; expect(csp).toContain("script-src 'self'"); expect(csp).not.toMatch(/script-src[^;]*unsafe/); + expect(csp).toContain("connect-src 'self'"); + expect(page).toContain(''); + const sessionsScript = await api('/sessions.js'); + expect(sessionsScript.headers.get('content-type')).toContain('javascript'); + expect(sessionsScript.body).toContain("'/sessions/body' + (fresh ? '?fresh=1' : '')"); + expect(sessionsScript.body).toContain('applySections(template.content)'); const script = await api('/sections.js'); expect(script.headers.get('content-type')).toContain('javascript'); expect(script.body).toContain('localStorage'); diff --git a/packages/server/tests/service.test.ts b/packages/server/tests/service.test.ts index 00ea86d1..b05c9431 100644 --- a/packages/server/tests/service.test.ts +++ b/packages/server/tests/service.test.ts @@ -279,6 +279,8 @@ describe('review requests', () => { github.failSearchWith = 422; try { expect(await api.reviewRequests('search-token')).toEqual([]); + github.failSearchWith = 403; + await expect(api.reviewRequests('search-token')).rejects.toThrow('GitHub answered 403'); github.failSearchWith = 500; await expect(api.reviewRequests('search-token')).rejects.toThrow('GitHub answered 500'); } finally { @@ -286,6 +288,36 @@ describe('review requests', () => { } }); + it('asks GitHub’s GraphQL search for the token’s own pull requests, with threads and requested reviewers', async () => { + github.ownPulls['own-search-token'] = [ + { owner: 'acme', repo: 'widgets', number: 8, title: 'Eight', author: 'erin', draft: true, updatedAt: '2026-10-02T08:30:00Z', + threads: [false, true], threadCount: 120, reviewers: ['kirill', { team: 'platform' }], reviewerCount: 2, reviewDecision: 'REVIEW_REQUIRED' }, + ]; + const api = new GitHubApi(github.url); + expect(await api.ownPulls('own-search-token')).toEqual([{ + owner: 'acme', repo: 'widgets', number: 8, title: 'Eight', author: 'erin', draft: true, + updatedAt: '2026-10-02T08:30:00Z', url: 'https://github.com/acme/widgets/pull/8', + openThreads: 1, moreThreads: true, requestedReviewers: ['kirill', 'platform'], requestedReviewerCount: 2, + reviewDecision: 'REVIEW_REQUIRED', + }]); + expect(github.searches.at(-1)).toBe('is:pr is:open archived:false author:@me sort:updated-desc'); + + try { + github.failSearchWith = 403; + await expect(api.ownPulls('own-search-token')).rejects.toThrow('GitHub answered 403'); + github.failSearchWith = 422; + expect(await api.ownPulls('own-search-token')).toEqual([]); + github.failSearchWith = 500; + await expect(api.ownPulls('own-search-token')).rejects.toThrow('GitHub answered 500'); + github.failSearchWith = null; + github.graphqlSearchErrors = ['API rate limit exceeded']; + await expect(api.ownPulls('own-search-token')).rejects.toThrow('Searching pull requests failed: API rate limit exceeded'); + } finally { + github.failSearchWith = null; + github.graphqlSearchErrors = null; + } + }); + it('lists them for the user with their newest session on each, filtered, and asks GitHub once a minute', async () => { github.reviewRequests['bob-token'] = [ { owner: 'acme', repo: 'widgets', number: 1, title: 'Change line ten', author: 'octocat', updatedAt: '2026-10-03T09:00:00Z' }, @@ -297,19 +329,47 @@ describe('review requests', () => { const now = Date.now(); const searches = github.searches.length; - const found = await service.reviewRequests(bob, now); + const found = await service.reviewRequests(bob, { now }); expect(found?.hidden).toBe(1); expect(found?.requests.map(r => [r.number, r.session?.id ?? null])).toEqual([[1, newest.id], [2, null]]); expect(found?.requests[0].session).toEqual({ id: newest.id, headSha: newest.headSha, createdAt: newest.createdAt }); await users.setSettings(bob, { skipTitles: ['bump'], includeDrafts: true }); - expect((await service.reviewRequests(bob, now + 30_000))?.requests.map(r => r.number)).toEqual([1, 3]); + expect((await service.reviewRequests(bob, { now: now + 30_000 }))?.requests.map(r => r.number)).toEqual([1, 3]); expect(github.searches.length).toBe(searches + 1); - await service.reviewRequests(bob, now + 61_000); + await service.reviewRequests(bob, { now: now + 61_000 }); expect(github.searches.length).toBe(searches + 2); await users.setSettings(bob, { skipTitles: [], includeDrafts: false }); }); + it('says when GitHub was asked, and asks again when fresh, replacing the cached answer', async () => { + github.reviewRequests['bob-token'] = [ + { owner: 'acme', repo: 'widgets', number: 1, title: 'Change line ten', author: 'octocat', updatedAt: '2026-10-03T09:00:00Z' }, + ]; + github.ownPulls['bob-token'] = [ + { owner: 'acme', repo: 'widgets', number: 4, title: 'Mine', author: 'bob', updatedAt: '2026-10-03T09:00:00Z' }, + ]; + const now = Date.now() + 200_000; + const searches = github.searches.length; + + expect((await service.reviewRequests(bob, { now }))?.fetchedAt).toBe(now); + expect((await service.reviewRequests(bob, { now: now + 10_000 }))?.fetchedAt).toBe(now); + expect(github.searches.length).toBe(searches + 1); + + expect((await service.reviewRequests(bob, { now: now + 5_000, fresh: true }))?.fetchedAt).toBe(now); + expect(github.searches.length).toBe(searches + 1); + expect((await service.reviewRequests(bob, { now: now + 20_000, fresh: true }))?.fetchedAt).toBe(now + 20_000); + expect(github.searches.length).toBe(searches + 2); + expect((await service.reviewRequests(bob, { now: now + 30_000 }))?.fetchedAt).toBe(now + 20_000); + expect(github.searches.length).toBe(searches + 2); + + const own = await service.ownPulls(bob, { now: now + 30_000 }); + expect(own?.fetchedAt).toBe(now + 30_000); + expect(own?.requests.map(r => r.number)).toEqual([4]); + expect((await service.ownPulls(bob, { now: now + 40_000, fresh: true }))?.fetchedAt).toBe(now + 40_000); + expect(github.searches.length).toBe(searches + 4); + }); + it('never links another user’s session, and answers null without GitHub access', async () => { github.reviewRequests['alice-token'] = [ { owner: 'acme', repo: 'widgets', number: 1, title: 'Change line ten', author: 'octocat', updatedAt: '2026-10-03T09:00:00Z' },