From 783d9cdc72df5a2bcb4f1d019005139c246006e1 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Fri, 21 Aug 2026 15:10:20 +0200 Subject: [PATCH] feat(review): show the pull request's own context, and allow a bare verdict Reading a diff without the description means re-deriving intent from the code, and without the existing reviews it means repeating a point someone else already made. A collapsible panel above the diff now carries the description as markdown and every existing review with its author, verdict and body. A COMMENTED review with an empty body is dropped: that is what the forge records for a batch of inline comments, and those arrive through the inline pull. A bodiless APPROVED or CHANGES_REQUESTED is kept, because there the verdict is the content. Submitting an approval no longer requires attaching anything. Only a plain comment needs a body or a comment to carry - a verdict stands on its own, in the dialog, in the route validation and in createReview. Fixes a real consequence of the merged-PR checkout fallback: `gh pr view` resolves the pull request from the current branch, so a detached checkout returned no details at all and every forge feature went dark. Details now take the pull request number. Drops an esbuild jsx block from the UI vite config that vite 8 ignores in favour of oxc, and warned about on every test run. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- package-lock.json | 15 ++++ packages/cli/src/index.ts | 3 + packages/cli/src/server.ts | 18 ++-- packages/github/src/detection.ts | 18 +++- packages/github/src/index.ts | 3 +- packages/github/src/pr.ts | 2 +- packages/github/src/reviews.ts | 54 +++++++++++ packages/github/src/types.ts | 12 +++ packages/github/tests/reviews.test.ts | 61 +++++++++++++ packages/ui/package.json | 1 + packages/ui/src/components/diff/diff-page.tsx | 2 + .../src/components/layout/github-dialog.tsx | 14 ++- .../components/layout/pull-request-panel.tsx | 89 +++++++++++++++++++ packages/ui/src/lib/api.ts | 10 +++ packages/ui/src/lib/review-submission.ts | 23 ++++- .../ui/tests/github-dialog-guard.test.tsx | 32 ++++++- packages/ui/tests/pull-request-panel.test.tsx | 59 ++++++++++++ packages/ui/tests/review-submission.test.ts | 21 +++++ packages/ui/vite.config.ts | 1 - 19 files changed, 420 insertions(+), 18 deletions(-) create mode 100644 packages/github/src/reviews.ts create mode 100644 packages/github/tests/reviews.test.ts create mode 100644 packages/ui/src/components/layout/pull-request-panel.tsx create mode 100644 packages/ui/tests/pull-request-panel.test.tsx diff --git a/package-lock.json b/package-lock.json index af28989..1e8ffb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2766,6 +2766,20 @@ } } }, + "node_modules/@testing-library/user-event": { + "version": "14.6.5", + "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-14.6.5.tgz", + "integrity": "sha512-FhqjldLTpteueBaKflhNFlMT3+PM0O5fiBUivht6b9CZ1eesJyy7+g3Jr7XwJzt/Hip3ZG5hWwK1MX1FuDiE4w==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12", + "npm": ">=6" + }, + "peerDependencies": { + "@testing-library/dom": ">=7.21.4" + } + }, "node_modules/@tybys/wasm-util": { "version": "0.10.1", "resolved": "https://registry.npmjs.org/@tybys/wasm-util/-/wasm-util-0.10.1.tgz", @@ -8516,6 +8530,7 @@ "@react-router/dev": "^7.13.2", "@react-router/fs-routes": "^7.13.2", "@testing-library/react": "^16.3.2", + "@testing-library/user-event": "^14.6.5", "@types/nprogress": "^0.2.3", "@types/react": "^19.2.14", "@types/react-dom": "^19.2.3", diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index fedb3ff..77c6f94 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -126,6 +126,7 @@ range syntax (main..feature, main...feature) also work.`) } let prBase: PrBase | null = null; + let parsedPrNumber: number | undefined; if (refs.length === 1 && isGitHubPrUrl(refs[0])) { const parsed = parseGitHubPrUrl(refs[0]); @@ -181,6 +182,7 @@ range syntax (main..feature, main...feature) also work.`) process.exit(1); } + parsedPrNumber = parsed.number; refs[0] = prBase.oid; } @@ -316,6 +318,7 @@ range syntax (main..feature, main...feature) also work.`) description, effectiveRef, pinnedRef: prBase?.oid, + prNumber: parsedPrNumber, version: pkg.version, registryInfo: { repoRoot, repoHash, repoName }, }); diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index fce985f..dad46a7 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -179,6 +179,8 @@ interface ServerOptions { * from. A `ref` in the URL that disagrees is corrected rather than honoured. */ pinnedRef?: string; + /** Set in pull-request mode, so details still resolve on a detached checkout. */ + prNumber?: number; version?: string; registryInfo?: { repoRoot: string; @@ -230,6 +232,7 @@ export function startServer(options: ServerOptions): Promise { version, registryInfo, pinnedRef, + prNumber, } = options; const includeUntracked = diffArgs.length === 0; @@ -488,13 +491,13 @@ export function startServer(options: ServerOptions): Promise { sendJson(res, null); return; } - const details = fetchGitHubDetails(githubRemote.owner, githubRemote.repo); + const details = fetchGitHubDetails(githubRemote.owner, githubRemote.repo, prNumber); sendJson(res, details); return; } if (pathname === '/api/github/create-review' && req.method === 'POST') { - const details = githubRemote ? fetchGitHubDetails(githubRemote.owner, githubRemote.repo) : null; + const details = githubRemote ? fetchGitHubDetails(githubRemote.owner, githubRemote.repo, prNumber) : null; if (!githubRemote || !details?.headSha) { sendError(res, 400, 'No GitHub PR detected'); return; @@ -512,8 +515,13 @@ export function startServer(options: ServerOptions): Promise { const comments = (body.comments ?? []) as PrComment[]; const summary = typeof body.body === 'string' ? body.body : ''; const event = REVIEW_EVENTS.has(body.event) ? (body.event as ReviewEvent) : 'COMMENT'; - if (!Array.isArray(comments) || (comments.length === 0 && !summary.trim())) { - sendError(res, 400, 'A review needs a summary or at least one comment'); + if (!Array.isArray(comments)) { + sendError(res, 400, 'comments must be an array'); + return; + } + // A verdict carries its own meaning; only a plain comment needs something in it. + if (event === 'COMMENT' && comments.length === 0 && !summary.trim()) { + sendError(res, 400, 'A comment review needs a summary or at least one comment'); return; } const result = createGitHubReview( @@ -532,7 +540,7 @@ export function startServer(options: ServerOptions): Promise { sendError(res, 400, 'No GitHub repo detected'); return; } - const details = fetchGitHubDetails(githubRemote.owner, githubRemote.repo); + const details = fetchGitHubDetails(githubRemote.owner, githubRemote.repo, prNumber); if (!details) { sendError(res, 400, 'No GitHub PR detected'); return; diff --git a/packages/github/src/detection.ts b/packages/github/src/detection.ts index 4fcbf43..696ed9f 100644 --- a/packages/github/src/detection.ts +++ b/packages/github/src/detection.ts @@ -1,4 +1,5 @@ import { exec, execSilent } from './exec.js'; +import { getReviews } from './reviews.js'; import type { GitHubRemote, GitHubDetails } from './types.js'; export function getRemote(): { owner: string; repo: string } | null { @@ -30,12 +31,12 @@ export function detectRemote(): GitHubRemote | null { return remote; } -export function fetchDetails(owner: string, repo: string): GitHubDetails | null { +export function fetchDetails(owner: string, repo: string, prNumber?: number): GitHubDetails | null { if (!isCliInstalled() || !isAuthenticated()) { return null; } - const pr = getPr(); + const pr = getPr(prNumber); if (!pr) { return null; } @@ -50,6 +51,8 @@ export function fetchDetails(owner: string, repo: string): GitHubDetails | null headSha: pr.headSha, commentCount, viewerDidAuthor: !!pr.authorLogin && pr.authorLogin === getViewerLogin(), + prBody: pr.body, + reviews: getReviews(owner, repo, pr.number), }; } @@ -60,6 +63,7 @@ interface PrData { headSha: string; createdAt: string; authorLogin: string | null; + body: string; } // gh has no `viewerDidAuthor` field, so authorship is settled by comparing logins. The @@ -77,9 +81,14 @@ function getViewerLogin(): string | null { return viewerLogin; } -function getPr(): PrData | null { +/** + * Without a number, gh resolves the pull request from the current branch — which fails on a + * detached checkout, and a merged pull request has no branch left to check out. + */ +function getPr(prNumber?: number): PrData | null { try { - const json = exec('gh pr view --json number,title,url,headRefOid,createdAt,author'); + const target = prNumber ? `${prNumber} ` : ''; + const json = exec(`gh pr view ${target}--json number,title,url,headRefOid,createdAt,author,body`); const data = JSON.parse(json); if (data.number && data.url && data.headRefOid) { return { @@ -89,6 +98,7 @@ function getPr(): PrData | null { headSha: data.headRefOid, createdAt: data.createdAt, authorLogin: data.author?.login ?? null, + body: data.body ?? '', }; } return null; diff --git a/packages/github/src/index.ts b/packages/github/src/index.ts index e829d17..d1e513c 100644 --- a/packages/github/src/index.ts +++ b/packages/github/src/index.ts @@ -1,4 +1,5 @@ -export type { GitHubRemote, GitHubDetails, PrBase, PrComment, PulledThread, ReviewEvent, ReviewResult, ReviewSubmission } from './types.js'; +export type { GitHubRemote, GitHubDetails, PrBase, PrComment, PrReview, PulledThread, ReviewEvent, ReviewResult, ReviewSubmission } from './types.js'; export { detectRemote, fetchDetails, isCliInstalled, isAuthenticated } from './detection.js'; export { getFiles, getComments, getCommentCount, pullComments, createReview } from './pr.js'; +export { getReviews, parseReviews } from './reviews.js'; export { isGitHubPrUrl, parseGitHubPrUrl, checkoutPr, getPrBase, parsePrBase } from './pr-url.js'; diff --git a/packages/github/src/pr.ts b/packages/github/src/pr.ts index 967c5f2..88ea6d3 100644 --- a/packages/github/src/pr.ts +++ b/packages/github/src/pr.ts @@ -175,7 +175,7 @@ export function createReview( const dropped = errors.length; const body = submission.body.trim(); - if (comments.length === 0 && !body) { + if (comments.length === 0 && !body && submission.event === 'COMMENT') { return { submitted: 0, skipped, failed: dropped, errors, reviewUrl: null }; } diff --git a/packages/github/src/reviews.ts b/packages/github/src/reviews.ts new file mode 100644 index 0000000..6797665 --- /dev/null +++ b/packages/github/src/reviews.ts @@ -0,0 +1,54 @@ +import { execFileSync } from 'node:child_process'; +import type { PrReview } from './types.js'; + +interface RawReview { + user?: { login?: string; type?: string }; + state?: string; + body?: string; + submitted_at?: string; +} + +/** + * A review with no body and no verdict has nothing a reader can act on — the forge records one of + * those for a batch of inline comments, and those arrive through the inline pull instead. + */ +function isWorthShowing(review: PrReview): boolean { + return review.body.trim().length > 0 || review.state !== 'COMMENTED'; +} + +export function parseReviews(json: string): PrReview[] { + let data: unknown; + try { + data = JSON.parse(json); + } catch { + return []; + } + + if (!Array.isArray(data)) { + return []; + } + + return (data as RawReview[]) + .map(review => ({ + author: review.user?.login ?? 'unknown', + isBot: review.user?.type === 'Bot', + state: review.state ?? 'COMMENTED', + body: review.body ?? '', + submittedAt: review.submitted_at ?? '', + })) + .filter(isWorthShowing); +} + +export function getReviews(owner: string, repo: string, prNumber: number): PrReview[] { + try { + return parseReviews( + execFileSync( + 'gh', + ['api', `repos/${owner}/${repo}/pulls/${prNumber}/reviews`, '--paginate'], + { encoding: 'utf-8', stdio: 'pipe', maxBuffer: 10 * 1024 * 1024 }, + ), + ); + } catch { + return []; + } +} diff --git a/packages/github/src/types.ts b/packages/github/src/types.ts index 5723bf3..df6c721 100644 --- a/packages/github/src/types.ts +++ b/packages/github/src/types.ts @@ -12,6 +12,9 @@ export interface GitHubDetails { commentCount: number; /** GitHub refuses to approve or request changes on your own pull request. */ viewerDidAuthor: boolean; + /** The description, which is where the author says what the change is for. */ + prBody: string; + reviews: PrReview[]; } export type ReviewEvent = 'COMMENT' | 'APPROVE' | 'REQUEST_CHANGES'; @@ -30,6 +33,15 @@ export interface ReviewResult { reviewUrl: string | null; } +export interface PrReview { + author: string; + isBot: boolean; + /** APPROVED, CHANGES_REQUESTED, COMMENTED, DISMISSED. */ + state: string; + body: string; + submittedAt: string; +} + export interface PrBase { /** The base branch's name, for display. */ name: string; diff --git a/packages/github/tests/reviews.test.ts b/packages/github/tests/reviews.test.ts new file mode 100644 index 0000000..cde371a --- /dev/null +++ b/packages/github/tests/reviews.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect } from 'vitest'; +import { parseReviews } from '../src/reviews.js'; + +const raw = JSON.stringify([ + { + user: { login: 'copilot-pull-request-reviewer[bot]', type: 'Bot' }, + state: 'COMMENTED', + body: '## Pull request overview\n\nUpdates the experiment form.', + submitted_at: '2026-08-21T11:54:36Z', + }, + { + user: { login: 'fiddur', type: 'User' }, + state: 'APPROVED', + body: 'lgtm', + submitted_at: '2026-08-21T12:11:34Z', + }, + { + user: { login: 'someone', type: 'User' }, + state: 'COMMENTED', + body: '', + submitted_at: '2026-08-21T12:20:00Z', + }, +]); + +describe('parseReviews', () => { + it('keeps author, state, body and time, newest last', () => { + const reviews = parseReviews(raw); + + expect(reviews).toHaveLength(2); + expect(reviews[0]).toEqual({ + author: 'copilot-pull-request-reviewer[bot]', + isBot: true, + state: 'COMMENTED', + body: '## Pull request overview\n\nUpdates the experiment form.', + submittedAt: '2026-08-21T11:54:36Z', + }); + expect(reviews[1].author).toBe('fiddur'); + expect(reviews[1].state).toBe('APPROVED'); + }); + + it('drops a review with nothing to read', () => { + // A bodiless COMMENTED review is what the forge records for inline-only comments; the inline + // comments themselves come through the existing pull, so there is nothing to show here. + expect(parseReviews(raw).some(review => review.author === 'someone')).toBe(false); + }); + + it('keeps a bodiless approval, because the verdict is the content', () => { + const approvals = JSON.stringify([ + { user: { login: 'a', type: 'User' }, state: 'APPROVED', body: '', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'b', type: 'User' }, state: 'CHANGES_REQUESTED', body: '', submitted_at: '2026-01-02T00:00:00Z' }, + ]); + + expect(parseReviews(approvals).map(r => r.state)).toEqual(['APPROVED', 'CHANGES_REQUESTED']); + }); + + it('survives junk rather than failing the page', () => { + expect(parseReviews('not json')).toEqual([]); + expect(parseReviews('{}')).toEqual([]); + expect(parseReviews('[]')).toEqual([]); + }); +}); diff --git a/packages/ui/package.json b/packages/ui/package.json index 02f5baa..353fd13 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -37,6 +37,7 @@ "@react-router/dev": "^7.13.2", "@react-router/fs-routes": "^7.13.2", "@testing-library/react": "^16.3.2", + "@testing-library/user-event": "^14.6.5", "@types/nprogress": "^0.2.3", "@types/react": "^19.2.14", "@types/react-dom": "^19.2.3", diff --git a/packages/ui/src/components/diff/diff-page.tsx b/packages/ui/src/components/diff/diff-page.tsx index eb4ef2d..9239455 100644 --- a/packages/ui/src/components/diff/diff-page.tsx +++ b/packages/ui/src/components/diff/diff-page.tsx @@ -18,6 +18,7 @@ import { Sidebar } from '../layout/sidebar'; import { ShortcutModal } from '../layout/shortcut-modal'; import { StaleDiffBanner } from '../layout/stale-diff-banner'; import { ReviewProgressBanner } from '../layout/review-progress-banner'; +import { PullRequestPanel } from '../layout/pull-request-panel'; import { CheckCircleIcon } from '../icons/check-circle-icon'; import { PageLoader } from '../layout/skeleton'; import { useDiffStaleness } from '../../hooks/use-diff-staleness'; @@ -423,6 +424,7 @@ export function DiffPage() { onGitHubPulled={() => queryClient.invalidateQueries({ queryKey: ['threads'] })} /> {isStale && } + {info?.review?.inProgress && ( )} diff --git a/packages/ui/src/components/layout/github-dialog.tsx b/packages/ui/src/components/layout/github-dialog.tsx index 45149b3..cbe77d1 100644 --- a/packages/ui/src/components/layout/github-dialog.tsx +++ b/packages/ui/src/components/layout/github-dialog.tsx @@ -14,6 +14,7 @@ import { } from '../../lib/api'; import type { CommentThread } from '../comments/types'; import { + canSubmitReview, isSubmittable, summaryFromGeneralThreads, threadToPayload, @@ -86,7 +87,12 @@ export function GitHubDialog(props: GitHubDialogProps) { }, [onClose]); const chosen = submittable.filter(thread => selected.has(thread.id)); - const canSubmit = !reviewInProgress && (chosen.length > 0 || summary.trim().length > 0); + const canSubmit = canSubmitReview({ + event, + comments: chosen.length, + summary, + reviewInProgress, + }); const toggle = (id: string) => { setSelected(prev => { @@ -304,7 +310,11 @@ export function GitHubDialog(props: GitHubDialogProps) { ) : ( )} - Submit {chosen.length > 0 ? `${chosen.length} ` : ''}as one review + {chosen.length > 0 + ? `Submit ${chosen.length} as one review` + : event === 'COMMENT' + ? 'Submit as one review' + : `Submit ${EVENT_LABELS[event].toLowerCase()}`} diff --git a/packages/ui/src/components/layout/pull-request-panel.tsx b/packages/ui/src/components/layout/pull-request-panel.tsx new file mode 100644 index 0000000..0731871 --- /dev/null +++ b/packages/ui/src/components/layout/pull-request-panel.tsx @@ -0,0 +1,89 @@ +import { useState } from 'react'; +import dayjs from 'dayjs'; +import relativeTime from 'dayjs/plugin/relativeTime'; +import type { GitHubDetails } from '../../lib/api'; +import { MarkdownContent } from './markdown-content'; +import { ChevronDownIcon } from '../icons/chevron-down-icon'; +import { ChevronUpIcon } from '../icons/chevron-up-icon'; + +dayjs.extend(relativeTime); + +interface PullRequestPanelProps { + details: GitHubDetails | null; +} + +function stateClass(state: string): string { + if (state === 'APPROVED') { + return 'text-added'; + } + if (state === 'CHANGES_REQUESTED') { + return 'text-deleted'; + } + return 'text-text-muted'; +} + +/** + * What the author says the change is for, and what other reviewers have already said. Reading a + * diff without either means re-deriving the intent from the code, and repeating a point someone + * else has already made. + */ +export function PullRequestPanel(props: PullRequestPanelProps) { + const { details } = props; + const [open, setOpen] = useState(true); + + if (!details) { + return null; + } + + const { prBody, reviews } = details; + + return ( +
+ + + {open && ( +
+
+ {prBody.trim() ? ( + + ) : ( + No description on the pull request. + )} +
+ + {reviews.map((review, index) => ( +
+
+ {review.author} + {review.isBot && ( + bot + )} + {review.state} + {review.submittedAt && ( + {dayjs(review.submittedAt).fromNow()} + )} +
+ {review.body.trim() && ( +
+ +
+ )} +
+ ))} +
+ )} +
+ ); +} diff --git a/packages/ui/src/lib/api.ts b/packages/ui/src/lib/api.ts index 979f1c3..037cee1 100644 --- a/packages/ui/src/lib/api.ts +++ b/packages/ui/src/lib/api.ts @@ -37,6 +37,14 @@ export interface GitHubRemote { repo: string; } +export interface PrReview { + author: string; + isBot: boolean; + state: string; + body: string; + submittedAt: string; +} + export interface GitHubDetails { prNumber: number; prTitle: string; @@ -45,6 +53,8 @@ export interface GitHubDetails { headSha: string; commentCount: number; viewerDidAuthor: boolean; + prBody: string; + reviews: PrReview[]; } export interface ReviewRun { diff --git a/packages/ui/src/lib/review-submission.ts b/packages/ui/src/lib/review-submission.ts index 0128291..088253d 100644 --- a/packages/ui/src/lib/review-submission.ts +++ b/packages/ui/src/lib/review-submission.ts @@ -1,6 +1,6 @@ import type { CommentThread } from '../components/comments/types'; import { GENERAL_THREAD_FILE_PATH, isThreadResolved } from '../components/comments/types'; -import type { PrCommentPayload } from './api'; +import type { PrCommentPayload, ReviewEvent } from './api'; /** * A thread's replies are part of the same finding, so they are folded into the one comment @@ -39,3 +39,24 @@ export function summaryFromGeneralThreads(threads: CommentThread[]): string { .flatMap(thread => thread.comments.map(comment => comment.body)) .join('\n\n'); } + +/** + * A plain comment needs something to say. A verdict does not: an approval with nothing attached + * is a normal thing to send, and the forge accepts it. + */ +export function canSubmitReview(input: { + event: ReviewEvent; + comments: number; + summary: string; + reviewInProgress?: boolean; +}): boolean { + if (input.reviewInProgress) { + return false; + } + + if (input.event !== 'COMMENT') { + return true; + } + + return input.comments > 0 || input.summary.trim().length > 0; +} diff --git a/packages/ui/tests/github-dialog-guard.test.tsx b/packages/ui/tests/github-dialog-guard.test.tsx index a5b5241..f90a01d 100644 --- a/packages/ui/tests/github-dialog-guard.test.tsx +++ b/packages/ui/tests/github-dialog-guard.test.tsx @@ -1,5 +1,6 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { render, cleanup, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { GitHubDialog } from '../src/components/layout/github-dialog'; import type { GitHubDetails } from '../src/lib/api'; import type { CommentThread } from '../src/components/comments/types'; @@ -28,11 +29,11 @@ function thread(id: string): CommentThread { }; } -function renderDialog(reviewInProgress: boolean) { +function renderDialog(reviewInProgress: boolean, threads = [thread('t1')]) { return render( /as one review/i.test(button.textContent ?? '')); + .find(button => /^submit/i.test((button.textContent ?? '').trim())); if (!found) { throw new Error('submit button not found'); } @@ -68,3 +69,28 @@ describe('submitting while a review is still running', () => { expect(screen.queryByText(/still in progress/i)).toBeNull(); }); }); + +describe('approving with nothing attached', () => { + it('is allowed once the comments are deselected', async () => { + const user = userEvent.setup(); + renderDialog(false); + + await user.click(screen.getByRole('button', { name: /deselect all/i })); + // A plain comment with nothing in it says nothing, so it stays refused... + expect(submitButton().disabled).toBe(true); + + // ...but a verdict stands on its own. + await user.click(screen.getByRole('button', { name: /^approve$/i })); + expect(submitButton().disabled).toBe(false); + expect(submitButton().textContent).toMatch(/approve/i); + }); + + it('is allowed with no findings at all', async () => { + const user = userEvent.setup(); + renderDialog(false, []); + + expect(submitButton().disabled).toBe(true); + await user.click(screen.getByRole('button', { name: /^approve$/i })); + expect(submitButton().disabled).toBe(false); + }); +}); diff --git a/packages/ui/tests/pull-request-panel.test.tsx b/packages/ui/tests/pull-request-panel.test.tsx new file mode 100644 index 0000000..fb8d621 --- /dev/null +++ b/packages/ui/tests/pull-request-panel.test.tsx @@ -0,0 +1,59 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup, screen } from '@testing-library/react'; +import { PullRequestPanel } from '../src/components/layout/pull-request-panel'; +import type { GitHubDetails } from '../src/lib/api'; + +function details(overrides: Partial = {}): GitHubDetails { + return { + prNumber: 671, + prTitle: 'fix: keep experiment date range in sync', + prUrl: 'https://github.com/o/r/pull/671', + prCreatedAt: '2026-08-21T10:00:00.000Z', + headSha: 'abc123', + commentCount: 0, + viewerDidAuthor: false, + prBody: '## Summary\n\nThe field went stale until a refresh.', + reviews: [], + ...overrides, + }; +} + +afterEach(cleanup); + +describe('PullRequestPanel', () => { + it('shows the description, which is where the author says what the change is for', () => { + render(); + + expect(screen.getByText(/went stale until a refresh/i)).toBeTruthy(); + }); + + it('shows what other reviewers already said', () => { + render( + , + ); + + expect(screen.getByText('copilot[bot]')).toBeTruthy(); + expect(screen.getByText(/Overview of the change/)).toBeTruthy(); + expect(screen.getByText('fiddur')).toBeTruthy(); + expect(screen.getByText('APPROVED')).toBeTruthy(); + }); + + it('says so plainly when there is no description', () => { + render(); + + expect(screen.getByText(/no description/i)).toBeTruthy(); + }); + + it('renders nothing at all without a pull request', () => { + const { container } = render(); + + expect(container.textContent).toBe(''); + }); +}); diff --git a/packages/ui/tests/review-submission.test.ts b/packages/ui/tests/review-submission.test.ts index aa2b1aa..23ffa81 100644 --- a/packages/ui/tests/review-submission.test.ts +++ b/packages/ui/tests/review-submission.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; import { + canSubmitReview, isGeneral, isSubmittable, summaryFromGeneralThreads, @@ -105,3 +106,23 @@ describe('summaryFromGeneralThreads', () => { expect(isGeneral(thread())).toBe(false); }); }); + +describe('canSubmitReview', () => { + it('needs something to say for a plain comment', () => { + expect(canSubmitReview({ event: 'COMMENT', comments: 0, summary: '' })).toBe(false); + expect(canSubmitReview({ event: 'COMMENT', comments: 0, summary: ' ' })).toBe(false); + expect(canSubmitReview({ event: 'COMMENT', comments: 1, summary: '' })).toBe(true); + expect(canSubmitReview({ event: 'COMMENT', comments: 0, summary: 'looks fine' })).toBe(true); + }); + + it('lets a verdict stand on its own', () => { + // An approval with nothing attached is a normal thing to send, and the forge accepts it. + expect(canSubmitReview({ event: 'APPROVE', comments: 0, summary: '' })).toBe(true); + expect(canSubmitReview({ event: 'REQUEST_CHANGES', comments: 0, summary: '' })).toBe(true); + }); + + it('refuses anything while a review is still running', () => { + expect(canSubmitReview({ event: 'APPROVE', comments: 0, summary: '', reviewInProgress: true })).toBe(false); + expect(canSubmitReview({ event: 'COMMENT', comments: 5, summary: 'x', reviewInProgress: true })).toBe(false); + }); +}); diff --git a/packages/ui/vite.config.ts b/packages/ui/vite.config.ts index d67866f..cac0dba 100644 --- a/packages/ui/vite.config.ts +++ b/packages/ui/vite.config.ts @@ -8,7 +8,6 @@ const isTest = !!process.env.VITEST; export default defineConfig({ plugins: isTest ? [tailwindcss()] : [tailwindcss(), reactRouter()], - esbuild: { jsx: "automatic", jsxImportSource: "react" }, test: { include: ["tests/**/*.test.{ts,tsx}"], // Component tests render for real: a render-time ReferenceError has nothing else catching it.