From 6163e59eb49469200e3ff8f2cefd8909b88978f3 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Sun, 4 Oct 2026 21:03:59 +0800 Subject: [PATCH 1/3] feat(desktop): separate Git review empty and error states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Changes panel treated every unreadable source alike: one error Banner with Retry. A directory that is not a repository can never succeed on retry, so the shared failure surface invited a retry that could not help (#5940). Map the failure reasons to distinct states in SessionReviewPanel: - not_git_repository: neutral guidance (EmptyState surface, not the red Banner) naming what Changes needs, the task directory it looked at, and the next step (git init here or move the task into a repository); no Retry, since retrying cannot create a repository. - workspace_unavailable: keeps its error Banner and Retry (restoring the directory makes a retry succeed) and adds recovery guidance pointing at restore-or-relocate paths. - git_failed and unborn_repository: unchanged error Banner + Retry. - empty (no differences) and invalid_base_branch (auto-recovered in load()) stay as they were. The task directory reaches the panel through an optional `cwd` on the GitReviewReadResult failure branch; the git-review:read handler fills it from the Session's declared workspace (and from the directory the read ran in, for the other reasons). The app default workspacePath is deliberately not used — it is not the task directory. Stories: ChangesSourceNotGit now asserts guidance without Retry; new ChangesWorkspaceUnavailable covers the recovery Banner. Fixes #5940 Generated-by: GLM-5.3-Flash (ZCode) --- .../session-review-panel-recovery.test.ts | 92 +++++++++++++++++++ .../main/runtime-host-workspace-ipc-main.ts | 25 ++++- .../contracts/conversation-copy.ts | 17 ++++ .../tools/review/session-review-panel.tsx | 72 ++++++++++++--- .../stories/session-workbar.stories.tsx | 31 ++++++- packages/core/src/git-review.ts | 6 ++ 6 files changed, 222 insertions(+), 21 deletions(-) diff --git a/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts b/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts index c5637efca04..1143bf0e557 100644 --- a/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts +++ b/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts @@ -124,6 +124,98 @@ for (const reason of ['not_git_repository', 'workspace_unavailable', 'git_failed }); } +test('a task directory outside any repository guides instead of failing', async () => { + const { document, restore } = installDom(); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + const services = createFakeWorkbarServices({ review: { + read: async () => ({ ok: false, reason: 'not_git_repository', cwd: '/tmp/plain-task' }), + subscribeSessionEvents: () => () => undefined, + } }); + try { + await act(async () => { + root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(WorkbarServicesProvider, { services }, + createElement(SessionReviewPanel, { sessionId: 'plain-task', active: true })), + })); + }); + assert.match(container.textContent ?? '', /not a Git repository/); + assert.match(container.textContent ?? '', /git init/); + assert.match(container.textContent ?? '', /Task directory: \/tmp\/plain-task/); + assert.equal( + container.querySelector('button'), + null, + 'retrying cannot turn a directory into a repository', + ); + } finally { + await act(async () => { root.unmount(); }); + restore(); + } +}); + +test('an unavailable workspace keeps retry and names the recovery path', async () => { + const { document, restore } = installDom(); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + const services = createFakeWorkbarServices({ review: { + read: async () => ({ ok: false, reason: 'workspace_unavailable', cwd: '/tmp/vanished-task' }), + subscribeSessionEvents: () => () => undefined, + } }); + try { + await act(async () => { + root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(WorkbarServicesProvider, { services }, + createElement(SessionReviewPanel, { sessionId: 'vanished-task', active: true })), + })); + }); + assert.match(container.textContent ?? '', /unavailable/); + assert.match(container.textContent ?? '', /may have been moved/); + assert.match(container.textContent ?? '', /Task directory: \/tmp\/vanished-task/); + const retry = Array.from(container.querySelectorAll('button')) + .find((button) => button.textContent === 'Retry'); + assert.ok(retry, 'restoring the directory makes a retry meaningful'); + } finally { + await act(async () => { root.unmount(); }); + restore(); + } +}); + +for (const reason of ['git_failed', 'unborn_repository'] as const) { + test(`a ${reason} read failure keeps the error banner and retry`, async () => { + const { document, restore } = installDom(); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + const services = createFakeWorkbarServices({ review: { + read: async () => ({ ok: false, reason, cwd: '/tmp/live-task' }), + subscribeSessionEvents: () => () => undefined, + } }); + try { + await act(async () => { + root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(WorkbarServicesProvider, { services }, + createElement(SessionReviewPanel, { sessionId: 'failed-read', active: true })), + })); + }); + assert.match(container.textContent ?? '', /Could not read|no commit to compare/); + const retry = Array.from(container.querySelectorAll('button')) + .find((button) => button.textContent === 'Retry'); + assert.ok(retry, 'a retriable read failure keeps its retry'); + if (reason === 'git_failed') { + assert.doesNotMatch(container.textContent ?? '', /Task directory:/); + } + } finally { + await act(async () => { root.unmount(); }); + restore(); + } + }); +} + test('a disappeared saved branch clears the pin and retries with the dynamic default', async () => { const { document, restore } = installDom(); const container = document.querySelector('#root'); diff --git a/apps/desktop/src/main/runtime-host-workspace-ipc-main.ts b/apps/desktop/src/main/runtime-host-workspace-ipc-main.ts index f2f2d91ca69..ad81c50b971 100644 --- a/apps/desktop/src/main/runtime-host-workspace-ipc-main.ts +++ b/apps/desktop/src/main/runtime-host-workspace-ipc-main.ts @@ -40,17 +40,32 @@ export function registerRuntimeHostWorkspaceIpc( return { ok: false as const, reason: 'workspace_unavailable' as const }; } const request = readRequest(raw); - const cwd = await sessionWorkspace(input.client, request.sessionId); - if (!cwd) return { ok: false as const, reason: 'workspace_unavailable' as const }; - return readGitReview(cwd, request.source, undefined, request.baseBranch); + const workspace = await sessionWorkspace(input.client, request.sessionId); + if (!workspace.readable) { + // The guidance states name the directory they refer to; report the + // path the Session declares even though it cannot be read. + return { + ok: false as const, + reason: 'workspace_unavailable' as const, + cwd: workspace.declared, + }; + } + const result = await readGitReview(workspace.readable, request.source, undefined, request.baseBranch); + return result.ok ? result : { ...result, cwd: workspace.readable }; }); } -async function sessionWorkspace(client: WorkspaceClient, sessionId: string): Promise { +async function sessionWorkspace( + client: WorkspaceClient, + sessionId: string, +): Promise<{ declared: string; readable: string | null }> { const session = await client.getSession(sessionId); if (!session) throw new Error(`No such Session: ${sessionId}`); const workspace = await stat(session.workspace.hostCwd).catch(() => null); - return workspace?.isDirectory() ? session.workspace.hostCwd : null; + return { + declared: session.workspace.hostCwd, + readable: workspace?.isDirectory() ? session.workspace.hostCwd : null, + }; } function readRequest(value: unknown): { diff --git a/apps/desktop/src/renderer/application/contracts/conversation-copy.ts b/apps/desktop/src/renderer/application/contracts/conversation-copy.ts index d3b1d2e8483..cf13ac810bd 100644 --- a/apps/desktop/src/renderer/application/contracts/conversation-copy.ts +++ b/apps/desktop/src/renderer/application/contracts/conversation-copy.ts @@ -141,7 +141,13 @@ export interface DesktopConversationCopy { /** The panel-empty (tier 2) sentence under `empty`. */ emptyHelp: string; notGitRepository: string; + /** Neutral guidance under `notGitRepository`: what Changes needs and the next step. */ + notGitRepositoryHelp: string; workspaceUnavailable: string; + /** Recovery guidance under `workspaceUnavailable`, pointing at existing recovery paths. */ + workspaceUnavailableHelp: string; + /** Names the task directory a guidance state refers to. */ + taskDirectoryPath(path: string): string; unbornRepository: string; gitFailed: string; baseBranchLabel: string; @@ -406,7 +412,10 @@ const COPY = { empty: '当前 Git 工作区没有变化', emptyHelp: '提交、暂存或修改文件后，变化会显示在这里。', notGitRepository: '当前任务目录不是 Git 仓库', + notGitRepositoryHelp: '变更基于 Git 历史进行比较，因此该目录需要是 Git 仓库。可在此初始化仓库（git init），或将任务移到已有仓库。', workspaceUnavailable: '当前任务目录已不可用', + workspaceUnavailableHelp: '该目录可能已被移动、删除或暂时无法访问。恢复该目录或切换项目的工作目录后重试。', + taskDirectoryPath: (path) => `任务目录：${path}`, unbornRepository: 'Git 仓库还没有可比较的提交', gitFailed: '无法读取 Git 工作区变化', baseBranchLabel: '对比分支', @@ -649,7 +658,10 @@ const COPY = { empty: '目前 Git 工作區沒有變化', emptyHelp: '提交、暫存或修改檔案後，變化會顯示在這裡。', notGitRepository: '目前任務目錄不是 Git 倉庫', + notGitRepositoryHelp: '變更基於 Git 歷史進行比較，因此該目錄需要是 Git 倉庫。可在此初始化倉庫（git init），或將任務移到已有倉庫。', workspaceUnavailable: '目前任務目錄已不可用', + workspaceUnavailableHelp: '該目錄可能已被移動、刪除或暫時無法存取。還原該目錄或切換專案的工作目錄後重試。', + taskDirectoryPath: (path) => `任務目錄：${path}`, unbornRepository: 'Git 倉庫還沒有可比較的提交', gitFailed: '無法讀取 Git 工作區變化', baseBranchLabel: '對比分支', @@ -883,7 +895,12 @@ const COPY = { empty: 'No changes in the current Git workspace', emptyHelp: 'Committed, staged, and modified files appear here.', notGitRepository: 'This task directory is not a Git repository', + notGitRepositoryHelp: + 'Changes reviews Git history, so this directory must be a Git repository. Initialize one here (git init), or move the task to an existing repository.', workspaceUnavailable: 'This task directory is unavailable', + workspaceUnavailableHelp: + "This directory may have been moved, deleted, or become inaccessible. Restore it or switch the project's working directory, then retry.", + taskDirectoryPath: (path) => `Task directory: ${path}`, unbornRepository: 'This Git repository has no commit to compare yet', gitFailed: 'Could not read Git workspace changes', baseBranchLabel: 'Compare against', diff --git a/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx index ec75c31932f..c0e426ce405 100644 --- a/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx +++ b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx @@ -198,17 +198,25 @@ export function SessionReviewPanel(props: { additions: gitSnapshot?.additions ?? 0, deletions: gitSnapshot?.deletions ?? 0, }; + // A directory without a repository is a neutral starting point, not a + // failure: retrying cannot create one, so it takes guidance instead of the + // error Banner. + const sourceFailure = gitResult?.ok === false ? gitResult : null; + const guidance = + sourceFailure?.reason === 'not_git_repository' ? sourceFailure : null; + // A missing workspace can recover (restore or relocate the directory), so it + // keeps its Retry and adds the recovery guidance to the Banner. + const unavailable = + sourceFailure?.reason === 'workspace_unavailable' ? sourceFailure : null; const sourceError = - gitResult?.ok !== false + sourceFailure === null || guidance || unavailable ? null - : gitResult.reason === 'not_git_repository' - ? copy.notGitRepository - : gitResult.reason === 'workspace_unavailable' - ? copy.workspaceUnavailable - : gitResult.reason === 'unborn_repository' - ? copy.unbornRepository - : copy.gitFailed; - const empty = !loading && !error && !sourceError && gitFiles.length === 0; + : sourceFailure.reason === 'unborn_repository' + ? copy.unbornRepository + : copy.gitFailed; + const empty = + !loading && !error && !sourceError && !guidance && !unavailable && + gitFiles.length === 0; return (
) : null} - {/* A source that cannot be read is a failure, not an absence — it takes - the same Banner the load error above does, not an EmptyState. */} + {guidance ? ( + /* Neutral guidance (issue #5940): what Changes needs, the directory + it looked at, and the next step — no Retry, since retrying cannot + create a repository. */ + + } + title={copy.notGitRepository} + description={copy.notGitRepositoryHelp} + /> + {guidance.cwd ? ( + + {copy.taskDirectoryPath(guidance.cwd)} + + ) : null} + + ) : null} + {unavailable ? ( + + {copy.workspaceUnavailableHelp} + {unavailable.cwd ? ( + + {copy.taskDirectoryPath(unavailable.cwd)} + + ) : null} + + } + endContent={ +