diff --git a/packages/webui/src/client/components/DiffCard.tsx b/packages/webui/src/client/components/DiffCard.tsx index 9ecb45ed..8305d294 100644 --- a/packages/webui/src/client/components/DiffCard.tsx +++ b/packages/webui/src/client/components/DiffCard.tsx @@ -43,14 +43,23 @@ export function reduceWebuiDiffState( ): WebuiDiffState { switch (action.type) { case "loaded": - return { ...state, view: action.view, unsupported: false, busy: false }; + return { ...state, view: action.view, unsupported: false, busy: false, mutationError: undefined }; case "unsupported": + return { ...state, unsupported: true, busy: false, mutationError: undefined }; + /* A failed mutation is not an absent capability: the runtime answered, it + * just declined this operation. Keeping `unsupported` false is what leaves + * the card interactive, and `buildWebuiDiffMutationRequest` gating on + * `unsupported` is what otherwise makes the first failure permanent. */ case "mutation-failed": - return { ...state, unsupported: true, busy: false }; + return { ...state, unsupported: false, busy: false, mutationError: trimmedOrUndefined(action.error) }; case "begin-mutation": - return state.busy ? state : { ...state, busy: true }; + // A retry supersedes the previous reason; leaving it up would report a + // stale failure next to an attempt that is still running. + return state.busy ? state : { ...state, busy: true, mutationError: undefined }; case "mutation-succeeded": - return { ...state, view: action.view, unsupported: false, busy: false }; + return { ...state, view: action.view, unsupported: false, busy: false, mutationError: undefined }; + case "dismiss-mutation-error": + return { ...state, mutationError: undefined }; case "toggle-expanded": return { ...state, expanded: !state.expanded }; case "toggle-review": @@ -58,6 +67,85 @@ export function reduceWebuiDiffState( } } +/** A blank or whitespace-only reason explains nothing, so it is dropped + * rather than rendered as an empty banner. */ +function trimmedOrUndefined(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed ? trimmed : undefined; +} + +/* What the runtime says when a revert or reapply does not apply, and what a + * person can do about it. + * + * The reasons are machine tokens, and the authoritative one is + * `applyLocalTurnDiffSnapshotMutation`'s `unsafe_path`. Read the runtime, not + * the name: `normalizeCapturePath` *accepts* a path resolving outside the + * workspace, and returns `undefined` for in-workspace paths the capture layer + * filters (`.git/`, `node_modules/`, the root itself). The `safeCapturedPath` + * check also sits inside the write loop, so the operation is NOT all-or-nothing + * — earlier entries have already been written or `fs.rm`'d. The copy below + * therefore says the run was interrupted and earlier files may have changed, + * because that is what happened. Surfacing the raw token tells the user + * nothing, and the other codes read the same way — `conflict` in particular + * does not say that the files changed *after* the turn, which is the part that + * matters. + * + * A reason not in this table is passed through unchanged. That is deliberate: + * `git apply` failures arrive as free-form stderr, and discarding them would + * replace a real message with a guess. */ +const WEBUI_DIFF_FAILURE_COPY: Readonly> = { + unsafe_path: "这轮改动里有文件的路径无法安全定位，操作已中断，之前处理过的文件可能已改动。", + conflict: "这轮改动之后文件又被修改过，撤销前请先确认当前内容。", + not_undoable: "这轮文件改动没有留下可撤销的快照。", + "Turn diff not found": "找不到这轮文件改动。", + "Only the latest turn diff can be changed": "只能撤销最近一轮的文件改动。", + "Turn diff is not undoable": "这轮文件改动没有可撤销的补丁。", +}; + +export function describeWebuiDiffFailure(reason: string | undefined): string | undefined { + const trimmed = trimmedOrUndefined(reason); + if (!trimmed) return undefined; + return WEBUI_DIFF_FAILURE_COPY[trimmed] ?? trimmed; +} + +/** Decides what a revert/reapply round trip meant. + * + * The two operations nest the resulting view differently, and reading either + * shape as success is a silent failure: + * + * * `revertTurnDiff` answers `{ success?, error?, turnDiff? }` — the view + * lives under `turnDiff`, and a refusal carries only `error`. + * * `reapplyTurnDiff` answers with the view itself (`success` required, + * `error` optional), so for reapply the result *is* the next view. + * + * Two independent things are therefore checked: whether the runtime said it + * worked, and whether it handed back an applied file list. A view with no + * files applied nothing, and the card renders `null` for an empty file list, + * so accepting one would delete the card with no message at all. */ +export function resolveWebuiDiffMutation( + action: "revert" | "reapply", + result: WebuiRevertTurnDiffResult | WebuiReapplyTurnDiffResult | undefined, + thrown?: unknown, +): WebuiDiffStateAction { + if (thrown !== undefined) { + const message = thrown instanceof Error ? thrown.message : undefined; + return { type: "mutation-failed", error: describeWebuiDiffFailure(message) }; + } + // `success` is required on reapply and optional on revert; an explicit + // `false` is a refusal even if a view rode along beside it. + if (result?.success === false) { + return { type: "mutation-failed", error: describeWebuiDiffFailure(result.error) }; + } + const nextView = + action === "revert" + ? (result as WebuiRevertTurnDiffResult | undefined)?.turnDiff + : (result as WebuiReapplyTurnDiffResult | undefined); + if (nextView && (nextView.fileChanges ?? []).length > 0) { + return { type: "mutation-succeeded", view: nextView }; + } + return { type: "mutation-failed", error: describeWebuiDiffFailure(result?.error) }; +} + export function buildWebuiDiffMutationRequest( state: WebuiDiffState, request: WebuiGetTurnDiffRequest, @@ -103,7 +191,7 @@ export function WebuiDiffCard({ ...initialState, ...(initialView ? { view: initialView } : {}), })); - const { view, unsupported, busy, expanded, reviewing } = diffState; + const { view, unsupported, busy, expanded, reviewing, mutationError } = diffState; const request = useMemo(() => { if (!sessionId || !getTurnDiff) return undefined; // The runtime keys a turn diff by the turn: an `assistantMessageId` is @@ -150,13 +238,9 @@ export function WebuiDiffCard({ setDiffState((current) => confirmWebuiDiffMutation(current, confirmed)); try { const result = await handler(mutationRequest); - const nextView = action === "revert" - ? (result as WebuiRevertTurnDiffResult).turnDiff - : (result as WebuiReapplyTurnDiffResult); - if (nextView) setDiffState((current) => reduceWebuiDiffState(current, { type: "mutation-succeeded", view: nextView })); - else setDiffState((current) => reduceWebuiDiffState(current, { type: "mutation-failed" })); - } catch { - setDiffState((current) => reduceWebuiDiffState(current, { type: "mutation-failed" })); + setDiffState((current) => reduceWebuiDiffState(current, resolveWebuiDiffMutation(action, result))); + } catch (error) { + setDiffState((current) => reduceWebuiDiffState(current, resolveWebuiDiffMutation(action, undefined, error))); } finally { setDiffState((current) => ({ ...current, busy: false })); } @@ -248,6 +332,20 @@ export function WebuiDiffCard({ ))} ) : null} + {mutationError ? ( + /* `role="alert"` because this replaces what used to be no feedback at + * all: the operation did nothing, and the file list above is now known + * to be out of date. The controls stay live so the user can retry. */ +
+ + 这轮文件改动没有生效 + {mutationError} + + +
+ ) : null} ); } diff --git a/packages/webui/src/client/components/SettingsModal.tsx b/packages/webui/src/client/components/SettingsModal.tsx index 366f5e05..3377d53a 100644 --- a/packages/webui/src/client/components/SettingsModal.tsx +++ b/packages/webui/src/client/components/SettingsModal.tsx @@ -1,6 +1,24 @@ -import { useEffect, useMemo, useState, type ReactElement, type ReactNode } from "react"; -import type { WebuiModelEntry, WebuiSessionListItem, WebuiVersionInfo } from "../../server/port.js"; +import { useCallback, useEffect, useMemo, useState, type ReactElement, type ReactNode } from "react"; +import type { WebuiModelEntry, WebuiSessionListItem, WebuiVersionInfo, WebuiWorkspaceReviewFileDiff } from "../../server/port.js"; import type { WebuiTransport } from "../contracts.js"; +import { + initialWebuiReviewState, + isWebuiReviewFiltering, + projectWebuiReviewLines, + reduceWebuiReviewState, + selectWebuiReviewVisibleFiles, + webuiReviewLineTarget, + WEBUI_REVIEW_DIFF_BATCH_SIZE, + type WebuiReviewFile, + type WebuiReviewState, +} from "../projection/review-state.js"; +import { + groupWebuiWorktreeWorkspaces, + selectWebuiPrimaryWorkspace, + selectWebuiWorktreeWorkspaces, + type WebuiWorktreeSourceSession, + type WebuiWorktreeWorkspace, +} from "../projection/worktree-state.js"; import { ToggleSwitch as Switch } from "./ToggleSwitch.js"; import { UsageModelSettings } from "./settings/UsageModelSettings.js"; @@ -8,7 +26,7 @@ export type SettingsTabKey = "desktop" | "shortcuts" | "voice" | "custom-instruc export interface SettingsTabDefinition { readonly key: SettingsTabKey; readonly group: "preferences" | "management" | "coding" | "archived"; readonly label: string; readonly icon: string; readonly disabled?: boolean; } export const DESKTOP_SETTINGS_TABS: readonly SettingsTabDefinition[] = [ { key: "desktop", group: "preferences", label: "通用", icon: "desktop" }, { key: "voice", group: "preferences", label: "语音", icon: "voice", disabled: true }, { key: "shortcuts", group: "preferences", label: "快捷键", icon: "shortcuts", disabled: true }, { key: "custom-instructions", group: "preferences", label: "个性化", icon: "custom-instructions", disabled: true }, - { key: "usage", group: "management", label: "用量与模型", icon: "chart" }, { key: "connection", group: "management", label: "连接", icon: "link", disabled: true }, { key: "account", group: "management", label: "账户", icon: "user" }, { key: "coding", group: "coding", label: "代码审查", icon: "coding", disabled: true }, { key: "worktree", group: "coding", label: "工作树", icon: "worktree", disabled: true }, { key: "archived", group: "archived", label: "已归档任务", icon: "archived" }, + { key: "usage", group: "management", label: "用量与模型", icon: "chart" }, { key: "connection", group: "management", label: "连接", icon: "link", disabled: true }, { key: "account", group: "management", label: "账户", icon: "user" }, { key: "coding", group: "coding", label: "代码审查", icon: "coding" }, { key: "worktree", group: "coding", label: "工作树", icon: "worktree" }, { key: "archived", group: "archived", label: "已归档任务", icon: "archived" }, ]; export const SETTINGS_GROUPS = [{ key: "preferences", label: "偏好" }, { key: "management", label: "管理" }, { key: "coding", label: "编码" }, { key: "archived", label: "归档" }] as const; export function resolveThemePreference(preference: string, systemDark: boolean): "light" | "dark" { return preference === "system" ? (systemDark ? "dark" : "light") : preference === "dark" ? "dark" : "light"; } @@ -73,6 +91,10 @@ export type WebuiSettingsModalCapabilities = Pick< | "startCodexOAuthLogin" | "cancelCodexOAuthLogin" | "refreshModels" + | "getWorkspaceReviewSummary" + | "listWorkspaceReviewFileDiffs" + | "searchWorkspaceReviewDiffs" + | "loadSessions" >; interface SettingsModalProps { @@ -81,13 +103,20 @@ interface SettingsModalProps { readonly dataDir?: string; readonly version?: WebuiVersionInfo; readonly sessionId?: string; - /** Capability source for the modal. Typed as the narrow 9-member - * contract so the modal cannot accidentally start reading members it - * does not consume. */ + /** Workspace the code-review page reads its change set from. Every + * workspace review operation is keyed by an absolute directory, so without + * this the page has nothing to ask about. */ + readonly workspaceDir?: string; + /** Opens a file at a line. This is what makes a review line a real jump + * rather than decoration: the editor receives the path and the new-side + * line number the diff projection resolved. */ + readonly onOpenFileLine?: (path: string, line: number) => void; + /** Capability source for the modal. Typed as the narrow contract so the + * modal cannot accidentally start reading members it does not consume. */ readonly transport?: WebuiSettingsModalCapabilities; } -export function SettingsModal({ open, onClose, dataDir, version, sessionId, transport }: SettingsModalProps): ReactElement | null { +export function SettingsModal({ open, onClose, dataDir, version, sessionId, workspaceDir, onOpenFileLine, transport }: SettingsModalProps): ReactElement | null { // The transport is optional. Each capability is optional too, so we bind // only when both are present; otherwise we surface `undefined` and let the // call sites do their existing null checks. @@ -119,6 +148,10 @@ export function SettingsModal({ open, onClose, dataDir, version, sessionId, tran startCodexOAuthLogin: transport?.startCodexOAuthLogin?.bind(transport), cancelCodexOAuthLogin: transport?.cancelCodexOAuthLogin?.bind(transport), refreshModels: transport?.refreshModels?.bind(transport), + getWorkspaceReviewSummary: transport?.getWorkspaceReviewSummary?.bind(transport), + listWorkspaceReviewFileDiffs: transport?.listWorkspaceReviewFileDiffs?.bind(transport), + searchWorkspaceReviewDiffs: transport?.searchWorkspaceReviewDiffs?.bind(transport), + loadSessions: transport?.loadSessions?.bind(transport), }), [transport]); const { listModels, selectModel, getUsageQuota, getAccountStatus, @@ -130,6 +163,8 @@ export function SettingsModal({ open, onClose, dataDir, version, sessionId, tran getMiniMaxModelSource, setMiniMaxModelSource, testUserModelCandidate, revealModelProviderApiKey, startCodexOAuthLogin, cancelCodexOAuthLogin, refreshModels, + getWorkspaceReviewSummary, listWorkspaceReviewFileDiffs, searchWorkspaceReviewDiffs, + loadSessions, } = boundCapabilities; const usageCapabilities: WebuiSettingsModalCapabilities = useMemo(() => ({ getUsageQuota, getMiniMaxApiKeyStatus, listUserModelProviders, createUserModelProvider, @@ -146,7 +181,7 @@ export function SettingsModal({ open, onClose, dataDir, version, sessionId, tran const visibleTabs = useMemo(() => filterSettingsTabs(query), [query]); if (!open) return null; const selected = models.find((model) => model.selected); const modelValue = selected ? `${selected.providerId}/${selected.modelId}/${selected.variant ?? ""}` : ""; const groups = SETTINGS_GROUPS.map((group) => ({ ...group, tabs: visibleTabs.filter((tab) => tab.group === group.key) })).filter((group) => group.tabs.length > 0); const label = DESKTOP_SETTINGS_TABS.find((tab) => tab.key === active)?.label; const changeModel = async (value: string) => { const model = models.find((candidate) => `${candidate.providerId}/${candidate.modelId}/${candidate.variant ?? ""}` === value); if (!model || !selectModel) return; await selectModel({ providerId: model.providerId, modelId: model.modelId, ...(model.variant ? { variant: model.variant } : {}), ...(sessionId ? { sessionId } : {}) }); }; const handleSignOut = async () => { if (!signOut) return; try { setSignOutError(undefined); await signOut(); onClose(); } catch (error) { setSignOutError(error instanceof Error ? error.message : String(error)); } }; const handleDeleteAllArchived = async () => { if (!deleteSession || !archived.length || !window.confirm("确定删除全部已归档任务吗？此操作无法撤销。")) return; const ids = archived.map((session) => session.sessionId); try { await Promise.all(ids.map((id) => deleteSession({ id }))); setArchived([]); } catch (error) { window.alert(`删除失败：${error instanceof Error ? error.message : String(error)}`); } }; - return
{ if (event.target === event.currentTarget) onClose(); }}>
event.stopPropagation()}>

{label}

{active === "archived" ? : null}
{active === "desktop" ? : null}{active === "usage" ? : null}{active === "account" ?
{signOutError ?

{signOutError}

: null}
: null}{active === "archived" ? { if (!deleteSession) return; await deleteSession({ id }); setArchived((items) => items.filter((item) => item.sessionId !== id)); }} onUnarchive={async (id) => { if (!archiveSession) return; await archiveSession({ id, archived: false }); setArchived((items) => items.filter((item) => item.sessionId !== id)); }} /> : null}{active !== "desktop" && active !== "usage" && active !== "account" && active !== "archived" ?
: null}{active === "desktop" && dataDir ?

{dataDir}

: null}
; + return
{ if (event.target === event.currentTarget) onClose(); }}>
event.stopPropagation()}>

{label}

{active === "archived" ? : null}
{active === "desktop" ? : null}{active === "usage" ? : null}{active === "account" ?
{signOutError ?

{signOutError}

: null}
: null}{active === "archived" ? { if (!deleteSession) return; await deleteSession({ id }); setArchived((items) => items.filter((item) => item.sessionId !== id)); }} onUnarchive={async (id) => { if (!archiveSession) return; await archiveSession({ id, archived: false }); setArchived((items) => items.filter((item) => item.sessionId !== id)); }} /> : null}{active === "coding" ? : null}{active === "worktree" ? : null}{active !== "desktop" && active !== "usage" && active !== "account" && active !== "archived" && active !== "coding" && active !== "worktree" ?
: null}{active === "desktop" && dataDir ?

{dataDir}

: null}
; } function GenericPage({ theme, setTheme, wrap, setWrap, newTab, setNewTab, contextWindow, setContextWindow, version }: { readonly theme: string; readonly setTheme: (value: string) => void; readonly wrap: boolean; readonly setWrap: (value: boolean) => void; readonly newTab: boolean; readonly setNewTab: (value: boolean) => void; readonly contextWindow: boolean; readonly setContextWindow: (value: boolean) => void; readonly version: string }): ReactElement { @@ -155,6 +190,408 @@ function GenericPage({ theme, setTheme, wrap, setWrap, newTab, setNewTab, contex function Appearance({ theme, setTheme }: { readonly theme: string; readonly setTheme: (value: string) => void }): ReactElement { return
{([['light', '浅色模式', 'light.d3fbb1aa.svg'], ['dark', '深色模式', 'dark.14c569ba.svg'], ['system', '跟随系统', 'system.aba90841.svg']] as const).map(([value, text, src]) => )}
; } function ModeCard({ testId, title, description, icon, selected = false }: { readonly testId: string; readonly title: string; readonly description: string; readonly icon: string; readonly selected?: boolean }): ReactElement { return ; } function SettingPanel({ title, children }: { readonly title: string; readonly children: ReactNode }): ReactElement { return

{title}

{children}
; } +/** The subset of the settings capability contract the code-review page reads. + * Named so the page cannot quietly grow a dependency on a member the modal + * does not otherwise use. */ +type WebuiSettingsReviewCapabilities = Pick< + WebuiSettingsModalCapabilities, + "getWorkspaceReviewSummary" | "listWorkspaceReviewFileDiffs" | "searchWorkspaceReviewDiffs" +>; + +type WebuiReviewStateAction = Parameters[1]; + +/** Roadmap E 区「Review 审查模式」and「修复建议+跳转」. + * + * The review capability was already wired end to end; what was missing was a + * place to read a whole change set. Every decision that does not need the + * network lives in `review-state.ts` so it can be tested without a DOM — this + * component is the thin shell that fetches and renders. */ +function SettingsReviewPage({ workspaceDir, onOpenFileLine, loadSessions, getWorkspaceReviewSummary, listWorkspaceReviewFileDiffs, searchWorkspaceReviewDiffs }: { + readonly workspaceDir?: string; + readonly onOpenFileLine?: (path: string, line: number) => void; + readonly loadSessions?: WebuiSettingsModalCapabilities["loadSessions"]; +} & WebuiSettingsReviewCapabilities): ReactElement { + const [state, setState] = useState(initialWebuiReviewState); + const dispatch = useCallback((action: WebuiReviewStateAction) => { + setState((current) => reduceWebuiReviewState(current, action)); + }, []); + + /* The prop is the selected session's workspace, and it is empty whenever the + * selected session is not bound to a project — including the default + * workspace, which the rail groups under 「未选项目」. A workspace chosen here + * overrides the prop rather than replacing it, so picking one takes effect + * immediately instead of waiting for the user to go change session in the + * rail, which the `aria-modal` dialog does not even let them click. */ + const [pickedWorkspaceDir, setPickedWorkspaceDir] = useState(undefined); + const [workspaceChoices, setWorkspaceChoices] = useState([]); + const [workspaceChoicesLoading, setWorkspaceChoicesLoading] = useState(false); + const [workspaceChoicesError, setWorkspaceChoicesError] = useState(undefined); + const effectiveWorkspaceDir = workspaceDir?.trim() || pickedWorkspaceDir; + + /* Only the chooser needs the session list, so a page opened from a + * workspace-bound session never pays for this fetch. */ + useEffect(() => { + if (workspaceDir || !loadSessions) return; + let cancelled = false; + setWorkspaceChoicesLoading(true); + setWorkspaceChoicesError(undefined); + void loadSessions() + .then((page) => { + if (cancelled) return; + setWorkspaceChoices((page?.sessions ?? []) as readonly WebuiWorktreeSourceSession[]); + setWorkspaceChoicesLoading(false); + }) + .catch((cause: unknown) => { + if (cancelled) return; + setWorkspaceChoicesError(cause instanceof Error ? cause.message : String(cause)); + setWorkspaceChoicesLoading(false); + }); + return () => { cancelled = true; }; + }, [loadSessions, workspaceDir]); + + useEffect(() => { + if (!effectiveWorkspaceDir || !getWorkspaceReviewSummary) return; + let cancelled = false; + dispatch({ type: "load-begun" }); + void getWorkspaceReviewSummary({ workspaceDir: effectiveWorkspaceDir }) + .then((summary) => { + if (cancelled) return; + const snapshotId = summary?.reviewSnapshotId; + const files = summary?.files ?? []; + // A workspace with nothing staged against it is a normal state, and + // showing it as a failure would put a red banner on every fresh repo. + if (!snapshotId || !files.length) { + dispatch({ type: "unavailable", reason: "当前工作区没有待审查的变更" }); + return; + } + const totals = summary?.totals ?? { + files: files.length, + additions: files.reduce((sum, file) => sum + (file.additions ?? 0), 0), + deletions: files.reduce((sum, file) => sum + (file.deletions ?? 0), 0), + }; + dispatch({ type: "summary-loaded", reviewSnapshotId: snapshotId, files: [...files], totals }); + }) + .catch((error: unknown) => { + if (cancelled) return; + dispatch({ type: "load-failed", reason: error instanceof Error ? error.message : String(error) }); + }); + return () => { cancelled = true; }; + }, [dispatch, getWorkspaceReviewSummary, effectiveWorkspaceDir]); + + /* The snapshot id and the query are passed in rather than read back out of + * state: reading a `useState` value from inside a setter callback is a type + * error waiting to happen and silently captures whatever the reducer saw, + * not what the caller meant. */ + const loadDiffs = useCallback(async (fileIds: readonly string[], reviewSnapshotId: string) => { + if (!effectiveWorkspaceDir || !listWorkspaceReviewFileDiffs || !fileIds.length) return; + dispatch({ type: "diffs-begun", fileIds }); + for (let index = 0; index < fileIds.length; index += WEBUI_REVIEW_DIFF_BATCH_SIZE) { + const batch = fileIds.slice(index, index + WEBUI_REVIEW_DIFF_BATCH_SIZE); + try { + const result = await listWorkspaceReviewFileDiffs({ workspaceDir: effectiveWorkspaceDir, reviewSnapshotId, fileIds: [...batch] }); + const diffs: Record = {}; + const errors: Record = {}; + for (const entry of (result?.diffs ?? []) as readonly WebuiWorkspaceReviewFileDiff[]) { + if (entry.error) errors[entry.fileId] = entry.error; + else if (entry.diff?.type === "text" && entry.diff.content) diffs[entry.fileId] = entry.diff.content; + else if (entry.diff?.type === "binary") errors[entry.fileId] = "二进制文件没有可显示的补丁"; + else errors[entry.fileId] = "运行时没有返回这个文件的补丁"; + } + dispatch({ type: "diffs-loaded", diffs, errors }); + } catch (error: unknown) { + dispatch({ type: "diffs-loaded", diffs: {}, errors: Object.fromEntries(batch.map((fileId) => [fileId, error instanceof Error ? error.message : String(error)])) }); + } + } + }, [dispatch, listWorkspaceReviewFileDiffs, effectiveWorkspaceDir]); + + const runSearch = useCallback(async (reviewSnapshotId: string, query: string) => { + if (!effectiveWorkspaceDir || !searchWorkspaceReviewDiffs || !query.trim()) return; + dispatch({ type: "search-begun" }); + try { + const result = await searchWorkspaceReviewDiffs({ workspaceDir: effectiveWorkspaceDir, reviewSnapshotId, query, includeUntrackedFiles: true }); + dispatch({ type: "search-settled", fileIds: (result?.matchedFiles ?? []).map((entry) => entry.fileId) }); + } catch { + // A failed search must not leave the list filtered by the previous one. + dispatch({ type: "search-settled", fileIds: [] }); + } + }, [dispatch, searchWorkspaceReviewDiffs, effectiveWorkspaceDir]); + + const visible = selectWebuiReviewVisibleFiles(state); + const filtering = isWebuiReviewFiltering(state); + + /* The picker is the way in. It lists real checkouts, taken from the same + * grouping the worktree page uses, so one checkout cannot appear twice just + * because the runtime spelled its path two ways. */ + if (!effectiveWorkspaceDir) { + return ; + } + if (state.status === "idle" || state.status === "loading") { + return ; + } + if (state.status === "unavailable" || state.status === "error") { + return ; + } + + return { + dispatch({ type: "query-changed", query }); + // Live search: the server is asked for every keystroke, and the + // reducer drops the previous matches first so the list never shows + // results for a query that is no longer in the box. + if (state.reviewSnapshotId && query.trim()) void runSearch(state.reviewSnapshotId, query); + }} + onClearFilters={() => dispatch({ type: "clear-filters" })} + onToggleFile={(fileId, willExpand, file) => { + dispatch({ type: "toggle-file", fileId }); + if (willExpand && !state.diffs[fileId] && !state.diffErrors[fileId] && state.reviewSnapshotId) { + void loadDiffs([fileId], state.reviewSnapshotId); + } + void file; + }} + onOpenFileLine={onOpenFileLine} + />; +} + +/** Presentational half of the code-review page. + * + * Split from the fetching shell on purpose: `renderToStaticMarkup` runs the + * first render and never runs effects, so a page that only reaches its + * populated state through an effect cannot be asserted on at all. Everything + * that depends on data is decided upstream in `review-state.ts` and arrives + * here as a plain value, which makes the populated render reachable from a + * test without a DOM. */ +/* Roadmap E 区 follow-up: the review page used to answer "先打开一个工作区" + * and stop. The instruction named the problem and gave no way out of it — and + * the settings dialog is `aria-modal`, so the rail cannot be clicked while it + * is open. Choosing here is the only route that stays inside the page. + * + * Presentational like `WebuiReviewPanel`, so the list is assertable without a + * DOM. It deliberately reuses the worktree panel's class names instead of + * inventing a second look for what is the same thing: a list of checkouts. */ +export function WebuiReviewWorkspacePicker({ workspaces, selected, loading, error, onSelect }: { + readonly workspaces: readonly WebuiWorktreeWorkspace[]; + readonly selected?: string; + readonly loading?: boolean; + readonly error?: string; + readonly onSelect?: (workspaceDir: string) => void; +}): ReactElement { + const state = error ? "error" : loading ? "loading" : workspaces.length ? "ready" : "empty"; + return
+

选一个工作区来审查它的变更。

+ {error ?

{error}

: null} + {loading ?

正在读取可审查的工作区…

: null} + {!loading && !error && workspaces.length === 0 ? ( + /* Saying why matters more than saying that: an empty list under the same + * heading reads as the bug this page is fixing. */ +

现在没有绑定项目目录的会话，所以没有可审查的工作区。在左栏给一个会话选好项目目录，再回到这里。

+ ) : null} +
    + {workspaces.map((workspace) =>
  • + +

    {workspace.workspaceDir}

    +
    • )} +
+
; +} + +export function WebuiReviewPanel({ state, visible, filtering, loading, note, empty, workspaceDir, onQueryChange, onClearFilters, onToggleFile, onOpenFileLine }: { + readonly state: WebuiReviewState; + readonly visible?: readonly WebuiReviewFile[]; + readonly filtering?: boolean; + readonly loading?: boolean; + readonly note?: string; + readonly empty?: string; + readonly workspaceDir?: string; + readonly onQueryChange?: (query: string) => void; + readonly onClearFilters?: () => void; + readonly onToggleFile?: (fileId: string, willExpand: boolean, file: WebuiReviewFile) => void; + readonly onOpenFileLine?: (path: string, line: number) => void; +}): ReactElement { + const rows = visible ?? state.files; + const filterActive = filtering ?? isWebuiReviewFiltering(state); + return
+ {state.status === "ready" ?
+ {state.totals.files} 个文件 + {`+${state.totals.additions}`} + {`-${state.totals.deletions}`} +
: null} + {state.status === "ready" ?
+ + onQueryChange?.(event.target.value)} + /> + {filterActive ? : null} +
: null} + {loading || state.searchPending ?

{loading ? "正在读取待审查的变更…" : "正在搜索…"}

: null} + {note || state.error ?

{note ?? state.error}

: null} + {workspaceDir ?

{workspaceDir}

: null} + {empty && !note ?

{empty}

: null} + {state.status === "ready" && rows.length === 0 ?

{filterActive ? "没有匹配的文件" : "没有可显示的文件"}

: null} +
    + {rows.map((file) => { + const expanded = state.expandedFileIds.includes(file.fileId); + const diff = state.diffs[file.fileId]; + const diffError = state.diffErrors[file.fileId]; + const pending = state.loadingFileIds.includes(file.fileId); + return
  • + + {expanded ?
    + {pending ?

    正在读取这个文件的补丁…

    : null} + {diffError ?

    {diffError}

    : null} + {diff ?
      + {projectWebuiReviewLines(diff).map((line, index) => { + const target = webuiReviewLineTarget(line); + const text = ( + <> + {line.newLine ?? ""} + {line.text} + + ); + return
    1. + {target === undefined || !onOpenFileLine + ? {text} + : } +
      2. ; + })} +
    : null} +
    : null} +
    • ; + })} +
+
; +} + +/** Roadmap E 区「工作树隔离」. + * + * Creating an isolated worktree already works from the rail's context menu; + * this is the other half of the row — being able to see which parallel + * experiment branches exist and switch into one. Grouping is done by + * `groupWebuiWorktreeWorkspaces`, so the list a test asserts on is the same + * list the page renders. */ +function SettingsWorktreePage({ loadSessions }: { + readonly loadSessions?: WebuiSettingsModalCapabilities["loadSessions"]; +}): ReactElement { + const [status, setStatus] = useState<"loading" | "ready" | "error">("loading"); + const [sessions, setSessions] = useState([]); + const [error, setError] = useState(); + + useEffect(() => { + if (!loadSessions) return; + let cancelled = false; + setStatus("loading"); + void loadSessions() + .then((page) => { + if (cancelled) return; + setSessions((page?.sessions ?? []) as readonly WebuiWorktreeSourceSession[]); + setStatus("ready"); + }) + .catch((cause: unknown) => { + if (cancelled) return; + setError(cause instanceof Error ? cause.message : String(cause)); + setStatus("error"); + }); + return () => { cancelled = true; }; + }, [loadSessions]); + + const workspaces = groupWebuiWorktreeWorkspaces(sessions); + const worktrees = selectWebuiWorktreeWorkspaces(workspaces); + const primary = selectWebuiPrimaryWorkspace(workspaces); + + if (status === "error") { + return ; + } + if (status === "loading") { + return
+

正在读取工作树…

+
; + } + return ; +} + +/** Presentational half of the worktree page, split for the same reason as + * `WebuiReviewPanel`: `renderToStaticMarkup` runs no effects, so a page that + * only reaches its populated state through a fetch cannot be asserted on. */ +export function WebuiWorktreePanel({ workspaces, worktrees, primary, error }: { + readonly workspaces: readonly WebuiWorktreeWorkspace[]; + readonly worktrees?: readonly WebuiWorktreeWorkspace[]; + readonly primary?: WebuiWorktreeWorkspace; + readonly error?: string; +}): ReactElement { + const branches = worktrees ?? selectWebuiWorktreeWorkspaces(workspaces); + const main = primary ?? selectWebuiPrimaryWorkspace(workspaces); + return
+ {error ?

{error}

: null} + {main ?
+
+ {main.name} + 主检出 +
+

{main.workspaceDir}

+
    + {main.sessions.map((entry) =>
  • + {entry.title} +
    • )} +
+
: null} +

并行实验分支

+ {branches.length === 0 ?

还没有工作树。在会话的右键菜单里选「复制到新工作树」即可开一个。

: null} +
    + {branches.map((branch) =>
  • +
    + {branch.name} + {branch.sessions.length} 个会话 +
    +

    {branch.workspaceDir}

    +
      + {branch.sessions.map((entry) =>
    • + {/* Session navigation in this shell is the `#session=` hash, + which is what the rail's own links use. Reusing it keeps one + navigation path instead of adding a second one. */} + {entry.title} + {entry.parentSessionId ? 派生 : null} +
      • )} +
    +
    • )} +
+
; +} + function ArchivedSessionsPage({ sessions, canDelete, canUnarchive, onDelete, onUnarchive }: { readonly sessions: readonly WebuiSessionListItem[]; readonly canDelete: boolean; readonly canUnarchive: boolean; readonly onDelete: (id: string) => Promise; readonly onUnarchive: (id: string) => Promise }): ReactElement { const [search, setSearch] = useState(""); const [actionError, setActionError] = useState(""); diff --git a/packages/webui/src/client/components/UserMenu.tsx b/packages/webui/src/client/components/UserMenu.tsx index 5fd31f90..1739cd07 100644 --- a/packages/webui/src/client/components/UserMenu.tsx +++ b/packages/webui/src/client/components/UserMenu.tsx @@ -37,6 +37,12 @@ interface UserMenuProps { readonly dataDir?: string; readonly version?: WebuiVersionInfo; readonly sessionId?: string; + /** Forwarded to the settings modal so the code-review page knows which + * workspace to read a change set from. */ + readonly workspaceDir?: string; + /** Forwarded to the settings modal. The review page turns a diff line into + * a real editor jump through it. */ + readonly onOpenFileLine?: (path: string, line: number) => void; /** Capability source for the menu's own panels and the settings modal. * Typed as the narrow 9-member contract so neither the menu nor the * modal can accidentally start reading members they do not consume. */ @@ -685,6 +691,8 @@ export function UserMenu({ dataDir, sessionId, version, + workspaceDir, + onOpenFileLine, transport, getSigninPanel, claimSignin, @@ -860,6 +868,6 @@ export function UserMenu({ : null} - {typeof document !== "undefined" ? createPortal( setSettingsOpen(false)} dataDir={dataDir} version={version} sessionId={sessionId} transport={transport} />, document.body) : setSettingsOpen(false)} dataDir={dataDir} version={version} sessionId={sessionId} transport={transport} />} + {typeof document !== "undefined" ? createPortal( setSettingsOpen(false)} dataDir={dataDir} version={version} sessionId={sessionId} workspaceDir={workspaceDir} onOpenFileLine={onOpenFileLine} transport={transport} />, document.body) : setSettingsOpen(false)} dataDir={dataDir} version={version} sessionId={sessionId} workspaceDir={workspaceDir} onOpenFileLine={onOpenFileLine} transport={transport} />} ; } diff --git a/packages/webui/src/client/components/WebuiClientFoundationApp.tsx b/packages/webui/src/client/components/WebuiClientFoundationApp.tsx index 445881ef..dfb714fb 100644 --- a/packages/webui/src/client/components/WebuiClientFoundationApp.tsx +++ b/packages/webui/src/client/components/WebuiClientFoundationApp.tsx @@ -1232,6 +1232,15 @@ export function WebuiClientFoundationApp( dataDir={dataDir} version={runtimeVersion} sessionId={selectedSessionId} + workspaceDir={selectedSession?.workspaceDir} + onOpenFileLine={(path, line) => { + // The open-file command needs a concrete session and + // workspace; the review page is only reachable from a + // selected session, so both are present in practice, but + // a jump must never fire a command with holes in it. + if (!selectedSessionId || !selectedSession?.workspaceDir) return; + dispatchWorkspacePanel({ type: "open-file", sessionId: selectedSessionId, workspaceDir: selectedSession.workspaceDir, path, lineStart: line, lineEnd: line }); + }} transport={transport} getSigninPanel={transport?.getSigninPanel} claimSignin={transport?.claimSignin} diff --git a/packages/webui/src/client/contracts.ts b/packages/webui/src/client/contracts.ts index 6cefd9ee..119ae581 100644 --- a/packages/webui/src/client/contracts.ts +++ b/packages/webui/src/client/contracts.ts @@ -412,6 +412,16 @@ export interface WebuiDiffState { readonly busy: boolean; readonly expanded: boolean; readonly reviewing: boolean; + /** Why the last revert/reapply did not apply, when it did not apply. + * + * Deliberately NOT the same thing as `unsupported`: that flag means the + * runtime never offered the capability, this one means the runtime answered + * and the operation did not take effect. Collapsing the two made every + * failure read as "当前运行时未提供 session diff 能力", which is both + * untrue and, because `buildWebuiDiffMutationRequest` refuses every request + * once `unsupported` is set and nothing ever resets it, permanently + * unrecoverable. */ + readonly mutationError?: string; } export type WebuiDiffStateAction = @@ -419,7 +429,8 @@ export type WebuiDiffStateAction = | { readonly type: "unsupported" } | { readonly type: "begin-mutation" } | { readonly type: "mutation-succeeded"; readonly view: WebuiTurnDiffView } - | { readonly type: "mutation-failed" } + | { readonly type: "mutation-failed"; readonly error?: string } + | { readonly type: "dismiss-mutation-error" } | { readonly type: "toggle-expanded" } | { readonly type: "toggle-review" }; diff --git a/packages/webui/src/client/projection/review-state.ts b/packages/webui/src/client/projection/review-state.ts new file mode 100644 index 00000000..06071e67 --- /dev/null +++ b/packages/webui/src/client/projection/review-state.ts @@ -0,0 +1,273 @@ +/** Pure state for the WebUI code-review surface. + * + * The workspace review capability (getWorkspaceReviewSummary / + * listWorkspaceReviewFileDiffs / searchWorkspaceReviewDiffs) is already wired + * end to end — the diff card's Review button dispatches into the workspace + * panel. What was missing was a place to read a whole change set, which is + * what roadmap E 区's "Review 审查模式" row asks for. + * + * Everything that can be decided without the network lives here so it can be + * tested directly: the webui suite runs `environment: "node"` with no jsdom, + * so the component itself is only reachable through `renderToStaticMarkup`, + * which runs no effects and fires no events. + */ + +export interface WebuiReviewFile { + readonly fileId: string; + readonly path: string; + readonly originalPath?: string; + readonly status: string; + readonly type?: "text" | "binary"; + readonly additions: number; + readonly deletions: number; +} + +export interface WebuiReviewTotals { + readonly files: number; + readonly additions: number; + readonly deletions: number; +} + +/** One rendered diff line, carrying the new-side line number so a click can + * be turned into a real "open this file at this line" request. */ +export interface WebuiReviewLine { + readonly kind: "context" | "addition" | "deletion" | "hunk" | "meta"; + readonly text: string; + /** 1-based line number on the new side, when this line exists there. */ + readonly newLine?: number; + /** 1-based line number on the old side, when this line exists there. */ + readonly oldLine?: number; +} + +export type WebuiReviewStatus = "idle" | "loading" | "ready" | "unavailable" | "error"; + +export interface WebuiReviewState { + readonly status: WebuiReviewStatus; + readonly files: readonly WebuiReviewFile[]; + readonly totals: WebuiReviewTotals; + readonly reviewSnapshotId?: string; + /** Unified diff text per fileId. Absent means "not loaded yet". */ + readonly diffs: Readonly>; + /** Per-file failure text, kept apart from the diff so a file that failed to + * load never reads as a file with no changes. */ + readonly diffErrors: Readonly>; + readonly loadingFileIds: readonly string[]; + readonly expandedFileIds: readonly string[]; + readonly query: string; + /** Non-empty only while a search has run. An empty array means "no match", + * which is why the query these matches belong to is kept alongside them: + * before a search has run for the current query the list must stay + * unfiltered, and after one that matched nothing it must be empty. */ + readonly searchedQuery: string; + readonly matchedFileIds: readonly string[]; + readonly searchPending: boolean; + readonly error?: string; +} + +export const initialWebuiReviewState: WebuiReviewState = { + status: "idle", + files: [], + totals: { files: 0, additions: 0, deletions: 0 }, + diffs: {}, + diffErrors: {}, + loadingFileIds: [], + expandedFileIds: [], + query: "", + searchedQuery: "", + matchedFileIds: [], + searchPending: false, +}; + +export type WebuiReviewStateAction = + | { readonly type: "load-begun" } + | { + readonly type: "summary-loaded"; + readonly reviewSnapshotId: string; + readonly files: readonly WebuiReviewFile[]; + readonly totals: WebuiReviewTotals; + } + | { readonly type: "unavailable"; readonly reason: string } + | { readonly type: "load-failed"; readonly reason: string } + | { readonly type: "diffs-begun"; readonly fileIds: readonly string[] } + | { + readonly type: "diffs-loaded"; + readonly diffs: Readonly>; + readonly errors?: Readonly>; + } + | { readonly type: "toggle-file"; readonly fileId: string } + | { readonly type: "query-changed"; readonly query: string } + | { readonly type: "search-begun" } + | { readonly type: "search-settled"; readonly fileIds: readonly string[] } + | { readonly type: "clear-filters" }; + +const withoutKeys = (source: Readonly>, keys: readonly string[]): Record => { + const next: Record = { ...source }; + for (const key of keys) delete next[key]; + return next; +}; + +const toggle = (list: readonly string[], value: string): readonly string[] => + list.includes(value) ? list.filter((entry) => entry !== value) : [...list, value]; + +export function reduceWebuiReviewState( + state: WebuiReviewState, + action: WebuiReviewStateAction, +): WebuiReviewState { + switch (action.type) { + case "load-begun": + // A reload must not leave the previous snapshot's diffs on screen + // against a new reviewSnapshotId: those file ids belong to another + // snapshot and would render as this one's changes. + return { + ...state, + status: "loading", + error: undefined, + diffs: {}, + diffErrors: {}, + loadingFileIds: [], + matchedFileIds: [], + searchedQuery: "", + searchPending: false, + }; + case "summary-loaded": + return { + ...state, + status: "ready", + error: undefined, + reviewSnapshotId: action.reviewSnapshotId, + files: action.files, + totals: action.totals, + }; + case "unavailable": + // Distinct from "error": the runtime has no review snapshot for this + // workspace, which is a normal state, not a failure to report as one. + return { ...state, status: "unavailable", error: undefined, searchPending: false }; + case "load-failed": + return { ...state, status: "error", error: action.reason, searchPending: false }; + case "diffs-begun": + return { ...state, loadingFileIds: [...new Set([...state.loadingFileIds, ...action.fileIds])] }; + case "diffs-loaded": { + const errors = action.errors ?? {}; + // A file leaves the in-flight list when this response carried it, either + // as a diff or as an error. Deriving the settled set from the incoming + // payload rather than from what is already in state is what makes the + // marker clear: a freshly loaded file is not in `state.diffs` yet, so + // reading only the existing state would leave it spinning forever. + const settled = [...new Set([...Object.keys(action.diffs), ...Object.keys(errors)])].filter( + (fileId) => state.loadingFileIds.includes(fileId), + ); + return { + ...state, + diffs: { ...withoutKeys(state.diffs, settled), ...action.diffs }, + diffErrors: { ...withoutKeys(state.diffErrors, settled), ...errors }, + loadingFileIds: state.loadingFileIds.filter((fileId) => !settled.includes(fileId)), + }; + } + case "toggle-file": + return { ...state, expandedFileIds: toggle(state.expandedFileIds, action.fileId) }; + case "query-changed": + // Editing the query invalidates the previous result set immediately. + // Keeping it would filter the list by a query that no longer matches. + return { ...state, query: action.query, matchedFileIds: [], searchedQuery: "", searchPending: false }; + case "search-begun": + return { ...state, searchPending: true }; + case "search-settled": + return { + ...state, + searchPending: false, + searchedQuery: state.query, + matchedFileIds: action.fileIds, + }; + case "clear-filters": + return { ...state, query: "", searchedQuery: "", matchedFileIds: [], searchPending: false }; + } +} + +/** The file list the page should render, honouring an active search filter. + * + * "No query yet", "query not searched yet" and "query matched nothing" all + * produce different lists, so the filter is only applied once a search has + * actually settled for the query currently in the box. */ +export function selectWebuiReviewVisibleFiles(state: WebuiReviewState): readonly WebuiReviewFile[] { + const query = state.query.trim(); + if (!query || state.searchedQuery !== state.query) return state.files; + const matched = new Set(state.matchedFileIds); + return state.files.filter((file) => matched.has(file.fileId)); +} + +export function isWebuiReviewFiltering(state: WebuiReviewState): boolean { + return state.query.trim().length > 0; +} + +/** Splits a unified diff into renderable lines, numbering both sides. + * + * The new-side number is what makes "诊断 → 建议 → 行内定位" possible: a click + * on an added line has to resolve to a real line in the file the editor will + * open, and only the `+` side survives into the working tree. Hunk headers + * carry no line of their own, so they are reported as `hunk` with no number + * rather than inheriting the previous line's. */ +export function projectWebuiReviewLines(unifiedDiff: string): readonly WebuiReviewLine[] { + const lines = unifiedDiff.split(/\r?\n/u); + const projected: WebuiReviewLine[] = []; + let oldLine = 0; + let newLine = 0; + let inHunk = false; + for (const raw of lines) { + if (raw.startsWith("@@")) { + const header = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/u.exec(raw); + oldLine = header ? Number(header[1]) : 0; + newLine = header ? Number(header[2]) : 0; + inHunk = true; + projected.push({ kind: "hunk", text: raw }); + continue; + } + if (raw.startsWith("diff ") || raw.startsWith("index ") || raw.startsWith("--- ") || raw.startsWith("+++ ")) { + projected.push({ kind: "meta", text: raw }); + continue; + } + if (!inHunk) { + // Content before the first hunk header is file metadata, not a line of + // either side; numbering it would point the editor at the wrong place. + projected.push({ kind: "meta", text: raw }); + continue; + } + if (raw.startsWith("+")) { + projected.push({ kind: "addition", text: raw.slice(1), newLine }); + newLine += 1; + continue; + } + if (raw.startsWith("-")) { + projected.push({ kind: "deletion", text: raw.slice(1), oldLine }); + oldLine += 1; + continue; + } + if (raw.startsWith("\\")) { + // "\ No newline at end of file" belongs to the preceding line and must + // not consume a line number. + projected.push({ kind: "meta", text: raw }); + continue; + } + projected.push({ kind: "context", text: raw.slice(1), newLine, oldLine }); + newLine += 1; + oldLine += 1; + } + return projected; +} + +/** The line an editor jump should target, if this line can have one. + * + * A deleted line is not in the file the editor will open, and a hunk header + * or a file-metadata row is not a line of the file at all. Rather than + * re-check the kinds here, this reads the projection's own invariant: only + * `addition` and `context` rows are ever given a `newLine`, so a row without + * one has no destination. That invariant is asserted directly in + * `review-state.test.ts` — putting the check in a guard instead made it + * untestable, since removing the guard could not change the result. */ +export function webuiReviewLineTarget(line: WebuiReviewLine): number | undefined { + return line.newLine; +} + +/** Batch size for listWorkspaceReviewFileDiffs. The server takes a fileId + * array per call; the workspace panel already batches at five and the same + * batcher is reused here so both surfaces behave identically. */ +export const WEBUI_REVIEW_DIFF_BATCH_SIZE = 5; diff --git a/packages/webui/src/client/projection/worktree-state.ts b/packages/webui/src/client/projection/worktree-state.ts new file mode 100644 index 00000000..ae03b92b --- /dev/null +++ b/packages/webui/src/client/projection/worktree-state.ts @@ -0,0 +1,128 @@ +/** Pure grouping for the WebUI worktree surface (roadmap E 区「工作树隔离」). + * + * Creating an isolated worktree already works — the rail's context menu + * carries 「复制到新工作树」 and the server answers with + * `worktreeVisible` / `worktreeUnavailableReason`. What did not exist was a + * place to see what those worktrees are, which is the "并行实验分支" half of + * the row. + * + * A worktree is not a first-class field on a session. It is inferred: git + * creates a second checkout with its own absolute path, so a session whose + * `workspaceDir` differs from the project's default checkout is running in + * one. `isDefaultWorkspace` is the runtime's own statement of which checkout + * is the primary one, and it is trusted over any guess made here. + * + * Grouping is a pure function so it can be asserted without a DOM: the page + * that renders it runs `renderToStaticMarkup`, which never runs effects. + */ + +export interface WebuiWorktreeSession { + readonly sessionId: string; + readonly title: string; + readonly updatedAt: number; + readonly parentSessionId?: string; +} + +export interface WebuiWorktreeWorkspace { + readonly workspaceDir: string; + readonly name: string; + /** True for the project's own checkout. A worktree is a non-primary one. */ + readonly isPrimary: boolean; + readonly sessions: readonly WebuiWorktreeSession[]; + readonly updatedAt: number; +} + +export interface WebuiWorktreeSourceSession { + readonly sessionId: string; + readonly title?: string; + readonly archived?: boolean; + readonly updatedAt: number; + readonly workspaceDir?: string; + readonly isDefaultWorkspace?: boolean; + readonly parentSessionId?: string; +} + +/** Windows and POSIX spell the same checkout differently in stored paths + * (`C:\repo` vs `C:/repo`). Normalising the separator keeps one worktree from + * being reported twice because a fork round-tripped the path. */ +export function normalizeWebuiWorkspaceDir(workspaceDir: string): string { + return workspaceDir.trim().replace(/\\/gu, "/").replace(/\/+$/u, "").toLowerCase(); +} + +export function webuiWorkspaceName(workspaceDir: string): string { + const normalized = workspaceDir.trim().replace(/[\\/]+$/u, ""); + if (!normalized) return normalized; + const segments = normalized.split(/[\\/]/u).filter(Boolean); + return segments.at(-1) ?? normalized; +} + +/** Groups sessions into checkouts, most recently touched first. + * + * Archived sessions are dropped: the archived page already owns them, and a + * worktree whose every session is archived is not a parallel experiment + * branch anyone is using. Sessions with no `workspaceDir` are dropped too — + * there is no checkout to attach them to, and inventing a bucket for them + * would show up as a phantom worktree. */ +export function groupWebuiWorktreeWorkspaces( + sessions: readonly WebuiWorktreeSourceSession[], +): readonly WebuiWorktreeWorkspace[] { + const groups = new Map(); + for (const session of sessions) { + if (session.archived) continue; + const raw = session.workspaceDir?.trim(); + if (!raw) continue; + const key = normalizeWebuiWorkspaceDir(raw); + if (!key) continue; + const existing = groups.get(key); + const entry: WebuiWorktreeSession = { + sessionId: session.sessionId, + title: session.title?.trim() || session.sessionId, + updatedAt: session.updatedAt, + ...(session.parentSessionId ? { parentSessionId: session.parentSessionId } : {}), + }; + if (existing) { + existing.sessions.push(entry); + // A checkout only counts as primary if the runtime said so. A later + // session without the flag must not demote one that has it. + existing.isPrimary = existing.isPrimary || session.isDefaultWorkspace === true; + existing.updatedAt = Math.max(existing.updatedAt, session.updatedAt); + continue; + } + groups.set(key, { + workspaceDir: raw, + isPrimary: session.isDefaultWorkspace === true, + sessions: [entry], + updatedAt: session.updatedAt, + }); + } + const workspaces = [...groups.values()].map((group) => ({ + workspaceDir: group.workspaceDir, + name: webuiWorkspaceName(group.workspaceDir), + isPrimary: group.isPrimary, + sessions: [...group.sessions].sort((a, b) => b.updatedAt - a.updatedAt), + updatedAt: group.updatedAt, + })); + // Primary first, then most recently touched. The tie-break on path keeps the + // order stable when two checkouts share a timestamp, which a fast test + // fixture almost always does. + return workspaces.sort((a, b) => { + if (a.isPrimary !== b.isPrimary) return a.isPrimary ? -1 : 1; + if (a.updatedAt !== b.updatedAt) return b.updatedAt - a.updatedAt; + return a.workspaceDir.localeCompare(b.workspaceDir); + }); +} + +/** The parallel experiment branches: every checkout that is not the project's + * own. This is the list the page is named after, so it is kept separate from + * the grouping function rather than re-derived by the component. */ +export function selectWebuiWorktreeWorkspaces( + workspaces: readonly WebuiWorktreeWorkspace[], +): readonly WebuiWorktreeWorkspace[] { + return workspaces.filter((workspace) => !workspace.isPrimary); +} + +export function selectWebuiPrimaryWorkspace( + workspaces: readonly WebuiWorktreeWorkspace[], +): WebuiWorktreeWorkspace | undefined { + return workspaces.find((workspace) => workspace.isPrimary); +} diff --git a/packages/webui/src/client/styles/shell.css b/packages/webui/src/client/styles/shell.css index d264c5b4..0547612e 100644 --- a/packages/webui/src/client/styles/shell.css +++ b/packages/webui/src/client/styles/shell.css @@ -252,6 +252,52 @@ .webui-archived-delete-all { display: inline-flex; height: 36px; align-items: center; gap: 6px; border: 0; border-radius: 12px; padding: 0 12px; background: rgba(255, 59, 48, .1); color: #e5484d; font: inherit; font-size: var(--size_14); cursor: pointer; } .webui-archived-delete-all:hover { background: rgba(255, 59, 48, .16); } .webui-archived-delete-all:disabled { opacity: .45; cursor: default; } + + /* Code review (roadmap E 区 Review 审查模式 / 修复建议+跳转). */ + .webui-review-page { width: 100%; max-width: 760px; margin: 0 auto; padding: 0 var(--spacing_20) var(--spacing_32); } + .webui-review-totals { display: flex; align-items: center; gap: var(--spacing_8); margin-bottom: var(--spacing_12); color: var(--text_default_secondary); font-size: var(--size_12); } + .webui-review-search { display: flex; min-width: 0; height: 38px; align-items: center; gap: var(--spacing_8); margin-bottom: var(--spacing_12); border: 1px solid var(--border_default); border-radius: 12px; padding: 0 12px; background: var(--bg_default_primary); color: var(--text_default_secondary); } + .webui-review-search:focus-within { border-color: var(--border_accent); } + .webui-review-search input { width: 100%; min-width: 0; border: 0; outline: 0; background: transparent; color: var(--text_default_primary); font: inherit; font-size: var(--size_14); } + .webui-review-clear { flex: none; border: 0; background: transparent; color: var(--text_default_secondary); font-size: var(--size_12); } + .webui-review-files { display: flex; flex-direction: column; gap: var(--spacing_4); margin: 0; padding: 0; list-style: none; } + .webui-review-file { border: 0.5px solid var(--border_default); border-radius: 10px; overflow: hidden; } + .webui-review-file-heading { display: flex; width: 100%; min-width: 0; align-items: center; gap: var(--spacing_8); padding: 10px 12px; border: 0; background: transparent; color: inherit; font: inherit; text-align: left; } + .webui-review-file-heading:hover { background: var(--bg_interaction_secondary_default); } + .webui-review-file-path { min-width: 0; flex: 1; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; color: var(--text_default_primary); font-size: var(--size_12); } + .webui-review-file-status { flex: none; color: var(--text_default_tertiary); font-size: var(--size_12); } + .webui-review-file-stats { display: inline-flex; flex: none; gap: 4px; font-size: var(--size_12); } + .webui-review-file-body { border-top: 0.5px solid var(--border_default); background: var(--bg_default_primary); } + .webui-review-file-error { margin: 0; padding: 10px 12px; color: var(--text_label_danger_secondary_default); font-size: var(--size_12); } + .webui-review-lines { max-height: 420px; margin: 0; padding: 0; overflow: auto; list-style: none; font-family: var(--font-mono, monospace); font-size: var(--size_12); line-height: 1.6; } + .webui-review-line { display: flex; min-width: max-content; } + .webui-review-line--addition { background: var(--bg_status_positive); color: #17251d; } + .webui-review-line--deletion { background: var(--bg_status_error); color: #302022; } + .webui-review-line--hunk { background: var(--bg_interaction_secondary_default); color: var(--text_default_tertiary); } + .webui-review-line--meta { color: var(--text_default_tertiary); } + .webui-review-line-number { flex: none; width: 48px; padding-right: 8px; opacity: .6; text-align: right; user-select: none; } + .webui-review-line-text { flex: 1; padding-right: 12px; white-space: pre; } + /* Only a line that survives into the working tree can be jumped to, so the + clickable surface is deliberately absent on deletions. */ + .webui-review-line-jump { display: flex; min-width: max-content; width: 100%; border: 0; padding: 0; background: transparent; color: inherit; font: inherit; text-align: left; cursor: pointer; } + .webui-review-line-jump:hover { text-decoration: underline; } + .webui-review-line-static { display: flex; min-width: max-content; width: 100%; } + .webui-review-loading, .webui-review-empty { margin: 0; padding: var(--spacing_16) 0; color: var(--text_default_secondary); font-size: var(--size_12); } + .webui-review-error { margin: 0 0 var(--spacing_8); color: var(--text_label_danger_secondary_default); font-size: var(--size_12); } + + /* Worktree list (roadmap E 区 工作树隔离). */ + .webui-worktree-section-title { margin: var(--spacing_20) 0 var(--spacing_8); color: var(--text_default_primary); font-size: var(--size_14); } + .webui-worktree-branches { display: flex; flex-direction: column; gap: var(--spacing_8); margin: 0; padding: 0; list-style: none; } + .webui-worktree-group { padding: 10px 12px; border: 0.5px solid var(--border_default); border-radius: 10px; } + .webui-worktree-heading { display: flex; align-items: center; gap: var(--spacing_8); } + .webui-worktree-name { display: inline-flex; min-width: 0; flex: 1; align-items: center; gap: 6px; overflow: hidden; color: var(--text_default_primary); font-size: var(--size_14); } + .webui-worktree-kind { flex: none; color: var(--text_default_tertiary); font-size: var(--size_12); } + .webui-worktree-path { margin: 4px 0 0; overflow: hidden; color: var(--text_default_tertiary); font-size: var(--size_12); text-overflow: ellipsis; white-space: nowrap; } + .webui-worktree-sessions { display: flex; flex-direction: column; gap: 2px; margin: var(--spacing_8) 0 0; padding: 0; list-style: none; } + .webui-worktree-session { display: flex; min-width: 0; align-items: center; gap: var(--spacing_8); font-size: var(--size_12); } + .webui-worktree-session-link { min-width: 0; flex: 1; overflow: hidden; color: var(--text_default_secondary); text-decoration: none; text-overflow: ellipsis; white-space: nowrap; } + .webui-worktree-session-link:hover { color: var(--text_default_primary); text-decoration: underline; } + .webui-worktree-forked { flex: none; color: var(--text_default_tertiary); } .webui-archived-page { width: 100%; max-width: 760px; margin: 0 auto; padding: 0 var(--spacing_20) var(--spacing_32); } .webui-archived-filters { display: grid; grid-template-columns: minmax(0, 1fr); margin-bottom: var(--spacing_24); } .webui-archived-search { display: flex; min-width: 0; height: 38px; align-items: center; gap: var(--spacing_8); border: 1px solid var(--border_default); border-radius: 12px; padding: 0 12px; background: var(--bg_default_primary); color: var(--text_default_secondary); } @@ -4255,6 +4301,52 @@ opacity: 0.5; } + /* A revert/reapply that did not apply. Deliberately a footnote on the live + card rather than a replacement for it: the file list above is stale, but + hiding the card would also hide the control that retries. */ + .webui-diff-error { + display: flex; + align-items: flex-start; + gap: 12px; + padding: 8px 12px; + border-top: 0.5px solid var(--border_default); + background: var(--bg_interaction_secondary_default); + font-size: 12px; + } + + .webui-diff-error-copy { + display: flex; + flex: 1; + min-width: 0; + flex-direction: column; + gap: 2px; + } + + .webui-diff-error-title { + color: var(--text_default_primary); + } + + /* The reason is whatever the runtime reported, so it can be long and will + often be a path; wrapping it keeps the dismiss control on the card. */ + .webui-diff-error-reason { + color: var(--text_label_danger_secondary_default); + overflow-wrap: anywhere; + } + + .webui-diff-error-dismiss { + flex: none; + height: 24px; + padding: 0 8px; + border: 0.5px solid var(--border_default); + border-radius: 4px; + background: transparent; + color: var(--text_default_secondary); + } + + .webui-diff-error-dismiss:hover { + background: var(--bg_interaction_tertiary_hover); + } + /* ===== Phase 5: message actions, goal banner, and questionnaire states ===== */ .webui-message-actions { display: flex; diff --git a/packages/webui/test/unit/diff-mutation-error.test.tsx b/packages/webui/test/unit/diff-mutation-error.test.tsx new file mode 100644 index 00000000..e0f5aba6 --- /dev/null +++ b/packages/webui/test/unit/diff-mutation-error.test.tsx @@ -0,0 +1,339 @@ +import { createElement } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; + +import { + WebuiDiffCard, + buildWebuiDiffMutationRequest, + describeWebuiDiffFailure, + initialWebuiDiffState, + reduceWebuiDiffState, + resolveWebuiDiffMutation, +} from "../../src/client/components/DiffCard.js"; +import type { WebuiTurnDiffView } from "../../src/server/port.js"; + +/* Why these tests are about the shape of a FAILED mutation, not about markup: + * + * `renderToStaticMarkup` does not run effects and cannot fire a click, so + * every assertion here is deliberately aimed at a pure function or at a + * state object the component accepts through `initialState`. The event path + * itself (`mutate` in DiffCard.tsx) is therefore not covered by this file — + * it is covered indirectly, because the whole decision of what a transport + * result means is delegated to `resolveWebuiDiffMutation` and the rendering + * of a failure is delegated to `reduceWebuiDiffState` + the JSX below. If + * someone re-inlines that decision into the component, these tests keep + * passing while the component rots; the browser suite is what actually + * drives the click. That split is deliberate, and it is why the negative + * injections recorded in the PR body target these two seams. + */ + +const files = [ + { file: "one.ts", additions: 2, deletions: 1 }, + { file: "two.ts", additions: 3, deletions: 0 }, +]; + +const activeView: WebuiTurnDiffView = { + status: "active", + changeSetId: "changes-1", + sourceMessageId: "assistant-1", + canUndo: true, + canReapply: false, + fileChanges: files, +}; + +const revertedView: WebuiTurnDiffView = { + ...activeView, + status: "reverted", + canUndo: false, + canReapply: true, +}; + +const loaded = (view: WebuiTurnDiffView = activeView) => + reduceWebuiDiffState(initialWebuiDiffState, { type: "loaded", view }); + +describe("a failed revert/reapply is reported, not relabelled as a missing capability", () => { + /* The defect this file exists for: `mutation-failed` used to reduce to + * `unsupported: true`, which renders "当前运行时未提供 session diff 能力。" + * The runtime DID answer — it declined the operation. The card then also + * became permanently dead, because `buildWebuiDiffMutationRequest` refuses + * every request once `unsupported` is set and nothing ever resets it. */ + it("does not claim the runtime lacks the capability when the operation merely failed", () => { + const failed = reduceWebuiDiffState(loaded(), { + type: "mutation-failed", + error: "plan-not-safe: 目标路径在工作区之外", + }); + + expect(failed.unsupported).toBe(false); + expect(failed.mutationError).toBe("plan-not-safe: 目标路径在工作区之外"); + }); + + it("keeps the card actionable so the user can retry after a failure", () => { + const failed = reduceWebuiDiffState(loaded(), { type: "mutation-failed" }); + + expect(failed.busy).toBe(false); + expect(buildWebuiDiffMutationRequest(failed, { id: "s" }, "revert")).toEqual({ + id: "s", + changeSetId: "changes-1", + }); + }); + + it("renders the failure on the live card instead of the capability-missing card", () => { + const markup = renderToStaticMarkup( + createElement(WebuiDiffCard, { + initialView: activeView, + initialState: { + ...initialWebuiDiffState, + mutationError: "plan-not-safe: 目标路径在工作区之外", + }, + }), + ); + + expect(markup).toContain('data-testid="turn-diff-mutation-error"'); + expect(markup).toContain("plan-not-safe: 目标路径在工作区之外"); + // The card itself must survive: a failure is not a reason to hide the + // file list or the retry control. + expect(markup).toContain('data-testid="turn-diff-card"'); + expect(markup).toContain('data-testid="turn-diff-undo"'); + expect(markup).toContain("one.ts"); + // And it must not claim the capability is missing. + expect(markup).not.toContain("当前运行时未提供 session diff 能力"); + }); + + it("offers a dismiss control so the reason is not stuck on screen forever", () => { + const markup = renderToStaticMarkup( + createElement(WebuiDiffCard, { + initialView: activeView, + initialState: { ...initialWebuiDiffState, mutationError: "boom" }, + }), + ); + expect(markup).toContain('data-testid="turn-diff-mutation-error-dismiss"'); + + const dismissed = reduceWebuiDiffState( + { ...initialWebuiDiffState, mutationError: "boom" }, + { type: "dismiss-mutation-error" }, + ); + expect(dismissed.mutationError).toBeUndefined(); + expect(dismissed.unsupported).toBe(false); + }); + + it("clears a stale reason when a new attempt starts or succeeds", () => { + const failed = { ...initialWebuiDiffState, view: activeView, mutationError: "old failure" }; + + expect( + reduceWebuiDiffState(failed, { type: "begin-mutation" }).mutationError, + ).toBeUndefined(); + expect( + reduceWebuiDiffState(failed, { type: "mutation-succeeded", view: activeView }) + .mutationError, + ).toBeUndefined(); + expect( + reduceWebuiDiffState(failed, { type: "loaded", view: activeView }).mutationError, + ).toBeUndefined(); + }); + + /* A genuinely unavailable capability keeps its old meaning. Without this, + * the fix would be "never say unsupported again", which is a different bug. + * + * It has to start from a state that actually carries a failure reason: the + * first draft of this assertion reduced `unsupported` from a freshly loaded + * card, whose `mutationError` was already `undefined`, so it passed whether + * or not the transition cleared anything. */ + it("still reports a genuinely unavailable capability as unsupported, and drops any stale failure", () => { + const failed = reduceWebuiDiffState(loaded(), { type: "mutation-failed", error: "boom" }); + expect(failed.mutationError).toBe("boom"); + + const unsupported = reduceWebuiDiffState(failed, { type: "unsupported" }); + expect(unsupported.unsupported).toBe(true); + expect(unsupported.mutationError).toBeUndefined(); + expect(buildWebuiDiffMutationRequest(unsupported, { id: "s" }, "revert")).toBeUndefined(); + }); +}); + +describe("resolveWebuiDiffMutation reads the transport result the way the wire means it", () => { + it("treats a revert that returns a populated view as success", () => { + expect( + resolveWebuiDiffMutation("revert", { success: true, turnDiff: revertedView }), + ).toEqual({ type: "mutation-succeeded", view: revertedView }); + }); + + /* `revertTurnDiff` reports refusal as `{ success: false, error }` and no + * `turnDiff` at all. The `error` string is the only explanation the user + * can get, so it has to survive into the rendered state. */ + it("surfaces the error string the server sent when a revert is refused", () => { + expect( + resolveWebuiDiffMutation("revert", { + success: false, + error: "plan-not-safe: 目标路径在工作区之外", + }), + ).toEqual({ + type: "mutation-failed", + error: "plan-not-safe: 目标路径在工作区之外", + }); + }); + + /* `reapplyTurnDiff` answers with the view itself, so its `error` is read + * from the top level. Reading it only off the revert branch dropped the + * explanation for every failed reapply — the one case where a user who just + * clicked "重新应用" most needs to be told why nothing happened. */ + it("surfaces the error string a refused reapply sent", () => { + expect( + resolveWebuiDiffMutation("reapply", { + success: false, + error: "change set 已不在当前工作区", + ...revertedView, + }), + ).toEqual({ + type: "mutation-failed", + error: "change set 已不在当前工作区", + }); + }); + + /* An explicit `success: false` is a refusal even if a view rode along. */ + it("does not let a view override an explicit refusal", () => { + expect( + resolveWebuiDiffMutation("revert", { success: false, error: "nope", turnDiff: revertedView }), + ).toEqual({ type: "mutation-failed", error: "nope" }); + }); + + /* `success` is OPTIONAL on `WebuiRevertTurnDiffResult`, so a runtime that + * reports only `error` and omits `success` is a contract-legal payload — + * and it takes a different path than the explicit-`false` refusal above, + * which returns early. Without this case the whole reason-carrying fallback + * is untested: the negative-injection run showed two mutations of that + * fallback surviving while every other seam died. */ + it("surfaces a reason from a revert that omits success entirely", () => { + expect( + resolveWebuiDiffMutation("revert", { + error: "plan-not-safe: 目标路径在工作区之外", + }), + ).toEqual({ + type: "mutation-failed", + error: "plan-not-safe: 目标路径在工作区之外", + }); + }); + + /* The card renders `null` when `fileChanges` is empty, so a view that + * applied nothing must be read as a failure — otherwise the card silently + * disappears with no message at all. */ + it("does not read an empty view as a success", () => { + expect(resolveWebuiDiffMutation("reapply", { success: true })).toEqual({ + type: "mutation-failed", + error: undefined, + }); + expect(resolveWebuiDiffMutation("reapply", { success: true, fileChanges: [] })).toEqual({ + type: "mutation-failed", + error: undefined, + }); + expect(resolveWebuiDiffMutation("revert", { success: true, turnDiff: { changeSetId: "c" } })).toEqual({ + type: "mutation-failed", + error: undefined, + }); + }); + + it("still accepts a reapply that really did return files", () => { + const applied = { success: true, ...activeView }; + expect(resolveWebuiDiffMutation("reapply", applied)).toEqual({ + type: "mutation-succeeded", + view: applied, + }); + }); + + it("falls back to a reason that does not blame the capability", () => { + // Whitespace-only is not a reason; the state must carry no string at all + // so the banner can still render its static title without a blank line. + expect(resolveWebuiDiffMutation("revert", { success: false, error: " " })).toEqual({ + type: "mutation-failed", + error: undefined, + }); + expect(resolveWebuiDiffMutation("revert", { success: false })).toEqual({ + type: "mutation-failed", + error: undefined, + }); + }); + + /* `reapplyTurnDiff` requires `success`, so reaching the reason-carrying + * fallback with a non-empty `error` means the payload contradicts itself: + * it claims to have worked and names a reason it did not. For any + * contract-legal input this case is unreachable, which is exactly why it + * needs pinning — without it the resolver can silently start preferring the + * `success` flag and drop the reason, and no legal payload would catch it. + * This is a socket boundary; a runtime that says both is better answered + * with the reason than with silence. */ + it("prefers a stated reason over a success flag when a reapply payload contradicts itself", () => { + expect( + resolveWebuiDiffMutation("reapply", { + success: true, + error: "change set 已不在当前工作区", + ...activeView, + fileChanges: [], + }), + ).toEqual({ type: "mutation-failed", error: "change set 已不在当前工作区" }); + }); + + it("carries a thrown transport error into the failure", () => { + expect( + resolveWebuiDiffMutation("revert", undefined, new Error("WebUI request timed out after 30000ms (revertTurnDiff)")), + ).toEqual({ + type: "mutation-failed", + error: "WebUI request timed out after 30000ms (revertTurnDiff)", + }); + }); + + /* A non-Error throw is not a crash, it is a failure with no explanation. + * It must still be reported as a failure rather than crashing the card. */ + it("treats a non-Error throw as a failure without a reason", () => { + expect(resolveWebuiDiffMutation("reapply", undefined, "socket closed")).toEqual({ + type: "mutation-failed", + error: undefined, + }); + expect(resolveWebuiDiffMutation("reapply", undefined, new Error(" "))).toEqual({ + type: "mutation-failed", + error: undefined, + }); + }); +}); + +/* The runtime's reasons are machine tokens. `applyLocalTurnDiffSnapshotMutation` + * reports `unsafe_path` when `safeCapturedPath` cannot resolve a captured + * entry — which `normalizeCapturePath` does for *filtered in-workspace* paths + * (`.git/`, `node_modules/`), not for paths escaping the workspace, those are + * accepted. The check runs inside the write loop, so the run can be + * half-applied. `conflict` fires when a file changed after the turn. Both reach + * the client verbatim through `assertMutationSucceeded`, which throws with + * `body.error` as the message. Without a translation the user reads the token. */ +describe("runtime reason codes become something a person can act on", () => { + it("translates every reason the mutation path can produce", () => { + expect(describeWebuiDiffFailure("unsafe_path")).toBe("这轮改动里有文件的路径无法安全定位，操作已中断，之前处理过的文件可能已改动。"); + expect(describeWebuiDiffFailure("conflict")).toBe("这轮改动之后文件又被修改过，撤销前请先确认当前内容。"); + expect(describeWebuiDiffFailure("not_undoable")).toBe("这轮文件改动没有留下可撤销的快照。"); + expect(describeWebuiDiffFailure("Turn diff not found")).toBe("找不到这轮文件改动。"); + expect(describeWebuiDiffFailure("Only the latest turn diff can be changed")).toBe("只能撤销最近一轮的文件改动。"); + expect(describeWebuiDiffFailure("Turn diff is not undoable")).toBe("这轮文件改动没有可撤销的补丁。"); + }); + + it("no longer shows a raw token as the user-facing reason", () => { + const action = resolveWebuiDiffMutation("revert", { success: false, error: "unsafe_path" }); + expect(action).toEqual({ type: "mutation-failed", error: "这轮改动里有文件的路径无法安全定位，操作已中断，之前处理过的文件可能已改动。" }); + expect(action.type === "mutation-failed" && action.error).not.toBe("unsafe_path"); + }); + + /* `git apply` failures arrive as free-form stderr. Passing them through keeps + * a real message; replacing them with a guess would be worse than useless. */ + it("passes an unknown reason through unchanged", () => { + expect(describeWebuiDiffFailure("error: patch failed: src/one.ts:3")).toBe("error: patch failed: src/one.ts:3"); + expect(describeWebuiDiffFailure(" unsafe_path ")).toBe("这轮改动里有文件的路径无法安全定位，操作已中断，之前处理过的文件可能已改动。"); + }); + + it("still reports no reason when there is none", () => { + expect(describeWebuiDiffFailure(undefined)).toBeUndefined(); + expect(describeWebuiDiffFailure("")).toBeUndefined(); + expect(describeWebuiDiffFailure(" ")).toBeUndefined(); + }); + + it("translates a thrown unsafe_path the same way as a reported one", () => { + expect(resolveWebuiDiffMutation("revert", undefined, new Error("unsafe_path"))).toEqual({ + type: "mutation-failed", + error: "这轮改动里有文件的路径无法安全定位，操作已中断，之前处理过的文件可能已改动。", + }); + }); +}); diff --git a/packages/webui/test/unit/review-panel.test.tsx b/packages/webui/test/unit/review-panel.test.tsx new file mode 100644 index 00000000..2cfbed0a --- /dev/null +++ b/packages/webui/test/unit/review-panel.test.tsx @@ -0,0 +1,348 @@ +import { createElement } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +import { WebuiReviewPanel, WebuiReviewWorkspacePicker } from "../../src/client/components/SettingsModal.js"; +import { + initialWebuiReviewState, + reduceWebuiReviewState, + selectWebuiReviewVisibleFiles, + isWebuiReviewFiltering, + type WebuiReviewFile, +} from "../../src/client/projection/review-state.js"; +import { groupWebuiWorktreeWorkspaces } from "../../src/client/projection/worktree-state.js"; + +/* Why this file asserts on markup rather than on clicks: + * + * The webui suite runs `environment: "node"` — there is no jsdom, and + * `renderToStaticMarkup` renders once without running effects. So the + * decisions that a click would reveal (does expanding load a diff, does a line + * carry a jump target, does a filter narrow the list) are all made upstream in + * `review-state.ts` and reach the presentational half as plain values. This + * file's job is to prove the panel renders those values faithfully and does + * not invent or drop a jump target on the way to the DOM — the negative + * injections recorded in the PR body target exactly that boundary. + */ + +const files: readonly WebuiReviewFile[] = [ + { fileId: "f1", path: "src/one.ts", status: "modified", type: "text", additions: 2, deletions: 1 }, + { fileId: "f2", path: "assets/logo.png", status: "modified", type: "binary", additions: 0, deletions: 0 }, +]; + +const ready = reduceWebuiReviewState(initialWebuiReviewState, { + type: "summary-loaded", + reviewSnapshotId: "snap-1", + files, + totals: { files: 2, additions: 2, deletions: 1 }, +}); + +const panel = (state = ready, props: Record = {}): string => + renderToStaticMarkup( + createElement(WebuiReviewPanel, { + state, + visible: selectWebuiReviewVisibleFiles(state), + filtering: isWebuiReviewFiltering(state), + onOpenFileLine: () => undefined, + ...props, + } as never), + ); + +describe("code review page states", () => { + it("marks every state it can be in, so a test can tell them apart", () => { + expect(panel(ready)).toContain('data-webui-review-state="ready"'); + expect(panel(reduceWebuiReviewState(initialWebuiReviewState, { type: "load-begun" }))).toContain( + 'data-webui-review-state="loading"', + ); + expect( + panel(reduceWebuiReviewState(initialWebuiReviewState, { type: "unavailable", reason: "" })), + ).toContain('data-webui-review-state="unavailable"'); + expect( + panel(reduceWebuiReviewState(initialWebuiReviewState, { type: "load-failed", reason: "boom" })), + ).toContain('data-webui-review-state="error"'); + }); + + /* A workspace with nothing to review is a normal state. Rendering it as a + * failure would put a red banner on every clean repository. */ + it("does not present an absent change set as an error", () => { + const unavailable = panel( + reduceWebuiReviewState(initialWebuiReviewState, { type: "unavailable", reason: "" }), + { empty: "当前工作区没有待审查的变更。" }, + ); + expect(unavailable).toContain('data-webui-review-state="unavailable"'); + expect(unavailable).not.toContain("webui-review-error"); + expect(unavailable).toContain("当前工作区没有待审查的变更。"); + }); + + it("does show a failed load as an error, with the reason", () => { + const failed = panel(reduceWebuiReviewState(initialWebuiReviewState, { type: "load-failed", reason: "仓库读取失败" })); + expect(failed).toContain('data-webui-review-state="error"'); + expect(failed).toContain('role="alert"'); + expect(failed).toContain("仓库读取失败"); + }); + + it("shows totals and one row per file when ready", () => { + const markup = panel(); + expect(markup).toContain('data-testid="review-totals"'); + expect(markup).toContain("2 个文件"); + expect(markup).toContain("src/one.ts"); + expect(markup).toContain("assets/logo.png"); + expect(markup).toContain('data-webui-review-snapshot="snap-1"'); + }); +}); + +describe("a review line is a real jump target, and only where one exists", () => { + const diff = [ + "diff --git a/src/one.ts b/src/one.ts", + "--- a/src/one.ts", + "+++ b/src/one.ts", + "@@ -10,2 +10,2 @@", + " const keep = 1;", + "-const gone = 2;", + "+const here = 3;", + ].join("\n"); + + const expanded = reduceWebuiReviewState( + reduceWebuiReviewState(ready, { type: "diffs-loaded", diffs: { f1: diff } }), + { type: "toggle-file", fileId: "f1" }, + ); + + it("renders the diff only for an expanded file", () => { + const collapsed = panel(); + expect(collapsed).not.toContain('data-testid="review-lines"'); + + const opened = panel(expanded); + expect(opened).toContain('data-testid="review-lines"'); + expect(opened).toContain("const here = 3;"); + }); + + it("gives the surviving line a jump control carrying its new-side number", () => { + const markup = panel(expanded); + expect(markup).toContain('data-webui-review-jump-line="10"'); + expect(markup).toContain('data-webui-review-jump-line="11"'); + }); + + /* The whole point of the row: a deleted line does not exist in the file the + * editor will open, so it must not be offered as a destination. */ + it("does not offer a jump on a deleted line", () => { + const markup = panel(expanded); + const deletionLine = markup.split('data-webui-review-line-kind="deletion"')[1]?.split("")[0] ?? ""; + expect(deletionLine).toContain("const gone = 2;"); + expect(deletionLine).not.toContain("review-line-jump"); + }); + + it("does not offer a jump on the hunk header", () => { + const markup = panel(expanded); + const hunk = markup.split('data-webui-review-line-kind="hunk"')[1]?.split("")[0] ?? ""; + expect(hunk).toContain("@@"); + expect(hunk).not.toContain("review-line-jump"); + }); + + /* Without a jump handler there is nothing for a click to do, so the control + * must not be rendered at all rather than rendered dead. */ + it("renders no jump control when there is nothing to call", () => { + const markup = panel(expanded, { onOpenFileLine: undefined }); + expect(markup).toContain('data-testid="review-lines"'); + expect(markup).not.toContain("review-line-jump"); + }); +}); + +describe("per-file diff outcomes are told apart", () => { + it("shows a file failure instead of an empty diff", () => { + const state = reduceWebuiReviewState( + reduceWebuiReviewState(ready, { type: "diffs-loaded", diffs: {}, errors: { f1: "补丁过大" } }), + { type: "toggle-file", fileId: "f1" }, + ); + const markup = panel(state); + expect(markup).toContain('data-testid="review-file-error"'); + expect(markup).toContain("补丁过大"); + expect(markup).not.toContain('data-testid="review-lines"'); + }); + + it("says it is loading rather than showing an empty diff body", () => { + const state = reduceWebuiReviewState( + reduceWebuiReviewState(ready, { type: "diffs-begun", fileIds: ["f1"] }), + { type: "toggle-file", fileId: "f1" }, + ); + const markup = panel(state); + expect(markup).toContain("正在读取这个文件的补丁…"); + expect(markup).not.toContain('data-testid="review-lines"'); + }); +}); + +describe("the search box reflects filter state", () => { + it("offers a clear control only while filtering", () => { + expect(panel()).not.toContain('data-testid="review-search-clear"'); + + const filtering = reduceWebuiReviewState(ready, { type: "query-changed", query: "one" }); + expect(panel(filtering)).toContain('data-testid="review-search-clear"'); + }); + + it("keeps the query visible in the box", () => { + const filtering = reduceWebuiReviewState(ready, { type: "query-changed", query: "one" }); + expect(panel(filtering)).toContain('value="one"'); + }); +}); + +/* Roadmap E 区 follow-up: the review page refused to do anything until the + * selected session already carried a workspace, and the refusal named the + * problem without offering a way out of it. Opening a workspace-bound session + * was the only route, and that route is not reachable from the page — the + * settings dialog is `aria-modal`, so the rail cannot be clicked while it is + * open. That is the same shape of defect as the disabled tabs: the user is + * parked in a state they cannot leave. + * + * The picker is a pure presentational component, exactly like + * `WebuiReviewPanel`, so the list it renders is assertable here. Choosing one + * is a click, and the click's effect on the review load is wiring — asserted + * at the bottom of this file alongside the tab routing. */ +describe("choosing a workspace to review", () => { + const workspaces = groupWebuiWorktreeWorkspaces([ + { sessionId: "s1", title: "主线", updatedAt: 20, workspaceDir: "C:\\repo\\alpha", isDefaultWorkspace: true }, + { sessionId: "s2", title: "实验", updatedAt: 10, workspaceDir: "C:\\repo\\alpha-wt" }, + ]); + + const picker = (props: Record = {}): string => + renderToStaticMarkup(createElement(WebuiReviewWorkspacePicker, props as never)); + + it("offers every reviewable workspace as a control that carries its path", () => { + const markup = picker({ workspaces }); + expect(markup).toContain('data-testid="review-workspace-picker"'); + expect(markup).toContain('data-webui-workspace-dir="C:\\repo\\alpha"'); + expect(markup).toContain('data-webui-workspace-dir="C:\\repo\\alpha-wt"'); + expect(markup).toContain('data-testid="review-workspace-option"'); + }); + + /* The same checkout spelled two ways is one workspace. If the picker did its + * own grouping it would offer the same directory twice. */ + it("does not offer the same checkout twice when the path spelling differs", () => { + const markup = picker({ + workspaces: groupWebuiWorktreeWorkspaces([ + { sessionId: "s1", title: "A", updatedAt: 20, workspaceDir: "C:\\repo\\alpha" }, + { sessionId: "s2", title: "B", updatedAt: 10, workspaceDir: "C:/repo/alpha/" }, + ]), + }); + expect(markup.match(/data-testid="review-workspace-option"/gu)?.length).toBe(1); + }); + + it("marks which workspace is being reviewed", () => { + const markup = picker({ workspaces, selected: "C:\\repo\\alpha" }); + expect(markup).toContain('data-webui-workspace-current="true"'); + const current = markup.split('data-webui-workspace-current="true"')[0]?.split(' { + const markup = picker({ workspaces: [] }); + expect(markup).toContain('data-testid="review-workspace-empty"'); + expect(markup).not.toContain('data-testid="review-workspace-option"'); + }); + + /* The other half of the same sentence: while there is something to pick, the + * explanation is not shown. Without this, "there is nothing here" can sit + * above a full list and still pass. */ + it("does not claim there is nothing to pick while there is something to pick", () => { + expect(picker({ workspaces })).not.toContain('data-testid="review-workspace-empty"'); + }); + + it("says it is loading rather than claiming there are no workspaces", () => { + const markup = picker({ workspaces: [], loading: true }); + expect(markup).toContain('data-webui-workspace-state="loading"'); + expect(markup).not.toContain('data-testid="review-workspace-empty"'); + }); + + /* The path is the identity of a workspace; the basename alone would make two + * checkouts named the same impossible to tell apart. The assertion reads the + * visible paragraph rather than the whole markup: every option also carries + * the path in `data-webui-workspace-dir`, so a whole-markup `toContain` is + * satisfied by the attribute even when the displayed text is the basename + * alone. */ + it("shows the full path, not just the folder name", () => { + const shown = [...picker({ workspaces }).matchAll(/

]*>([^<]*)<\/p>/gu)].map((match) => match[1] ?? ""); + expect(shown.join("|")).toContain("C:\\repo\\alpha"); + expect(shown.join("|")).toContain("C:\\repo\\alpha-wt"); + }); +}); + +/* Why these two are source assertions and not render assertions: + * + * `SettingsModal` opens on the `desktop` tab and only moves to another one + * from a click handler, so no `renderToStaticMarkup` call can ever reach the + * `coding` or `worktree` branch — the panels are asserted directly above and + * the wiring between the tab and the panel is not. Negative injection proved + * the gap is real: replacing `{active === "coding" ? { + const source = readFileSync( + fileURLToPath(new URL("../../src/client/components/SettingsModal.tsx", import.meta.url)), + "utf8", + ); + + it("routes the coding tab to the review page", () => { + expect(source).toContain('{active === "coding" ? { + expect(source).toContain('{active === "worktree" ? { + const fallback = source.split('webui-settings-empty-panel')[0]?.split('{active === "archived"')?.at(-1) ?? ""; + expect(fallback).toContain('active !== "coding"'); + expect(fallback).toContain('active !== "worktree"'); + }); +}); + +/* The picker's markup is asserted above; these two close the loop from a click + * to the review load, which no render here can reach — the choice is held in + * component state and the load lives in an effect. The assertions quote the + * exact expressions, so removing the picker call or hard-coding the prop back + * in turns them red. */ +describe("the chosen workspace reaches the review load", () => { + const source = readFileSync( + fileURLToPath(new URL("../../src/client/components/SettingsModal.tsx", import.meta.url)), + "utf8", + ); + const page = source.slice(source.indexOf("function SettingsReviewPage"), source.indexOf("function SettingsWorktreePage")); + + /* Without this the picker would render a list built from nothing. Scoped to + * the review page's own call: the worktree page is handed the same loader, so + * an unscoped substring check is satisfied by that call alone — which is + * exactly what the first negative-injection run found. */ + it("gives the review page the session loader it lists workspaces from", () => { + expect(source).toMatch(/]*loadSessions=\{loadSessions\}/u); + }); + + /* Asserting that the picker component exists would still pass if the page + * never rendered it, which is how the branch that shows it went untested. */ + it("shows the picker when no workspace is bound", () => { + expect(page).toContain("if (!effectiveWorkspaceDir)"); + expect(page).toContain(" { + expect(page).toContain("workspaceDir?.trim() || pickedWorkspaceDir"); + }); + + it("loads the summary for the effective workspace, not the raw prop", () => { + expect(page).toContain("getWorkspaceReviewSummary({ workspaceDir: effectiveWorkspaceDir })"); + }); +}); diff --git a/packages/webui/test/unit/review-state.test.ts b/packages/webui/test/unit/review-state.test.ts new file mode 100644 index 00000000..00760d0d --- /dev/null +++ b/packages/webui/test/unit/review-state.test.ts @@ -0,0 +1,256 @@ +import { describe, expect, it } from "vitest"; + +import { + initialWebuiReviewState, + isWebuiReviewFiltering, + projectWebuiReviewLines, + reduceWebuiReviewState, + selectWebuiReviewVisibleFiles, + webuiReviewLineTarget, + type WebuiReviewFile, +} from "../../src/client/projection/review-state.js"; + +const files: readonly WebuiReviewFile[] = [ + { fileId: "f1", path: "src/one.ts", status: "modified", type: "text", additions: 3, deletions: 1 }, + { fileId: "f2", path: "src/two.ts", status: "added", type: "text", additions: 10, deletions: 0 }, + { fileId: "f3", path: "assets/logo.png", status: "modified", type: "binary", additions: 0, deletions: 0 }, +]; + +const loaded = reduceWebuiReviewState(initialWebuiReviewState, { + type: "summary-loaded", + reviewSnapshotId: "snap-1", + files, + totals: { files: 3, additions: 13, deletions: 1 }, +}); + +describe("review page load lifecycle", () => { + it("carries the snapshot, file list and totals through", () => { + expect(loaded.status).toBe("ready"); + expect(loaded.reviewSnapshotId).toBe("snap-1"); + expect(loaded.files).toHaveLength(3); + expect(loaded.totals).toEqual({ files: 3, additions: 13, deletions: 1 }); + }); + + /* "This workspace has no review snapshot" is a normal state, not a failure. + * Reporting it as an error would show a red banner for a fresh workspace. */ + it("keeps an absent snapshot distinct from a failed load", () => { + const unavailable = reduceWebuiReviewState(loaded, { type: "unavailable", reason: "没有待审查的变更" }); + expect(unavailable.status).toBe("unavailable"); + expect(unavailable.error).toBeUndefined(); + + const failed = reduceWebuiReviewState(loaded, { type: "load-failed", reason: "仓库读取失败" }); + expect(failed.status).toBe("error"); + expect(failed.error).toBe("仓库读取失败"); + }); + + /* Diffs are keyed by fileId, and fileIds belong to a snapshot. Showing the + * previous snapshot's diffs under a new one would attribute another + * snapshot's changes to this change set. */ + it("drops the previous snapshot's diffs when a reload starts", () => { + const withDiff = reduceWebuiReviewState(loaded, { + type: "diffs-loaded", + diffs: { f1: "@@ -1 +1 @@\n-old\n+new" }, + }); + expect(withDiff.diffs.f1).toContain("+new"); + + const reloading = reduceWebuiReviewState(withDiff, { type: "load-begun" }); + expect(reloading.diffs).toEqual({}); + expect(reloading.diffErrors).toEqual({}); + expect(reloading.status).toBe("loading"); + }); + + it("settles only the files that were actually in flight", () => { + const loading = reduceWebuiReviewState(loaded, { type: "diffs-begun", fileIds: ["f1", "f2"] }); + expect(loading.loadingFileIds).toEqual(["f1", "f2"]); + + const settled = reduceWebuiReviewState(loading, { + type: "diffs-loaded", + diffs: { f1: "diff f1" }, + errors: { f2: "补丁过大" }, + }); + expect(settled.loadingFileIds).toEqual([]); + expect(settled.diffs.f1).toBe("diff f1"); + expect(settled.diffErrors.f2).toBe("补丁过大"); + }); + + it("drops a stale in-flight marker once its diff lands", () => { + const loading = reduceWebuiReviewState(loaded, { type: "diffs-begun", fileIds: ["f1", "f2"] }); + const first = reduceWebuiReviewState(loading, { type: "diffs-loaded", diffs: { f1: "a" } }); + expect(first.loadingFileIds).toEqual(["f2"]); + + const second = reduceWebuiReviewState(first, { type: "diffs-loaded", diffs: { f2: "b" } }); + expect(second.loadingFileIds).toEqual([]); + expect(Object.keys(second.diffs).sort()).toEqual(["f1", "f2"]); + }); +}); + +describe("review search filter", () => { + it("shows every file before a search has run for the current query", () => { + const searching = reduceWebuiReviewState(loaded, { type: "query-changed", query: "one" }); + expect(isWebuiReviewFiltering(searching)).toBe(true); + expect(selectWebuiReviewVisibleFiles(searching)).toHaveLength(3); + }); + + it("narrows to the matched files once the search settles", () => { + let state = reduceWebuiReviewState(loaded, { type: "query-changed", query: "one" }); + state = reduceWebuiReviewState(state, { type: "search-begun" }); + expect(state.searchPending).toBe(true); + state = reduceWebuiReviewState(state, { type: "search-settled", fileIds: ["f1"] }); + expect(selectWebuiReviewVisibleFiles(state).map((file) => file.fileId)).toEqual(["f1"]); + }); + + /* A search that matched nothing is an empty list. It must not silently fall + * back to the unfiltered list, which is what an empty match set used to do. */ + it("shows an empty list when the query matched nothing", () => { + let state = reduceWebuiReviewState(loaded, { type: "query-changed", query: "nothing-matches" }); + state = reduceWebuiReviewState(state, { type: "search-settled", fileIds: [] }); + expect(selectWebuiReviewVisibleFiles(state)).toEqual([]); + }); + + /* Editing the query must invalidate the previous matches, or the list keeps + * filtering by a query that is no longer in the box. */ + it("drops previous matches when the query is edited", () => { + let state = reduceWebuiReviewState(loaded, { type: "query-changed", query: "one" }); + state = reduceWebuiReviewState(state, { type: "search-settled", fileIds: ["f1"] }); + state = reduceWebuiReviewState(state, { type: "query-changed", query: "two" }); + expect(state.matchedFileIds).toEqual([]); + expect(selectWebuiReviewVisibleFiles(state)).toHaveLength(3); + }); + + it("restores the full list when filters are cleared", () => { + let state = reduceWebuiReviewState(loaded, { type: "query-changed", query: "one" }); + state = reduceWebuiReviewState(state, { type: "search-settled", fileIds: ["f1"] }); + state = reduceWebuiReviewState(state, { type: "clear-filters" }); + expect(state.query).toBe(""); + expect(isWebuiReviewFiltering(state)).toBe(false); + expect(selectWebuiReviewVisibleFiles(state)).toHaveLength(3); + }); + + it("treats a whitespace-only query as no filter at all", () => { + const state = reduceWebuiReviewState(loaded, { type: "query-changed", query: " " }); + expect(isWebuiReviewFiltering(state)).toBe(false); + expect(selectWebuiReviewVisibleFiles(state)).toHaveLength(3); + }); +}); + +describe("unified diff projection and the line target it yields", () => { + const diff = [ + "diff --git a/src/one.ts b/src/one.ts", + "index 123..456 100644", + "--- a/src/one.ts", + "+++ b/src/one.ts", + "@@ -10,4 +10,5 @@ export function thing() {", + " const before = 1;", + "-const removed = 2;", + "+const added = 3;", + "+const alsoAdded = 4;", + " const after = 5;", + ].join("\n"); + + it("numbers both sides from the hunk header", () => { + const lines = projectWebuiReviewLines(diff); + const context = lines.find((line) => line.text === "const before = 1;"); + // Header says -10,4 +10,5, so the first context line is 10 on both sides. + expect(context?.newLine).toBe(10); + expect(context?.oldLine).toBe(10); + }); + + it("numbers a deletion on the old side only and an addition on the new side only", () => { + const lines = projectWebuiReviewLines(diff); + const deletion = lines.find((line) => line.text === "const removed = 2;"); + expect(deletion?.kind).toBe("deletion"); + expect(deletion?.oldLine).toBe(11); + expect(deletion?.newLine).toBeUndefined(); + + const addition = lines.find((line) => line.text === "const added = 3;"); + expect(addition?.kind).toBe("addition"); + expect(addition?.newLine).toBe(11); + expect(addition?.oldLine).toBeUndefined(); + }); + + it("advances the new side past a deletion and both sides past a context line", () => { + const lines = projectWebuiReviewLines(diff); + const second = lines.find((line) => line.text === "const alsoAdded = 4;"); + expect(second?.newLine).toBe(12); + const after = lines.find((line) => line.text === "const after = 5;"); + expect(after?.newLine).toBe(13); + expect(after?.oldLine).toBe(12); + }); + + /* This is the whole point of the row: a deleted line does not exist in the + * file the editor will open, so it must not yield a jump target. */ + it("gives a deletion no editor target and gives additions and context one", () => { + const lines = projectWebuiReviewLines(diff); + const deletion = lines.find((line) => line.text === "const removed = 2;"); + const addition = lines.find((line) => line.text === "const added = 3;"); + const context = lines.find((line) => line.text === "const before = 1;"); + const header = lines.find((line) => line.kind === "hunk"); + expect(webuiReviewLineTarget(deletion!)).toBeUndefined(); + expect(webuiReviewLineTarget(addition!)).toBe(11); + expect(webuiReviewLineTarget(context!)).toBe(10); + expect(webuiReviewLineTarget(header!)).toBeUndefined(); + }); + + it("does not let a no-newline marker consume a line number", () => { + const withMarker = ["@@ -1,2 +1,2 @@", " keep", "-drop", "\\ No newline at end of file", "+added"].join("\n"); + const lines = projectWebuiReviewLines(withMarker); + const added = lines.find((line) => line.text === "added"); + expect(added?.newLine).toBe(2); + const marker = lines.find((line) => line.text.startsWith("\\ No newline")); + expect(marker?.kind).toBe("meta"); + }); + + it("treats content before the first hunk header as metadata, not as lines", () => { + const lines = projectWebuiReviewLines(diff); + const meta = lines.filter((line) => line.kind === "meta"); + expect(meta.map((line) => line.text)).toEqual([ + "diff --git a/src/one.ts b/src/one.ts", + "index 123..456 100644", + "--- a/src/one.ts", + "+++ b/src/one.ts", + ]); + expect(meta.every((line) => webuiReviewLineTarget(line) === undefined)).toBe(true); + }); + + it("survives a hunk header without line numbers", () => { + const lines = projectWebuiReviewLines("@@ -0,0 +1,2 @@\n+one\n+two"); + expect(lines.map((line) => line.newLine)).toEqual([undefined, 1, 2]); + }); + + it("handles CRLF diffs without leaking carriage returns into the line text", () => { + const lines = projectWebuiReviewLines("@@ -1,1 +1,2 @@\r\n keep\r\n+added\r\n"); + const added = lines.find((line) => line.text === "added"); + expect(added).toBeDefined(); + expect(lines.every((line) => !line.text.includes("\r"))).toBe(true); + }); + + it("handles an empty diff", () => { + expect(projectWebuiReviewLines("")).toEqual([{ kind: "meta", text: "" }]); + }); + + /* `webuiReviewLineTarget` reads this invariant instead of re-checking kinds: + * a guard there was untestable, because the projection already guaranteed + * the result. Asserting the invariant here is what makes the jump behaviour + * depend on a tested contract rather than on an untestable line. */ + it("gives a new-side line number only to lines that exist in the new file", () => { + const everyDiff = [ + "diff --git a/x b/x", + "--- a/x", + "+++ b/x", + "@@ -1,3 +1,3 @@", + " keep", + "-gone", + "\\ No newline at end of file", + "+here", + ].join("\n"); + for (const line of projectWebuiReviewLines(everyDiff)) { + if (line.kind === "addition" || line.kind === "context") { + expect(line.newLine, `${line.kind} should carry a new-side number`).toBeTypeOf("number"); + expect(webuiReviewLineTarget(line)).toBe(line.newLine); + } else { + expect(line.newLine, `${line.kind} must not carry a new-side number`).toBeUndefined(); + expect(webuiReviewLineTarget(line)).toBeUndefined(); + } + } + }); +}); diff --git a/packages/webui/test/unit/settings-modal.test.tsx b/packages/webui/test/unit/settings-modal.test.tsx index 619c153a..38d426c5 100644 --- a/packages/webui/test/unit/settings-modal.test.tsx +++ b/packages/webui/test/unit/settings-modal.test.tsx @@ -90,14 +90,19 @@ describe("desktop settings registry", () => { }); describe("account tab gating", () => { - it("leaves the account tab clickable while the unimplemented tabs stay disabled", () => { - // The account panel is fully implemented (email row, sign-out button, - // sign-out error region), so it must not carry the `disabled` gate the - // not-yet-built panels still need. - const account = DESKTOP_SETTINGS_TABS.find((tab) => tab.key === "account"); - expect(account?.disabled).toBeUndefined(); - for (const key of ["voice", "shortcuts", "custom-instructions", "connection", "coding", "worktree"]) + it("leaves the implemented tabs clickable while the unimplemented tabs stay disabled", () => { + // A fully implemented panel must not carry the `disabled` gate the + // not-yet-built panels still need. `account` (email row, sign-out button, + // sign-out error region), `coding` (workspace review) and `worktree` + // (parallel experiment branches) are all built now, so all three sit in + // the ungated group. The rest have no content behind them and clicking + // one would land on an empty pane, so they must stay disabled. + for (const key of ["account", "coding", "worktree"]) { + expect(DESKTOP_SETTINGS_TABS.find((tab) => tab.key === key)?.disabled).toBeUndefined(); + } + for (const key of ["voice", "shortcuts", "custom-instructions", "connection"]) { expect(DESKTOP_SETTINGS_TABS.find((tab) => tab.key === key)?.disabled).toBe(true); + } }); it("keeps the account tab reachable through the settings search", () => { diff --git a/packages/webui/test/unit/worktree-panel.test.tsx b/packages/webui/test/unit/worktree-panel.test.tsx new file mode 100644 index 00000000..8e3634e0 --- /dev/null +++ b/packages/webui/test/unit/worktree-panel.test.tsx @@ -0,0 +1,103 @@ +import { createElement } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; + +import { WebuiWorktreePanel } from "../../src/client/components/SettingsModal.js"; +import { + groupWebuiWorktreeWorkspaces, + selectWebuiWorktreeWorkspaces, + type WebuiWorktreeSourceSession, +} from "../../src/client/projection/worktree-state.js"; + +/* `renderToStaticMarkup` runs the first render and never runs effects, so the + * fetching half of this page cannot be asserted here. What this file proves is + * that the panel renders the grouping it is handed faithfully — in particular + * that a page with no worktrees says so instead of showing an empty shell, and + * that a branch's sessions are reachable by the same `#session=` link the rail + * uses, so there is one navigation path rather than two. */ + +const main = "C:\\repos\\my-app"; +const branchA = "C:\\repos\\my-app\\.worktrees\\feature-a"; +const branchB = "C:\\repos\\my-app\\.worktrees\\feature-b"; + +const session = ( + sessionId: string, + updatedAt: number, + extra: Partial = {}, +): WebuiWorktreeSourceSession => ({ sessionId, updatedAt, ...extra }); + +const panel = (sessions: readonly WebuiWorktreeSourceSession[], props: Record = {}): string => { + const workspaces = groupWebuiWorktreeWorkspaces(sessions); + return renderToStaticMarkup( + createElement(WebuiWorktreePanel, { + workspaces, + worktrees: selectWebuiWorktreeWorkspaces(workspaces), + ...props, + } as never), + ); +}; + +describe("worktree page", () => { + it("says so plainly when there is no worktree yet", () => { + const markup = panel([session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true })]); + expect(markup).toContain('data-webui-worktree-state="empty"'); + expect(markup).toContain('data-testid="worktree-empty"'); + expect(markup).toContain("复制到新工作树"); + }); + + /* The point of the row: a project with parallel experiment branches should + * be able to see them. A page that renders only the primary checkout would + * be indistinguishable from the empty state. */ + it("lists every worktree alongside the primary checkout", () => { + const markup = panel([ + session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true, title: "主线" }), + session("s2", 300, { workspaceDir: branchA, title: "实验 A", parentSessionId: "s1" }), + session("s4", 250, { workspaceDir: branchA, title: "实验 A 续", parentSessionId: "s1" }), + session("s3", 200, { workspaceDir: branchB, title: "实验 B", parentSessionId: "s1" }), + ]); + expect(markup).toContain('data-webui-worktree-state="ready"'); + expect(markup).toContain('data-testid="worktree-primary"'); + expect(markup).toContain("主线"); + expect(markup).toContain("实验 A"); + expect(markup).toContain("实验 A 续"); + expect(markup).toContain("实验 B"); + expect(markup).toContain("主检出"); + // feature-a holds two sessions and feature-b one, so the per-branch count + // has to differ between them rather than repeat a single number. + expect(markup).toContain("2 个会话"); + expect(markup).toContain("1 个会话"); + }); + + it("links a branch session through the same hash route the rail uses", () => { + const markup = panel([ + session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true }), + session("s2", 300, { workspaceDir: branchA, title: "实验 A" }), + ]); + expect(markup).toContain('href="#session=s2"'); + expect(markup).toContain('data-webui-worktree-session="s2"'); + }); + + it("marks a session forked from another as derived", () => { + const markup = panel([ + session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true }), + session("s2", 300, { workspaceDir: branchA, parentSessionId: "s1" }), + ]); + expect(markup).toContain("派生"); + expect(markup).toContain("派生自 s1"); + }); + + it("reports a failed load as an error", () => { + const markup = panel([], { error: "会话列表读取失败" }); + expect(markup).toContain('data-webui-worktree-state="error"'); + expect(markup).toContain('role="alert"'); + expect(markup).toContain("会话列表读取失败"); + }); + + it("shows the full checkout path so two branches of the same name are distinguishable", () => { + const markup = panel([ + session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true }), + session("s2", 300, { workspaceDir: branchA }), + ]); + expect(markup).toContain(branchA); + }); +}); diff --git a/packages/webui/test/unit/worktree-state.test.ts b/packages/webui/test/unit/worktree-state.test.ts new file mode 100644 index 00000000..1037b5d3 --- /dev/null +++ b/packages/webui/test/unit/worktree-state.test.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from "vitest"; + +import { + groupWebuiWorktreeWorkspaces, + normalizeWebuiWorkspaceDir, + selectWebuiPrimaryWorkspace, + selectWebuiWorktreeWorkspaces, + webuiWorkspaceName, + type WebuiWorktreeSourceSession, +} from "../../src/client/projection/worktree-state.js"; + +const session = ( + sessionId: string, + updatedAt: number, + extra: Partial = {}, +): WebuiWorktreeSourceSession => ({ sessionId, updatedAt, ...extra }); + +describe("workspace path normalization", () => { + /* A fork round-trips the path through the runtime, and Windows and POSIX + * spell the same checkout differently. One checkout reported as two + * worktrees is a wrong list, not a cosmetic one. */ + it("treats separator and trailing-slash variants as one checkout", () => { + expect(normalizeWebuiWorkspaceDir("C:\\repo")).toBe("c:/repo"); + expect(normalizeWebuiWorkspaceDir("C:/repo")).toBe("c:/repo"); + expect(normalizeWebuiWorkspaceDir("C:/repo/")).toBe("c:/repo"); + expect(normalizeWebuiWorkspaceDir(" C:\\repo\\ ")).toBe("c:/repo"); + }); + + it("names a checkout from its last path segment", () => { + expect(webuiWorkspaceName("C:\\repos\\my-app")).toBe("my-app"); + expect(webuiWorkspaceName("/home/dev/my-app/")).toBe("my-app"); + expect(webuiWorkspaceName("")).toBe(""); + }); +}); + +describe("grouping sessions into checkouts", () => { + const main = "C:\\repos\\my-app"; + const worktreeA = "C:\\repos\\my-app\\.worktrees\\feature-a"; + const worktreeB = "C:\\repos\\my-app\\.worktrees\\feature-b"; + + it("separates the primary checkout from its worktrees", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true }), + session("s2", 300, { workspaceDir: worktreeA, parentSessionId: "s1" }), + session("s3", 200, { workspaceDir: worktreeB, parentSessionId: "s1" }), + ]); + expect(groups).toHaveLength(3); + expect(groups[0]?.isPrimary).toBe(true); + expect(selectWebuiPrimaryWorkspace(groups)?.workspaceDir).toBe(main); + expect(selectWebuiWorktreeWorkspaces(groups).map((group) => group.name)).toEqual(["feature-a", "feature-b"]); + }); + + it("orders worktrees by most recently touched, primary first", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true }), + session("s2", 300, { workspaceDir: worktreeA }), + session("s3", 200, { workspaceDir: worktreeB }), + ]); + expect(groups.map((group) => group.workspaceDir)).toEqual([main, worktreeA, worktreeB]); + // The primary sorts first even though it is the oldest of the three. + expect(groups[0]?.updatedAt).toBe(100); + expect(groups[1]?.updatedAt).toBe(300); + }); + + it("keeps a checkout primary when a later session omits the flag", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s1", 200, { workspaceDir: main, isDefaultWorkspace: true }), + session("s2", 100, { workspaceDir: main }), + ]); + expect(groups).toHaveLength(1); + expect(groups[0]?.isPrimary).toBe(true); + expect(groups[0]?.sessions).toHaveLength(2); + }); + + it("merges sessions that reach the same checkout by different spellings", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s1", 200, { workspaceDir: "C:\\repos\\my-app", isDefaultWorkspace: true }), + session("s2", 300, { workspaceDir: "c:/repos/my-app/" }), + ]); + expect(groups).toHaveLength(1); + expect(groups[0]?.sessions.map((entry) => entry.sessionId).sort()).toEqual(["s1", "s2"]); + expect(groups[0]?.updatedAt).toBe(300); + }); + + it("sorts the sessions inside a checkout by most recently touched", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("old", 100, { workspaceDir: worktreeA }), + session("new", 500, { workspaceDir: worktreeA }), + session("mid", 300, { workspaceDir: worktreeA }), + ]); + expect(groups[0]?.sessions.map((entry) => entry.sessionId)).toEqual(["new", "mid", "old"]); + }); + + /* An archived session belongs to the archived page. Counting it here would + * make an abandoned worktree look like a live experiment branch. */ + it("drops archived sessions and the worktrees that were only archived", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s1", 100, { workspaceDir: main, isDefaultWorkspace: true }), + session("s2", 300, { workspaceDir: worktreeA, archived: true }), + ]); + expect(groups).toHaveLength(1); + expect(selectWebuiWorktreeWorkspaces(groups)).toEqual([]); + }); + + /* A session with no checkout has no worktree to belong to; a synthetic + * bucket for it would show up as a phantom branch. */ + it("drops sessions with no workspace", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s1", 100), + session("s2", 200, { workspaceDir: " " }), + session("s3", 300, { workspaceDir: main, isDefaultWorkspace: true }), + ]); + expect(groups).toHaveLength(1); + expect(groups[0]?.sessions.map((entry) => entry.sessionId)).toEqual(["s3"]); + }); + + it("falls back to the session id when a title is blank", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s1", 100, { workspaceDir: main, title: " " }), + ]); + expect(groups[0]?.sessions[0]?.title).toBe("s1"); + }); + + it("carries the fork parent so a branch can be traced to its origin", () => { + const groups = groupWebuiWorktreeWorkspaces([ + session("s2", 100, { workspaceDir: worktreeA, parentSessionId: "s1" }), + session("s3", 100, { workspaceDir: worktreeB }), + ]); + expect(groups[0]?.sessions[0]?.parentSessionId).toBe("s1"); + expect(groups[1]?.sessions[0]?.parentSessionId).toBeUndefined(); + }); + + it("orders equal timestamps deterministically by path", () => { + const first = groupWebuiWorktreeWorkspaces([ + session("s1", 100, { workspaceDir: "C:\\b" }), + session("s2", 100, { workspaceDir: "C:\\a" }), + ]); + const second = groupWebuiWorktreeWorkspaces([ + session("s2", 100, { workspaceDir: "C:\\a" }), + session("s1", 100, { workspaceDir: "C:\\b" }), + ]); + expect(first.map((group) => group.workspaceDir)).toEqual(second.map((group) => group.workspaceDir)); + expect(first.map((group) => group.name)).toEqual(["a", "b"]); + }); + + it("handles an empty session list", () => { + expect(groupWebuiWorktreeWorkspaces([])).toEqual([]); + expect(selectWebuiWorktreeWorkspaces([])).toEqual([]); + expect(selectWebuiPrimaryWorkspace([])).toBeUndefined(); + }); +}); diff --git a/release/public-source.json b/release/public-source.json index 11eeed79..b57a74e9 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -3586,6 +3586,7 @@ "packages/webui/src/client/projection/outside-close.ts", "packages/webui/src/client/projection/plan-mode.ts", "packages/webui/src/client/projection/questionnaire-state.ts", + "packages/webui/src/client/projection/review-state.ts", "packages/webui/src/client/projection/shell-surface.ts", "packages/webui/src/client/projection/thinking-control.ts", "packages/webui/src/client/projection/token-plan-model.ts", @@ -3597,6 +3598,7 @@ "packages/webui/src/client/projection/usage-settings.ts", "packages/webui/src/client/projection/workspace-panel-state.ts", "packages/webui/src/client/projection/workspace-progress.ts", + "packages/webui/src/client/projection/worktree-state.ts", "packages/webui/src/client/rail-buckets.ts", "packages/webui/src/client/router.ts", "packages/webui/src/client/session-activity.ts", @@ -3671,6 +3673,7 @@ "packages/webui/test/unit/context-breakdown.test.ts", "packages/webui/test/unit/context-usage-popover.test.ts", "packages/webui/test/unit/conversation-usage-banner.test.tsx", + "packages/webui/test/unit/diff-mutation-error.test.tsx", "packages/webui/test/unit/fork-session-operation.test.ts", "packages/webui/test/unit/markdown-autolink-double-anchor.test.tsx", "packages/webui/test/unit/markdown-codeblock-c1-highlight.test.tsx", @@ -3691,6 +3694,8 @@ "packages/webui/test/unit/rail-context-menu.test.ts", "packages/webui/test/unit/rail-pin-affordance.test.ts", "packages/webui/test/unit/rail-star-favorites.test.ts", + "packages/webui/test/unit/review-panel.test.tsx", + "packages/webui/test/unit/review-state.test.ts", "packages/webui/test/unit/session-activity.test.ts", "packages/webui/test/unit/session-import.test.ts", "packages/webui/test/unit/session-rail-search.test.ts", @@ -3741,6 +3746,8 @@ "packages/webui/test/unit/webui-workspace-html-preview.test.tsx", "packages/webui/test/unit/webui-workspace-media-preview.test.tsx", "packages/webui/test/unit/workspace-panel-state.test.ts", + "packages/webui/test/unit/worktree-panel.test.tsx", + "packages/webui/test/unit/worktree-state.test.ts", "packages/webui/tsconfig.client.json", "packages/webui/tsconfig.json", "packages/webui/tsconfig.paths.json", diff --git a/test/vitest-suites.json b/test/vitest-suites.json index 57fe0e30..38b16716 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -258,6 +258,11 @@ "packages/webui/test/unit/token-plan-model.test.ts", "packages/webui/test/unit/thinking-control.test.tsx", "packages/webui/test/unit/webui-transcript-widgets-integration.test.ts", + "packages/webui/test/unit/diff-mutation-error.test.tsx", + "packages/webui/test/unit/worktree-panel.test.tsx", + "packages/webui/test/unit/worktree-state.test.ts", + "packages/webui/test/unit/review-panel.test.tsx", + "packages/webui/test/unit/review-state.test.ts", "packages/webui/test/unit/webui-round3-acceptance.test.tsx", "packages/webui/test/unit/webui-w0-store-sequence.test.ts", "packages/webui/test/unit/webui-w0-projections.test.ts", diff --git a/test/webui-browser/settings-account-tab.spec.mjs b/test/webui-browser/settings-account-tab.spec.mjs index bf4e7336..e7916d1f 100644 --- a/test/webui-browser/settings-account-tab.spec.mjs +++ b/test/webui-browser/settings-account-tab.spec.mjs @@ -107,8 +107,13 @@ test("the account tab is enabled where the unfinished tabs are still disabled", // The other half, and the reason the assertion above is not vacuous: the nav // really does still ship disabled items, so "enabled" is a fact about this // tab rather than a property of every button in the sidebar. A nav that - // silently enabled 语音/快捷键/连接/代码审查 would pass the first two lines. - for (const key of ["voice", "shortcuts", "connection", "coding"]) { + // silently enabled 语音/快捷键/连接 would pass the first two lines. + // + // 代码审查 and 工作树 left this list when their pages were built — they are + // real panels now, not empty panes behind a clickable label. The three that + // remain have no content behind them, which is what the `disabled` gate is + // still for. + for (const key of ["voice", "shortcuts", "connection"]) { await expect(settingsNavItem(page, key)).toBeDisabled(); } // The app behind the modal is still mounted: the modal is a `document.body` @@ -117,6 +122,29 @@ test("the account tab is enabled where the unfinished tabs are still disabled", await expect(page.locator('[data-testid="webui-error-boundary"]')).toHaveCount(0); }); +test("the code-review and worktree tabs open their own panels instead of an empty pane", async ({ page }) => { + await openApp(page, "#session=A"); + await openSettings(page); + + // Same argument as the account tab above, and the reason the unit suite + // cannot stand in: `disabled: false` is a flag on the definition, while + // "clicking it lands on the page" is the feature. The empty-pane + // placeholder is the failure this has to rule out — a clickable label with + // nothing behind it is exactly the state these two tabs were in. + await expect(settingsNavItem(page, "coding")).toBeEnabled(); + await settingsNavItem(page, "coding").click(); + await expect(page.locator('[data-testid="settings-review-page"]')).toBeVisible(); + // One status marker, and it is the review page's own: with no workspace + // selected the page says so instead of rendering a bare heading. + await expect(page.locator("[data-webui-review-state]")).toHaveCount(1); + await expect(page.locator(".webui-settings-empty-panel")).toHaveCount(0); + + await expect(settingsNavItem(page, "worktree")).toBeEnabled(); + await settingsNavItem(page, "worktree").click(); + await expect(page.locator('[data-testid="settings-worktree-page"]')).toBeVisible(); + await expect(page.locator(".webui-settings-empty-panel")).toHaveCount(0); +}); + test("clicking the account tab shows the account panel", async ({ page }) => { await openApp(page, "#session=A"); await openSettings(page);