From 18f8a9d454947bf1fa31f216b5620199969344f2 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 03:56:54 +0800 Subject: [PATCH 01/24] feat(web-shell): git status chip, visual working-tree diff, and sidebar git status Bring working-tree Git awareness to the Web Shell (browser daemon session UI): - Toolbar branch chip becomes a live status indicator: dirty (staged/unstaged/ untracked), ahead/behind upstream, stash count, detached HEAD, in-progress operation (merge/rebase/cherry-pick/revert/bisect), and conflict count, each with a non-color cue. - Read-only "Changes" dialog: working-tree-vs-HEAD file list with per-file, line-level, per-side syntax-highlighted diffs; opens via /diff or a dirty chip; untracked files expand as fully-added and deleted files still diff. - Per-workspace git status in the sidebar: a compact icon-only chip per trusted workspace (status dot + hover tooltip); click opens that workspace's dialog. All git access goes through the daemon REST API with per-workspace trust gating; new SDK status fields are optional and additive (v2). --- .../2026-07-16-webshell-git-status-diff.md | 889 ++++++++++++++++++ .../2026-07-16-webshell-git-integration.md | 348 +++++++ .../serve/routes/workspace-git-diff.test.ts | 246 +++++ .../src/serve/routes/workspace-git-diff.ts | 201 ++++ packages/cli/src/serve/server.ts | 12 + .../cli/src/serve/workspace-git-state.test.ts | 107 ++- packages/cli/src/serve/workspace-git-state.ts | 50 +- packages/core/src/utils/gitDiff.test.ts | 436 +++++++++ packages/core/src/utils/gitDiff.ts | 366 +++++++ packages/sdk-typescript/scripts/build.js | 4 +- .../sdk-typescript/src/daemon/DaemonClient.ts | 38 + packages/sdk-typescript/src/daemon/index.ts | 5 + packages/sdk-typescript/src/daemon/types.ts | 90 +- packages/sdk-typescript/src/index.ts | 5 + packages/web-shell/client/App.tsx | 97 +- .../client/components/ChatEditor.module.css | 113 +++ .../client/components/ChatEditor.tsx | 14 +- .../components/GitBranchIndicator.test.tsx | 164 +++- .../client/components/GitBranchIndicator.tsx | 228 ++++- .../dialogs/GitDiffDialog.module.css | 155 +++ .../components/dialogs/GitDiffDialog.test.tsx | 204 ++++ .../components/dialogs/GitDiffDialog.tsx | 378 ++++++++ .../components/sidebar/WebShellSidebar.tsx | 7 + .../sidebar/WorkspaceSection.module.css | 19 + .../sidebar/WorkspaceSection.test.tsx | 194 ++++ .../components/sidebar/WorkspaceSection.tsx | 53 ++ .../client/constants/localCommands.ts | 1 + packages/web-shell/client/i18n.tsx | 58 ++ 28 files changed, 4419 insertions(+), 63 deletions(-) create mode 100644 docs/design/2026-07-16-webshell-git-status-diff.md create mode 100644 docs/plans/2026-07-16-webshell-git-integration.md create mode 100644 packages/cli/src/serve/routes/workspace-git-diff.test.ts create mode 100644 packages/cli/src/serve/routes/workspace-git-diff.ts create mode 100644 packages/web-shell/client/components/dialogs/GitDiffDialog.module.css create mode 100644 packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx create mode 100644 packages/web-shell/client/components/dialogs/GitDiffDialog.tsx create mode 100644 packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx diff --git a/docs/design/2026-07-16-webshell-git-status-diff.md b/docs/design/2026-07-16-webshell-git-status-diff.md new file mode 100644 index 00000000000..c6df81e85ff --- /dev/null +++ b/docs/design/2026-07-16-webshell-git-status-diff.md @@ -0,0 +1,889 @@ +# Web Shell git 状态感知与可视化 diff + +## 背景 + +当 workspace 是一个 git 仓库时，Web Shell 目前的 git 集成非常薄，只有两处： + +- 工具栏里的 branch chip（`GitBranchIndicator`），只显示当前分支名。数据来自 + daemon 的 `WorkspaceGitState`，它只追踪 `branch` 一个字段。 +- `/diff` 斜杠命令。在 Web Shell 中它是 ACP 透传，daemon 走非交互路径返回一段 + 纯文本统计（`diffCommand.ts` 的 `renderDiffModelText`）。 + +这意味着用户想确认“工作区干不干净”“有没有 commit 没推”“agent 到底改了哪些行” +时，要么只能看到一个分支名，要么只能读一段终端文本——而 Web Shell 是图形界面， +本应做得比终端更好。 + +core 里其实已经有完整的 git 能力可以复用： + +- `gitDirect.ts`：`resolveBranchName`（直读 `.git/HEAD`，微秒级）、 + `watchRepoBranch`（监听 `/logs/HEAD` reflog）、`readGitHead` + （区分 branch / detached）。 +- `gitDiff.ts`：`fetchGitDiff`（工作区 vs HEAD 的 per-file 统计）、 + `fetchGitDiffHunks`（`Map` 行级 hunk）、`GitDiffResult` / + `PerFileStats` / `GitDiffStats` 类型，以及一组成熟的上限 + （`MAX_FILES=50`、`MAX_DIFF_SIZE_BYTES=1MB`、`MAX_LINES_PER_FILE=400`）和 + transient state（merge/rebase/cherry-pick/revert）检测。 +- `gitUtils.ts`：`getRecentGitStatus` 已经用 `git status --short --branch` + 一次拿到 branch + short status + 最近 5 条 commit，并解析了 branch header。 + +本期目标是在不改变 agent 行为的前提下，把这些已有能力接到 Web Shell UI 上， +分两层落地：第一层增强状态感知（branch chip 旁边显示 dirty / ahead-behind / +stash / detached），第二层提供一个浏览器里的可视化 diff 查看器。 + +## 目标 + +- branch chip 在不打开任何弹窗的情况下，能一眼看出工作区是否干净、相对 + upstream 的 ahead/behind、是否有 stash、是否处于 detached HEAD。 +- 提供一个图形化 diff 查看器：变更文件列表 + 点击展开的单文件行级 diff，复用 + Shiki 做语法高亮。 +- 复用 core 已有的 `fetchGitDiff` / `fetchGitDiffHunks` / `resolveBranchName` + 等能力，不在 Web Shell 里重新实现 git 解析。 +- 兼容旧 daemon、旧 client、非 git 仓库、transient state、detached HEAD 等 + 边界状态，缺失数据时优雅降级而不是报错或空白。 +- 只读优先：所有展示能力都是只读的，不引入任何会改变仓库状态的写操作。 + +## 非目标 + +- 不做提交工作流（stage / commit / 生成 commit message）。属于后续增量。 +- 不做分支管理（切换 / 新建 / 删除分支）。 +- 不做 GitHub 集成（PR / issue / CI checks）。属于后续增量。 +- 不做远程同步（fetch / pull / push）。 +- 不监听整个工作区文件树来实时刷新 dirty 状态。dirty 状态在用户编辑文件后 + 不会逐键实时更新（见“刷新策略”），这是有意的成本取舍。 +- 不改变 `/diff` 在非 Web Shell 客户端（管道、日志、远程 transport）的纯文本 + 输出；那些路径继续走 daemon 的 `renderDiffModelText`。 +- 不为 untracked 文件合成行级 hunk（本期 untracked 只显示“新文件 + 行数”， + 与 CLI `DiffDialog` 行为一致）。 + +## 现状链路 + +### 数据来源（core） + +- `resolveBranchName(cwd)`：直读 `.git/HEAD`，返回分支名或 detached 时的短 + SHA；非仓库返回 `undefined`。微秒级，可放在渲染热路径。 +- `watchRepoBranch(cwd, onChange)`：多个订阅者共享一个对 + `/logs/HEAD` 的 `fs.watch`，在 branch 切换 / commit / reset 时触发。 + 它**不会**因为编辑工作区文件而触发（编辑不写 reflog）。 +- `readGitHead(gitDir)`：返回 `{ type: 'branch' | 'detached', name }`，可用于 + 判断 detached。 +- `fetchGitDiff(cwd)`：返回 `GitDiffResult { stats, perFileStats }`，比较工作区 + 与 HEAD；transient state 或非仓库返回 `null`。 +- `fetchGitDiffHunks(cwd)`：返回 `Map`，内部执行 + `git diff HEAD`。注意 untracked 文件不会出现在 `git diff HEAD` 输出里。 + +### daemon + +- `WorkspaceGitState`（`packages/cli/src/serve/workspace-git-state.ts`）： + 每个 workspace 一个 entry，缓存 `branch`，用 `watchRepoBranch` 监听变化， + 变化时通过 `bridge.publishWorkspaceEvent({ type: 'git_branch_changed', ... })` + 推送。`getStatus()` 当前返回 `{ v: 1, workspaceCwd, branch }`。 +- 路由（`packages/cli/src/serve/routes/workspace-git.ts`）： + `GET /workspace/git`（绑定 workspace）和 + `GET /workspaces/:workspace/git`（带 qualified workspace 参数，需 trusted + runtime）。 +- `/diff` 命令（`packages/cli/src/ui/commands/diffCommand.ts`）：交互模式打开 + Ink 的 `DiffDialog`；非交互 / ACP 返回 `fetchGitDiff` + `buildDiffRenderModel` + - `renderDiffModelText` 的纯文本。 + +### SDK / webui + +- `DaemonWorkspaceGitStatus` 类型 + `DaemonClient.workspaceGit()` + （`GET /workspace/git`）。 +- 事件 `git_branch_changed` 在 `sdk-typescript/src/daemon/events.ts` 注册。 +- `webui/src/daemon/session/mappers.ts` 把 `git_branch_changed` 映射到 + `connection.gitBranch`（并用 `workspaceCwd` 做了归属校验）。 + +### Web Shell + +- `GitBranchIndicator.tsx`：纯展示 chip（branch 名 + tooltip）。 +- `ChatEditor.tsx`：把 `gitBranch` 作为一个 toolbar action 渲染 + （`gitBranchVisible`、compact / expanded 两种形态）。 +- `App.tsx`： + - `connection.gitBranch`（来自 SSE `git_branch_changed`）驱动会话内的 chip。 + - `selectedWorkspaceGitBranch`：在选择 workspace 但**尚未连接 session** 时， + 通过 `workspace.client.workspaceByCwd(cwd).workspaceGit()` 拉取一次分支做 + 预览。 + - `activePanel` 机制统一管理各类弹窗（settings / status / sessions / + extensions / plugins 等），`components/dialogs/*.tsx` + 同名 + `.module.css` 是标准弹窗形态。 +- `customization.tsx`：markdown 代码块用 Shiki 高亮 + （`WebShellCodeBlockRenderInfo.resolvedLanguage` 是规范化后的 Shiki language + id），可作为 diff 行内语法高亮的复用基础。 +- `constants/localCommands.ts`：`getLocalCommands(t)` 定义本地斜杠命令补全。 + +## 方案概述 + +整体数据流沿用现有 branch chip 的形态，向下扩展 core / daemon / SDK，向上扩展 +Web Shell 组件： + +```text +core (gitDirect + gitDiff) + ├─ getGitWorkingTreeStatus(cwd) [新增] dirty / ahead / behind / stash / detached + ├─ fetchGitDiff(cwd) [复用] 文件列表 + 统计 + └─ fetchGitDiffHunksForFile(cwd, path) [新增] 单文件行级 hunk + │ + ▼ +daemon (serve) + ├─ WorkspaceGitState.getStatus() [扩展] 返回 enriched status + ├─ GET /workspace/git [扩展] 携带 enriched 字段 + ├─ GET /workspace/git/diff [新增] 文件列表 + 统计 + └─ GET /workspace/git/diff/file [新增] 单文件 hunk（按需） + │ + ▼ +SDK (DaemonClient + types + events) + ├─ DaemonWorkspaceGitStatus [扩展] 新字段（可选，向后兼容） + ├─ DaemonWorkspaceGitDiff / ...File [新增] + ├─ workspaceGitDiff() / workspaceGitDiffFile(path) [新增] + └─ git_status_changed [新增事件，可选] + │ + ▼ +webui (mappers) + └─ git_status_changed → connection.gitStatus [新增] + │ + ▼ +Web Shell (client) + ├─ GitBranchIndicator [扩展] dirty 点 / ahead-behind / stash / detached + └─ GitDiffDialog [新增] 文件列表 + 单文件行级 diff（Shiki 高亮） +``` + +两层共享同一组 daemon git 路由：第一层用 `GET /workspace/git`（enriched）， +第二层用 `GET /workspace/git/diff` 与 `.../diff/file`。dirty 指示点可点击， +点击直接打开 `GitDiffDialog`，把两层串起来。 + +## UI 草图（Before / After） + +落地后页面只变两处：输入框工具栏的 branch chip（第一层）和一个新的 Changes +弹窗（第二层）。 + +### 第一层：branch chip + +chip 仍在输入框左下角工具栏（位置不变），信息更丰富，有 compact / expanded +两种形态（工具栏空间够时自动展开，沿用现有 toolbar 测量逻辑）。 + +现在（只有分支名）： + +```text +┌─────────────────────────────────────────────┐ +│ [⑂ main] [@ ⏎ 发送] │ ← 输入框工具栏 +└─────────────────────────────────────────────┘ +``` + +之后 · compact（空间不够时）： + +```text + [⑂ main •] + └─ dirty 小圆点：有未提交改动时出现 +``` + +之后 · expanded（空间够时，显示完整状态）： + +```text + [⑂ main • ↑2 ↓1 ⧉3] + │ │ │ │ └─ stash 数量（3 个 stash） + │ │ │ └───── behind：落后 upstream 1 个 commit + │ │ └───────── ahead：领先 upstream 2 个 commit + │ └──────────── dirty 点 + └──────────────── 分支名 +``` + +几种特殊状态： + +```text + [⑂ main] 干净时没有 dirty 点 + [⑂ a1b2c3d ⚠] detached HEAD：chip 变警告色，显示短 SHA + [⑂ feature ↑2] 无 upstream 时不显示 ↓，只有 ahead +``` + +交互： + +- 悬停 → tooltip 显示完整说明（如 `main · 3 个未提交改动 · 领先 2 / 落后 1`）。 +- 点击 dirty 点 → 直接打开第二层的 Changes 弹窗。 + +### 第二层：Changes 弹窗 + +点 chip 或输入 `/diff` 打开。它是一个覆盖整个聊天区的浮层（与 `/status`、 +`/settings` 同一种 `activePanel` 形态，约 70vh 可滚动）： + +```text +┌─ Changes ───────────────────────────── vs HEAD ─ ✕ ┐ +│ │ +│ 4 files changed, +128 / -37 │ ← 汇总 header +│ │ +│ +12 -3 src/services/foo.ts │ ← 文件行（点开前） +│ +88 -0 src/components/Bar.tsx (new) │ +│ ~ assets/logo.png (binary) │ +│ +28 -34 legacy/old.ts (deleted) │ +│ │ +│ ▼ src/services/foo.ts │ ← 点开后：行级 diff +│ ┌──────────────────────────────────────────────────┐ │ +│ │ 11 const config = load(); │ │ ← 上下文行（灰） +│ │ 12 - const timeout = 2000; │ │ ← 删除行（红底） +│ │ 12 + const timeout = 5000; │ │ ← 新增行（绿底） +│ │ 13 + const retries = 3; │ │ +│ │ 14 export { config, timeout }; │ │ +│ └──────────────────────────────────────────────────┘ │ +│ │ +│ …and 2 more (showing first 50) │ ← 超上限时的截断提示 +└──────────────────────────────────────────────────────┘ +``` + +交互细节： + +- 文件行默认折叠，只显示 `+A -R 路径` 和标记（`(new)` / `(binary)` / + `(deleted)`）。 +- 点文件行 → 按需拉该文件的 hunk 并展开（不一次性加载全部，省流量）。 +- 行级 diff 用 Shiki 语法高亮（按扩展名识别语言），`+` / `-` 行分别绿 / 红底 + 着色，与现有 markdown 代码块同一套高亮器。 +- `(binary)` / `(new)` 文件不可展开（无 hunk），与 CLI `DiffDialog` 行为一致。 +- 非 git 仓库 / merge 进行中 → 弹窗显示占位文案（如“当前不是 git 仓库，或正在 + merge / rebase”），不报错。 + +## 数据结构 + +### 第一层：enriched git status + +扩展 `WorkspaceGitStatus`（daemon 端）与 `DaemonWorkspaceGitStatus`（SDK 端）。 +新字段全部可选，`v` 升到 `2`，保证旧 daemon / 旧 client 互相兼容： + +```ts +interface DaemonWorkspaceGitStatus { + v: 1 | 2; + workspaceCwd: string; + branch: string | null; + + // —— v2 新增字段，全部可选 —— + /** true 表示 detached HEAD（branch 此时为短 SHA）。 */ + detached?: boolean; + /** 已暂存文件数（porcelain X 列非 '.'）。 */ + staged?: number; + /** 已修改未暂存文件数（porcelain Y 列非 '.'）。 */ + unstaged?: number; + /** 未跟踪文件数（'??'）。 */ + untracked?: number; + /** 冲突（unmerged）文件数。 */ + conflicted?: number; + /** 是否配置了 upstream。 */ + hasUpstream?: boolean; + /** 领先 upstream 的 commit 数。 */ + ahead?: number; + /** 落后 upstream 的 commit 数。 */ + behind?: number; + /** stash 数量。 */ + stashCount?: number; + /** 进行中的操作（merge/rebase/cherry-pick/revert/bisect）。 */ + operation?: 'merge' | 'rebase' | 'cherry-pick' | 'revert' | 'bisect'; + /** 重字段（dirty/ahead/behind/stash）的计算时间戳（epoch ms），用于新鲜度判断。 */ + computedAt?: number; +} +``` + +派生信号（前端计算，不进 wire format）：`dirty = staged + unstaged + untracked > 0`。 + +> **transient state 处理变更**：早期设计让 `getGitWorkingTreeStatus` 在 +> merge/rebase 期间返回 `null`。Phase 1 决定把"进行中操作"显式 surfaced，因此 +> 改为：transient 期间仍返回状态，并通过 `operation` 字段标记操作类型 +> （`git status` 在这些状态下仍能正常输出）。返回 `null` 只保留给"非仓库 / +> git 失败"。`fetchGitDiff`（第二层）仍在 transient 时返回 null，二者语义不同。 + +### 第二层：diff payload + +文件列表与单文件 hunk 分两个路由，避免一次性把多文件 diff（最坏 +`MAX_FILES × MAX_DIFF_SIZE_BYTES`）塞进单个响应： + +```ts +interface DaemonWorkspaceGitDiffFile { + /** 仓库根相对路径，未净化，渲染前必须 sanitize。 */ + path: string; + /** 二进制文件为 undefined。 */ + added?: number; + removed?: number; + isBinary: boolean; + isUntracked: boolean; + isDeleted: boolean; + /** untracked 文本文件超过读取上限时为 true（added 为下界）。 */ + truncated: boolean; +} + +interface DaemonWorkspaceGitDiff { + v: 1; + workspaceCwd: string; + /** false 表示非仓库 / HEAD 缺失 / transient state，前端显示占位。 */ + available: boolean; + filesCount: number; + linesAdded: number; + linesRemoved: number; + files: DaemonWorkspaceGitDiffFile[]; + /** filesCount - files.length，per-file 上限截断时的剩余数。 */ + hiddenCount: number; +} + +/** 与 `diff` 库的 Hunk 字段对齐，序列化后传输。 */ +interface DaemonDiffHunk { + oldStart: number; + oldLines: number; + newStart: number; + newLines: number; + lines: string[]; // 带 ' ' / '+' / '-' 前缀 +} + +interface DaemonWorkspaceGitDiffHunks { + v: 1; + workspaceCwd: string; + path: string; + available: boolean; // false: 该文件无 hunk（untracked / 无变化 / 越界） + hunks: DaemonDiffHunk[]; +} +``` + +`DaemonDiffHunk` 直接对应 core 的 `GitDiffHunk`（即 `diff` 库的 `Hunk`）， +daemon 端只需把 `Map` 里对应文件的 hunk 数组序列化即可，前端不需要依赖 `diff` +库。 + +## 关键修改点 + +### 1. core：新增工作区状态与单文件 hunk + +两个函数都放在 `packages/core/src/utils/gitDiff.ts`，以便直接复用该文件内已有 +的 `findGitRoot`、`isInTransientGitState`（当前未导出）、`parseGitDiff` 等私有 +/ 公有构件，避免跨文件暴露内部函数： + +- `getGitWorkingTreeStatus(cwd): Promise` + - 复用 `findGitRoot` 判断是否仓库、`isInTransientGitState` 判断 transient + state（与 `fetchGitDiff` 一致），非仓库 / transient 返回 `null`。 + - 一次 `git --no-optional-locks status --porcelain=v1 --branch -z` 调用， + 解析 branch header（branch / detached / `...upstream` / `[ahead N, behind +M]`）和 porcelain 行（统计 staged / unstaged / untracked）。解析逻辑可参考 + `getRecentGitStatus` 已有的 branch header 处理。 + - stash 数量：优先直读 `/logs/refs/stash` 行数（与 `gitDirect.ts` + 的直读哲学一致，避免第二个子进程）；读不到则记 0。 + - detached 由 `readGitHead` 或 branch header（`HEAD (no branch)` / + `No commits yet`）判定。 + - 返回结构对齐 `DaemonWorkspaceGitStatus` 的 v2 字段。 +- `fetchGitDiffHunksForFile(cwd, filePath): Promise` + - 执行 `git --no-optional-locks diff --no-ext-diff --no-textconv HEAD -- +`，复用 `parseGitDiff` 取该文件的 hunk 数组。 + - 与 `fetchGitDiffHunks` 一样传 `--no-ext-diff` / `--no-textconv`，避免 + `GIT_EXTERNAL_DIFF` / textconv 在只读路径上执行用户命令。 + - 单文件调用，天然受 `MAX_DIFF_SIZE_BYTES` 约束，响应体积可控。 + +两个函数都需要对 `filePath` 做校验：拒绝绝对路径、拒绝以 `/` 开头、拒绝包含 +`..` 越界段的 path，确保只把它当作仓库根相对路径传给 git。 + +### 2. daemon：扩展 status + 新增 diff 路由 + +- `WorkspaceGitState.getStatus()`：在原有 `branch` 基础上调用 + `getGitWorkingTreeStatus`，合并出 enriched `WorkspaceGitStatus`（v2）。 + `branch` 仍走 `resolveBranchName` 的缓存 + `watchRepoBranch`；重字段 + （dirty/ahead/behind/stash）每次 `getStatus` 现算（调用频率受“刷新策略” + 约束，见下文），不长期缓存以免 stale。 +- `routes/workspace-git.ts`： + - `GET /workspace/git` / `GET /workspaces/:workspace/git` 返回 enriched 结果 + （路由签名不变，只是 payload 字段增多）。 + - 新增 `GET /workspace/git/diff` 与 `GET /workspace/git/diff/file?path=...`， + 以及对应的 qualified 版本 `/workspaces/:workspace/git/diff[/file]`，复用 + `requireTrustedWorkspaceRuntime` / `resolveWorkspaceRuntimeFromParam` 的 + trusted 校验。 + - diff 路由内部调用 `fetchGitDiff` / `fetchGitDiffHunksForFile`，把结果映射成 + `DaemonWorkspaceGitDiff` / `DaemonWorkspaceGitDiffHunks`。`path` 查询参数 + 必须经过第 1 步的校验后才能传给 git。 + +### 3. SDK：类型 + client 方法 + 事件 + +- `DaemonWorkspaceGitStatus` 增加 v2 可选字段（如上）。 +- 新增 `DaemonWorkspaceGitDiff` / `DaemonWorkspaceGitDiffFile` / + `DaemonWorkspaceGitDiffHunks` / `DaemonDiffHunk` 类型，从 + `sdk-typescript/src/index.ts` 与 `src/daemon/index.ts` 导出。 +- `DaemonClient` 新增： + - `workspaceGitDiff(): Promise` + - `workspaceGitDiffFile(path: string): Promise` + （`path` 作为 query 参数需 `urlEncode`，对齐现有 `workspaceMcpTools` 等 + 方法的写法）。 +- 事件：可选新增 `git_status_changed`（携带 enriched status）。本期更倾向于 + **不新增推送事件**，而是复用现有 `git_branch_changed` 作为“需要重新拉取 + status”的信号——见“刷新策略”。是否新增 `git_status_changed` 留作实施时权衡， + 默认不加以缩小 PR 面积。 + +### 4. webui：connection 状态 + +- 若采用“复用 `git_branch_changed` 触发重拉”方案：`mappers.ts` 无需改动， + Web Shell 在收到 `connection.gitBranch` 变化时重新调用 `workspaceGit()`。 +- 若后续新增 `git_status_changed`：在 `mappers.ts` 增加一个 case，写入 + `connection.gitStatus`（新增可选字段），并做与 `git_branch_changed` 相同的 + `workspaceCwd` 归属校验。 + +### 5. Web Shell：增强 chip + 新增 diff 弹窗 + +- `GitBranchIndicator` 扩展： + - 入参从 `branch` 扩展为接收 enriched status（dirty / ahead / behind / + stashCount / detached）。 + - compact 形态：branch 名 + dirty 小圆点（有任一变更时显示）。 + - expanded 形态：追加 `↑N`（ahead）`↓M`（behind）、stash 角标；detached 时 + chip 变色并显示短 SHA。 + - chip 可点击：dirty 时点击打开 `GitDiffDialog`；其余情况可打开一个轻量 + status popover（或直接复用 diff 弹窗的 header）。复用现有 + `useWebShellPortalRoot()` 挂载 popover，保留 `data-web-shell-git-branch` + 属性。 +- 新增 `components/dialogs/GitDiffDialog.tsx`（+ `.module.css`），对齐 + `DaemonStatusDialog` 的形态： + - 打开时调用 `workspaceGitDiff()` 拉文件列表 + 统计；展示 header + （`N files changed, +A / -R`）和文件行（`+A -R 文件名`，binary / untracked / + deleted 标记，复用 `diffCommand.ts` 的列布局语义）。 + - 点击文件行按需调用 `workspaceGitDiffFile(path)` 拉 hunk，展开为统一 diff + （`+`/`-`/` ` 行着色），行内语法高亮复用 Shiki（按文件扩展名解析 language）。 + - 文件名渲染前必须 sanitize（参考 `sanitizeFilenameForDisplay` 的语义）， + 防止 git 允许的原始控制字节 / 转义注入。 + - `available === false` 时显示占位文案（非仓库 / HEAD 缺失 / transient + state），对齐 `diffCommand.ts` 的提示语义。 + - 通过 `activePanel` 机制注册（新增一个 panel 值，如 `'diff'`），复用现有 + 打开 / 关闭 / 焦点管理逻辑。 +- `/diff` 命令本地化：在 Web Shell 中把 `/diff` 从 ACP 透传改为本地实现—— + 打开 `GitDiffDialog`（对齐 CLI 交互模式打开 `DiffDialog` 的行为）。在 + `App.tsx` 的命令分发处识别 `/diff` 并 `setActivePanel('diff')`，不再发给 + daemon。`getLocalCommands` 中补 `diff` 的补全项与 `local.diff` 文案。 + +### 6. 刷新策略（第一层的新鲜度） + +- branch：保持现状，`watchRepoBranch` 经 `git_branch_changed` 实时推送，热路径 + 直读，零额外成本。 +- 重字段（dirty / ahead / behind / stash）在以下时机重新拉取 + `workspaceGit()`： + 1. 用户打开 status popover 或 `GitDiffDialog` 时（按需，权威）。 + 2. 收到 `git_branch_changed`（commit / reset / 切分支都会同时改变这些值）。 + 3. 标签页 `visibilitychange` 重新可见时。 + 4. 仅对**当前选中 / 可见的 workspace**做一次低速轮询（如 15s，可配置）， + 保证 dirty 点在编辑后“足够新”。**不**对所有 workspace 轮询。 +- 明确取舍：不对工作区文件树建立 watcher，dirty 不会逐键实时刷新。这是为了 + 避免昂贵的全树监听；对“编辑后立刻想看 dirty”的场景，focus / 轮询 / 打开弹窗 + 都能覆盖。 + +### 7. i18n + +新增文案需同时提供 en 与 zh-CN（`i18n.tsx`）：chip 的 dirty / ahead / behind / +stash / detached 的 aria-label 与 tooltip、`GitDiffDialog` 的 header / 列标记 / +占位文案、`local.diff` 补全描述。复用 `git.currentBranch` 既有 key 的命名风格。 + +## 兼容性 + +- 旧 daemon（v1）只返回 `{ v, workspaceCwd, branch }`：新 client 把缺失的 v2 + 字段当作“未知”，chip 退化为当前的纯 branch 显示，不显示 dirty/ahead 等。 +- 旧 client 读到 v2 payload：只认 `branch`，忽略多余字段，行为不变。 +- 非 git 仓库 / detached / transient state：`getGitWorkingTreeStatus` 与 + `fetchGitDiff` 返回 null，前端显示占位或隐藏重字段，不报错。 +- `git_branch_changed` 仍保留，不破坏现有 branch chip 链路。 +- `/diff` 在非 Web Shell 客户端的纯文本输出不变（daemon `renderDiffModelText` + 路径不动）。 +- diff payload 受 core 既有上限约束（`MAX_FILES` / `MAX_DIFF_SIZE_BYTES` / + `MAX_LINES_PER_FILE`），大 diff 通过 `hiddenCount` 与单文件按需加载控制体积。 + +## 测试计划 + +### Unit tests + +- `getGitWorkingTreeStatus`：clean / dirty（staged、unstaged、untracked 混合）/ + detached / 有 upstream 的 ahead-behind / 无 upstream / transient state / + 非仓库各分支；branch header 解析正确。 +- `fetchGitDiffHunksForFile`：单文件有变化 / 无变化 / untracked 返回空 / + 非法 path（绝对路径、`..` 越界）被拒绝；`--no-ext-diff` / `--no-textconv` + 被传入。 +- `WorkspaceGitState.getStatus`：返回 enriched 结构，branch 仍来自缓存、 + 重字段来自 `getGitWorkingTreeStatus`。 +- diff 路由：`GET /workspace/git/diff` 把 `fetchGitDiff` 结果映射为 + `DaemonWorkspaceGitDiff`；`.../diff/file` 校验 `path` 并映射 hunk； + qualified 路由复用 trusted 校验（参考 `workspace-git.test.ts` 现有用例）。 +- `DaemonClient.workspaceGitDiff` / `workspaceGitDiffFile`：正确拼接 URL、 + `path` 经过 `urlEncode`。 +- `GitBranchIndicator`：dirty 点显示 / 隐藏；ahead-behind 渲染；detached 文案； + compact / expanded 形态；可点击 aria。 +- `GitDiffDialog`：文件列表渲染（binary / untracked / deleted 标记）；点击展开 + 按需拉 hunk；`available === false` 占位；文件名 sanitize；hunk 行着色。 +- `/diff` 本地化：`App.tsx` 收到 `/diff` 时 `setActivePanel('diff')` 而非透传 + daemon（参考 `App.test.tsx` 现有 panel 分支用例）。 + +### Integration / browser verification + +- 在干净仓库 / 有改动仓库 / detached / 无 upstream 仓库下，chip 显示符合预期。 +- 编辑文件后，focus 或打开弹窗时 dirty 点出现；commit 后 `git_branch_changed` + 触发 chip 更新。 +- 打开 `GitDiffDialog`：文件列表正确，点击文件展开行级 diff，Shiki 高亮正常， + 大文件 / 多文件被上限截断时有 `hiddenCount` 提示。 +- 非 git 目录下打开 `/diff` 显示占位文案而非报错。 + +## 风险和控制 + +- 风险：重字段每次 `getStatus` 现算会在多 workspace 下放大 `git status` 子进程 + 成本。控制：只对当前可见 workspace 低速轮询，其余按需（打开弹窗 / focus）才 + 拉取；branch 始终走直读，不进子进程。 +- 风险：dirty 不实时（编辑后不逐键刷新）可能让用户困惑。控制：在 chip tooltip + 或 popover 注明“点击刷新 / 数据为最近一次快照”，并保证打开弹窗时取权威值。 +- 风险：`path` 查询参数来自前端，可能构造越界路径。控制：daemon 与 core 两层 + 都校验（拒绝绝对路径 / `..` 越界段），且最终由 git 限定在仓库内。 +- 风险：git 允许的原始控制字节 / 转义进入文件名，造成渲染注入。控制：渲染前 + 统一 sanitize，复用 `sanitizeFilenameForDisplay` 语义。 +- 风险：跨包新增类型扩大 PR 面积。控制：diff hunk 用最小 `DaemonDiffHunk` + 结构，不让 SDK 反向依赖 `diff` 库或 Web Shell client 类型；默认不新增 + `git_status_changed` 事件以缩小改动面。 +- 风险：Shiki 对部分语言 / 大文件高亮有性能成本。控制：仅对展开的单个文件做 + 高亮，且受 `MAX_LINES_PER_FILE` 约束；流式无关，无需 debounce。 + +## Phase 1 详细实施计划（含进度） + +**目标**：branch chip 从“只显示分支名”升级为实时状态条——显示 dirty / +ahead-behind / stash / detached / **operation**（merge/rebase/…）/ **conflicted** +（冲突数）。纯增量、只读、向后兼容；不动 branch 显示路径。 + +**数据流**： + +```text +core getGitWorkingTreeStatus(cwd) + → daemon WorkspaceGitState.getStatus() (WorkspaceGitStatus v2) + → GET /workspace/git / GET /workspaces/:workspace/git + → SDK DaemonWorkspaceGitStatus (DaemonClient.workspaceGit()) + → webui connection（git_branch_changed 仍驱动 branch；重字段走 REST） + → Web Shell gitStatus 状态 → ChatEditor → GitBranchIndicator +``` + +**文件清单**： + +| 操作 | 文件 | 说明 | +| ---- | -------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 修改 | `packages/core/src/utils/gitDiff.ts` | `GitOperation` / `GitWorkingTreeStatus` / `getGitWorkingTreeStatus` / `parseStatusBranchLine` / `parseStatusEntries` / `detectGitOperation` / `countStashEntries` | +| 修改 | `packages/core/src/utils/gitDiff.test.ts` | 解析单测 + 真实仓库集成测试 | +| 修改 | `packages/cli/src/serve/workspace-git-state.ts` | `WorkspaceGitStatus` v2 + `getStatus` 合并 enriched | +| 修改 | `packages/cli/src/serve/workspace-git-state.test.ts` | mock + v2 断言 + enriched/operation 用例 | +| 修改 | `packages/sdk-typescript/src/daemon/...`（`DaemonWorkspaceGitStatus` 声明处） | 加 v2 可选字段 | +| 修改 | `packages/web-shell/client/components/GitBranchIndicator.tsx`（+ `.module.css`） | 渲染 dirty 点 / ↑N↓M / stash / detached / operation 徽标 / conflicted | +| 修改 | `packages/web-shell/client/components/ChatEditor.tsx` | 透传 enriched status | +| 修改 | `packages/web-shell/client/App.tsx` | `gitStatus` 状态 + 刷新策略 | +| 修改 | `packages/web-shell/client/i18n.tsx` | en + zh-CN 文案 | +| 修改 | `packages/web-shell/client/components/GitBranchIndicator.test.tsx` | 各状态渲染用例 | + +### Task 1 · core ✅ 已完成 + +- [x] `GitOperation` 类型 + `GitWorkingTreeStatus`（含 `conflicted` / `operation`） +- [x] `getGitWorkingTreeStatus`：`git status --porcelain=v1 --branch -z` 一次调用 + - 直读 stash reflog + `detectGitOperation`；transient 不再返回 null +- [x] `parseStatusBranchLine` / `parseStatusEntries`（含 unmerged → conflicted） +- [x] 单测：解析 + clean/dirty/detached/stash/ahead-behind/非仓库/merge/rebase/ + cherry-pick（**79 个通过**） + +### Task 2 · daemon ✅ 已完成 + +- [x] `WorkspaceGitStatus` 升级到 v2 + enriched 可选字段（dirty 部分） +- [x] `getStatus` 合并 `getGitWorkingTreeStatus`（branch 用 watcher 缓存，重字段 + 每次现算） +- [x] 接口加 `operation?` / `conflicted?`；`getStatus` 透传这两个字段 +- [x] 测试：enriched 输出用例补 `operation` / `conflicted`（**14 个通过**） + +### Task 3 · SDK ✅ 已完成 + +- [x] `DaemonWorkspaceGitStatus` 加 v2 可选字段（`detached/staged/unstaged/ +untracked/conflicted/hasUpstream/ahead/behind/stashCount/operation/ +computedAt`），`v: 1 | 2`；新增 `DaemonGitOperation` 类型并从 + `src/index.ts` / `src/daemon/index.ts` 导出 +- [x] `workspaceGit()` 无需改（返回更多字段即可） +- [x] SDK 类型单测（v1 mock）仍通过 + +### Task 4 · Web Shell ✅ 已完成 + +- [x] `GitBranchIndicator` 入参从 `branch` 扩展为接收 enriched status；渲染： + dirty 点、`↑N`/`↓M`、stash 角标、detached 换图标、**operation 徽标** + （`REBASING`/`MERGING`/…）、conflicted 数（**非颜色兜底**：形状+数字） +- [x] compact（图标角标）/ expanded 两种形态；保留 `data-web-shell-git-branch` +- [x] `App.tsx` 新增 `gitStatus` 状态，经 `workspaceGit()` 拉取；branch 仍用 + `connection.gitBranch`（SSE，实时） +- [x] 刷新策略（focus + branch 变化 + 仅当前 workspace 30s 可见性轮询） +- [x] `ChatEditor` 透传 enriched status 到 `GitBranchIndicator` +- [x] i18n（en + zh-CN）：aria-label / tooltip / operation 文案 +- [x] `GitBranchIndicator.test.tsx` 补各状态用例（**8 个通过**） + +**刷新策略**（第一层新鲜度）： + +- branch：保持现状，`git_branch_changed`（reflog watch）实时推送。 +- 重字段（dirty/ahead/behind/stash/operation/conflicted）在以下时机重拉 + `workspaceGit()`：① 打开 status popover / diff 弹窗；② 收到 + `git_branch_changed`；③ 标签页 `visibilitychange` 重新可见；④ 仅对**当前活跃 + workspace** 低速轮询（如 15s，可配置）。 +- 不对工作区文件树建 watcher，dirty 不逐键实时（成本取舍）。 + +### Task 5 · 验证 ✅ 已完成 + +- [x] `npm run build && npm run typecheck`（全仓通过） +- [x] `npm run lint`（改动文件全过） +- [x] 单测：core 79 / cli 14 / sdk 1 / web-shell GitBranchIndicator 8 + + ChatEditor&App 118 +- [ ] 浏览器验收（留待 PR 前补）：干净 / dirty / detached / 无 upstream / + rebase 中 各状态 chip 显示正确；focus / 打开弹窗后 dirty 点刷新 + +> Phase 1 已提交于分支 `feat/webshell-git-status-chip`。 + +--- + +## Phase 2 详细实施计划（含进度） + +**目标**：新增只读的「Changes」弹窗——文件列表（工作区 vs HEAD）+ 点开按需 +加载单文件行级 diff（Shiki 高亮）。`/diff` 从 ACP 透传改为本地打开该弹窗， +dirty chip 点击联动。纯增量、只读、向后兼容。 + +**数据流**： + +```text +core fetchGitDiff(cwd) / fetchGitDiffHunksForFile(cwd, path) + → daemon GET /workspace/git/diff (列表 + 统计) + GET /workspace/git/diff/file?path= (单文件 hunk) + （+ qualified /workspaces/:workspace/git/diff[/file]） + → SDK DaemonWorkspaceGitDiff / DaemonWorkspaceGitDiffHunks + + DaemonClient.workspaceGitDiff() / workspaceGitDiffFile(path) + → Web Shell GitDiffDialog（列表 → 懒加载单文件 hunk → Shiki 高亮） +``` + +**调研修正（与早期草案的差异，重要）**： + +- **daemon 是 express，不用 zod**：query 参数用手写助手 `requireStringQuery` / + `parseIntInRange`（参考 `routes/workspace-file-read.ts`），不要引入 zod schema。 +- **路径安全（实施修正：单文件 diff 路由不走 fs factory）**：早期草案要求 + `?path=` 经 `factory.forRequest(...).resolve(path, 'read')` 沙箱化。实施时 + 发现 `'read'` 意图会拒绝**工作区已删除的文件**（ENOENT），而这类文件仍在 + HEAD 中、必须能 diff，故单文件 diff 路由**不**经 fs factory。改由四层纵深 + 约束：(1) qualified 路由要求 trusted workspace；(2) core + `fetchGitDiffHunksForFile` 把 path 规范化为 repo-relative，拒绝绝对路径 / + 盘符 / `..` 越界；(3) git 只在仓库内 diff，untracked 合成读取用 + `O_NOFOLLOW`，且仅对 `ls-files --others` 确认为 untracked 的路径执行；(4) + 路由只读。详见 `routes/workspace-git-diff.ts` 顶部注释。 +- **读路由头**：复用 `applyReadHeaders(res)`（`no-store` + `nosniff`）。 +- **错误**：git 业务用 `sendBridgeError`，trust/解析失败用 runtime 助手 + （已自动发响应）；缺 `path` query 返回 `400 parse_error`。 +- **Shiki 已有封装**：复用 `components/messages/codeHighlighter.ts` + （`getCodeHighlighter` / `highlightToHtmlSync` / `isTooLargeToHighlight`）， + 不新接 highlighter。 +- **`virtual-viewport` 在代码库中不存在**（早期文档引用的概念未落地）。大 diff + 靠 core 既有上限（`MAX_FILES=50` / `MAX_LINES_PER_FILE=400` / + `MAX_DIFF_SIZE_BYTES=1MB`）+ **单文件懒加载**控制 DOM 规模；本期不引入虚拟 + 滚动（400 行内 DOM 可承受），如后续需要再单独立项。 +- **弹窗形态**：用 `components/ui/dialog`（Radix Dialog + `useWebShellPortalRoot`， + 对齐 `McpManagerPage`），`DialogContent` 覆盖 className 加宽；经 + `showGitDiffDialog` 状态标志开关并纳入 `dialogOpen` 聚合（`App.tsx:2739`）。 +- **`/diff` 本地化**：当前 `/diff` 是 ACP/agent 命令（serve 无对应路由）。本期 + 在 `App.tsx` 命令分发处拦截 `/diff` → 打开弹窗（不发给 daemon），并在 + `getLocalCommands` 补 `diff` 补全项（`local.diff` 文案已存在）。 + +**文件清单**： + +| 操作 | 文件 | 说明 | +| ---- | ----------------------------------------------------------------------------------- | -------------------------------------------------------------------- | +| 修改 | `packages/core/src/utils/gitDiff.ts` | 新增 `fetchGitDiffHunksForFile(cwd, path)`（单文件 hunk） | +| 修改 | `packages/core/src/utils/gitDiff.test.ts` | 单文件 hunk 真实仓库用例 | +| 新增 | `packages/cli/src/serve/routes/workspace-git-diff.ts` | 两个 GET 路由（bound + qualified） | +| 修改 | `packages/cli/src/serve/server.ts` | 注册新路由（import + 两处 register 调用） | +| 新增 | `packages/cli/src/serve/routes/workspace-git-diff.test.ts` | 路由单测（含 path 越界拒绝） | +| 修改 | `packages/sdk-typescript/src/daemon/types.ts` | `DaemonWorkspaceGitDiff` / `...File` / `...Hunks` / `DaemonDiffHunk` | +| 修改 | `packages/sdk-typescript/src/daemon/index.ts` + `src/index.ts` | 导出新类型 | +| 修改 | `packages/sdk-typescript/src/daemon/DaemonClient.ts` | `workspaceGitDiff()` / `workspaceGitDiffFile(path)` | +| 新增 | `packages/web-shell/client/components/dialogs/GitDiffDialog.tsx`（+ `.module.css`） | 文件列表 + 单文件 hunk 渲染 + Shiki 高亮 | +| 修改 | `packages/web-shell/client/components/GitBranchIndicator.tsx` | dirty chip 可点击（`onOpenDiff` 回调） | +| 修改 | `packages/web-shell/client/components/ChatEditor.tsx` | 透传 `onOpenGitDiff` 回调 | +| 修改 | `packages/web-shell/client/App.tsx` | `showGitDiffDialog` 状态 + 渲染弹窗 + `/diff` 本地拦截 | +| 修改 | `packages/web-shell/client/constants/localCommands.ts` | `diff` 补全项 | +| 修改 | `packages/web-shell/client/i18n.tsx` | 弹窗文案（en + zh-CN） | +| 新增 | `packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx` | 列表 / hunk / 占位 / 越界用例 | + +### Task 1 · core ✅ 已完成 + +- [x] `fetchGitDiffHunksForFile(cwd, filePath): Promise` + - `git --no-optional-locks diff --no-ext-diff --no-textconv HEAD -- `， + 复用 `parseGitDiff` 取该文件 hunk 数组。 + - 与 `fetchGitDiffHunks` 一致传 `--no-ext-diff` / `--no-textconv`。 + - repo-relative 校验（`toRepoRelativePath`）：拒绝绝对路径 / 盘符 / `..` + 越界段（纵深防御）。 + - **untracked 全新增**（`synthesizeUntrackedHunk`）：`git diff HEAD` 无输出 + 且 `ls-files --others` 确为 untracked 时，`O_NOFOLLOW` 读取文件内容合成单个 + 全新增 hunk（受 `MAX_LINES_PER_FILE` / `MAX_DIFF_SIZE_BYTES` 约束；二进制 + 按既有语义）。 + - 非仓库 / transient / 该文件无变化（且非 untracked）→ 返回 `null`。 +- [x] 单测：真实仓库改动单文件、未改文件、**untracked 文件全新增**、越界 path + （`gitDiff.test.ts` 共 88 用例通过）。 + +### Task 2 · daemon ✅ 已完成 + +- [x] 新建 `routes/workspace-git-diff.ts`，导出 + `registerWorkspaceGitDiffRoutes` + `registerWorkspaceQualifiedGitDiffRoutes`。 +- [x] `GET /workspace/git/diff`：调 `fetchGitDiff` → 映射 `DaemonWorkspaceGitDiff` + （files 列表 + 统计 + `hiddenCount`）；`available` 反映 null（非仓库/ + transient）。 +- [x] `GET /workspace/git/diff/file?path=`：校验 `req.query['path']`（缺则 + `400 parse_error`）→ `fetchGitDiffHunksForFile` → 映射 + `DaemonWorkspaceGitDiffHunks`；`applyReadHeaders`；错误 `sendBridgeError`。 + **不经 fs factory**（`'read'` 意图会拒绝已删除文件），改由 trust gate + + core repo-relative 规范化 + git 仓库内含 + `O_NOFOLLOW` + `ls-files` + gate 四层约束（见「调研修正」与路由顶部注释）。 +- [x] qualified 版本：`resolveWorkspaceRuntimeFromParam` + + `requireTrustedWorkspaceRuntime`（仿 `workspace-git.ts` 的 + `resolveTrustedRuntime`）。 +- [x] `server.ts`：import + 两处注册调用（紧邻 `registerWorkspaceGitRoutes`）。 +- [x] 路由单测：列表、单文件、越界 path 拒绝、非仓库占位（8 用例通过）。 + +### Task 3 · SDK ✅ 已完成 + +- [x] `types.ts`：`DaemonWorkspaceGitDiffFile` / `DaemonWorkspaceGitDiff` / + `DaemonDiffHunk` / `DaemonWorkspaceGitDiffHunks`（结构见「第二层 diff + payload」）。 +- [x] `daemon/index.ts` + `src/index.ts` 导出新类型。 +- [x] `DaemonClient`：`workspaceGitDiff()` / `workspaceGitDiffFile(path)` + （path 作为 query，`urlEncode`，对齐 `workspaceMcpTools` 写法）； + bound 与 workspace-qualified 两个 client 类各加一对方法。 +- [x] 浏览器 bundle 上限 160KB→165KB（`scripts/build.js`，含说明注释）。 + 未加 client 方法单测：SDK 现有 `workspaceGit()` 亦无对应单测，遵循既有 + 约定不补一次性测试；契约由 cli 路由单测 + typecheck + web-shell 消费侧 + 测试覆盖。 + +### Task 4 · Web Shell ✅ 已完成 + +- [x] `GitDiffDialog.tsx`（+ `.module.css`）： + - 打开时 `workspaceGitDiff()` 拉列表 + 统计；header `N files · +A / -R`。 + - 文件行：`+A -R 文件名`，binary / untracked / deleted 标记；文件名渲染前 + `sanitizeControlChars`（git 允许奇异字节）。 + - 点击文件行 `workspaceGitDiffFile(path)` 懒加载 hunk，展开统一 diff + （`+`/`-`/` ` 行背景着色）；Shiki **per-side 精确高亮**（`codeToTokens`， + language 由 `languageForPath` + `resolveFenceLanguage` 解析；复用 + `codeHighlighter.ts` 的懒加载与 `isTooLargeToHighlight` 降级）。 + - `available === false` / 空 diff / 错误 → 占位文案。 + - 用 `DialogShell`（内部走 `ui/dialog` + `useWebShellPortalRoot`），`size="xl"` + - `allowFullscreen`；`showGitDiffDialog` 状态纳入 `dialogOpen` 聚合。 +- [x] `GitBranchIndicator`：提供 `onOpenDiff` 时 chip 渲染为 ` + ) : ( + + {chipInner} + + )} - {branch} + +
+
+ {s.detached ? t('git.detached') : branch} +
+ {phrases.length > 0 ? ( + phrases.map((phrase) => ( +
+ {phrase} +
+ )) + ) : status ? ( +
{t('git.clean')}
+ ) : null} +
+ ); diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.module.css b/packages/web-shell/client/components/dialogs/GitDiffDialog.module.css new file mode 100644 index 00000000000..676f316c6c4 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.module.css @@ -0,0 +1,155 @@ +.placeholder { + padding: 24px 12px; + text-align: center; + color: var(--muted-foreground); +} + +.fileList { + display: flex; + flex-direction: column; + gap: 8px; +} + +.file { + border: 1px solid var(--border); + border-radius: 6px; + overflow: hidden; +} + +.fileHeader { + display: flex; + align-items: center; + gap: 8px; + width: 100%; + padding: 6px 10px; + background: var(--subtle-bg); + border: 0; + cursor: pointer; + font: inherit; + text-align: left; + color: inherit; +} + +.fileHeader:hover { + background: var(--subtle-bg-strong); +} + +.fileStats { + display: inline-flex; + gap: 6px; + flex-shrink: 0; + font-size: 11px; + font-variant-numeric: tabular-nums; +} + +.statAdd { + color: var(--success-color); +} + +.statDel { + color: var(--error-color); +} + +.fileBinary { + color: var(--muted-foreground); +} + +.filePath { + flex: 1 1 auto; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.fileTag { + flex-shrink: 0; + padding: 0 6px; + border-radius: 4px; + background: var(--muted); + color: var(--muted-foreground); + font-size: 10px; + text-transform: uppercase; +} + +.fileBody { + border-top: 1px solid var(--border); +} + +.filePlaceholder { + padding: 12px; + color: var(--muted-foreground); + font-size: 12px; +} + +.hiddenNote { + padding: 4px 10px; + color: var(--muted-foreground); + font-size: 11px; +} + +.diffLines { + max-height: 480px; + overflow: auto; + font-size: 12px; +} + +.diffLine { + display: flex; + min-height: 18px; + line-height: 18px; +} + +.diffLineAdd { + background: var(--success-bg); +} + +.diffLineDel { + background: var(--error-bg); +} + +.diffLineContext { + background: transparent; +} + +.diffLineMeta { + background: transparent; + color: var(--muted-foreground); + font-style: italic; +} + +.diffOldNo, +.diffNewNo { + width: 40px; + flex-shrink: 0; + text-align: right; + padding-right: 6px; + user-select: none; + color: var(--muted-foreground); + opacity: 0.6; + font-variant-numeric: tabular-nums; +} + +.diffMarker { + width: 14px; + flex-shrink: 0; + text-align: center; + user-select: none; + color: var(--muted-foreground); +} + +.diffLineAdd .diffMarker { + color: var(--success-color); +} + +.diffLineDel .diffMarker { + color: var(--error-color); +} + +.diffContent { + flex: 1 1 auto; + min-width: 0; + white-space: pre; + overflow-x: auto; + padding-right: 8px; +} diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx new file mode 100644 index 00000000000..92c457e3ff0 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -0,0 +1,204 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { I18nProvider } from '../../i18n'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = () => {}; +} + +// A STABLE client object: the dialog's fetch effect depends on `client`, so a +// fresh object per render (as a naive mock returns) would re-fire it in a loop. +const { workspaceGitDiff, workspaceGitDiffFile, workspaceClient } = vi.hoisted( + () => { + const workspaceGitDiff = vi.fn(); + const workspaceGitDiffFile = vi.fn(); + const workspaceClient = { + workspaceByCwd: () => ({ workspaceGitDiff, workspaceGitDiffFile }), + }; + return { workspaceGitDiff, workspaceGitDiffFile, workspaceClient }; + }, +); + +vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ + useWorkspace: () => ({ client: workspaceClient }), +})); + +// Shiki's WASM engine isn't available under jsdom; the dialog must degrade to +// plain text. Stub the highlighter so buildRows takes the plain-text path. +vi.mock('../messages/codeHighlighter', () => ({ + getCodeHighlighter: vi.fn().mockRejectedValue(new Error('no shiki in tests')), + isTooLargeToHighlight: () => false, +})); + +vi.mock('../messages/Markdown', () => ({ + resolveFenceLanguage: (lang: string) => ({ + label: lang, + lang, + resolvedLang: 'text', + }), +})); + +vi.mock('../messages/ToolGroup', () => ({ + languageForPath: () => 'text', +})); + +const { GitDiffDialog } = await import('./GitDiffDialog'); + +let container: HTMLDivElement; +let root: Root; + +function mount(workspaceCwd = '/repo') { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + vi.clearAllMocks(); +}); + +function diffPayload( + overrides: Partial<{ + available: boolean; + files: Array>; + }> = {}, +) { + const files = overrides.files ?? [ + { + path: 'src/a.ts', + added: 2, + removed: 1, + isBinary: false, + isUntracked: false, + isDeleted: false, + truncated: false, + }, + ]; + return { + v: 1 as const, + workspaceCwd: '/repo', + available: overrides.available ?? true, + filesCount: files.length, + linesAdded: 2, + linesRemoved: 1, + files, + hiddenCount: 0, + }; +} + +describe('GitDiffDialog', () => { + it('renders the changed file list with stats', async () => { + workspaceGitDiff.mockResolvedValue(diffPayload()); + mount(); + await flush(); + + expect(workspaceGitDiff).toHaveBeenCalled(); + expect(document.body.textContent).toContain('src/a.ts'); + expect(document.body.textContent).toContain('+2'); + expect(document.body.textContent).toContain('-1'); + }); + + it('loads and renders a file diff when expanded', async () => { + workspaceGitDiff.mockResolvedValue(diffPayload()); + workspaceGitDiffFile.mockResolvedValue({ + v: 1, + workspaceCwd: '/repo', + path: 'src/a.ts', + available: true, + hunks: [ + { + oldStart: 1, + oldLines: 1, + newStart: 1, + newLines: 2, + lines: ['-const a = 1', '+const a = 2', '+const b = 3'], + }, + ], + }); + mount(); + await flush(); + + const header = document.body.querySelector( + 'button[aria-expanded="false"]', + ) as HTMLButtonElement; + expect(header).not.toBeNull(); + await act(async () => { + header.click(); + }); + await flush(); + + expect(workspaceGitDiffFile).toHaveBeenCalledWith('src/a.ts'); + // Plain-text fallback: the line bodies render without the +/- prefix + // (the marker is a separate column). + expect(document.body.textContent).toContain('const a = 2'); + expect(document.body.textContent).toContain('const b = 3'); + expect(document.body.textContent).toContain('const a = 1'); + }); + + it('shows a placeholder when git is unavailable', async () => { + workspaceGitDiff.mockResolvedValue( + diffPayload({ available: false, files: [] }), + ); + mount(); + await flush(); + + expect(document.body.textContent).toContain('Git is not available'); + }); + + it('shows an empty placeholder for a clean working tree', async () => { + workspaceGitDiff.mockResolvedValue(diffPayload({ files: [] })); + mount(); + await flush(); + + expect(document.body.textContent).toContain('No changes'); + }); + + it('marks untracked and binary files in the list', async () => { + workspaceGitDiff.mockResolvedValue( + diffPayload({ + files: [ + { + path: 'new.txt', + added: 1, + removed: 0, + isBinary: false, + isUntracked: true, + isDeleted: false, + truncated: false, + }, + { + path: 'logo.png', + isBinary: true, + isUntracked: false, + isDeleted: false, + truncated: false, + }, + ], + }), + ); + mount(); + await flush(); + + expect(document.body.textContent).toContain('Untracked'); + expect(document.body.textContent).toContain('Binary'); + }); +}); diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx new file mode 100644 index 00000000000..a84f42ce0f5 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -0,0 +1,378 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useEffect, useState, type ReactNode } from 'react'; +import { useWorkspace } from '@qwen-code/webui/daemon-react-sdk'; +import type { + DaemonDiffHunk, + DaemonWorkspaceGitDiff, + DaemonWorkspaceGitDiffFile, +} from '@qwen-code/sdk/daemon'; +import type { BundledLanguage, ThemedToken } from 'shiki'; +import { useI18n } from '../../i18n'; +import { useTheme, WebShellThemeId } from '../../themeContext'; +import { + getCodeHighlighter, + isTooLargeToHighlight, +} from '../messages/codeHighlighter'; +import { resolveFenceLanguage } from '../messages/Markdown'; +import { languageForPath } from '../messages/ToolGroup'; +import { sanitizeControlChars } from '../messages/toolFormatting'; +import { DialogShell } from './DialogShell'; +import styles from './GitDiffDialog.module.css'; + +type RowType = 'add' | 'del' | 'context' | 'meta'; + +interface DiffRow { + type: RowType; + oldNo: number | null; + newNo: number | null; + text: string; + tokens: ThemedToken[] | null; +} + +const ROW_CLASS: Record = { + add: styles.diffLineAdd, + del: styles.diffLineDel, + context: styles.diffLineContext, + meta: styles.diffLineMeta, +}; + +function shikiThemeFor(theme: ReturnType): string { + return theme === WebShellThemeId.Light + ? 'github-light-default' + : 'github-dark-default'; +} + +// Build the unified-diff rows for a file's hunks, highlighting each side +// (context+added / context+removed) as its own code block so multi-line tokens +// (a comment or string crossing an add/delete boundary) still tokenize +// correctly. Each rendered line then pulls its tokens from the matching side: +// `+` from the new side, `-` from the old side, context from either (identical). +async function buildRows( + hunks: DaemonDiffHunk[], + path: string, + theme: string, +): Promise { + const { resolvedLang } = resolveFenceLanguage(languageForPath(path)); + let highlighter: Awaited> | null = null; + if (resolvedLang !== 'text') { + try { + highlighter = await getCodeHighlighter(resolvedLang); + } catch { + highlighter = null; + } + } + + const rows: DiffRow[] = []; + for (const hunk of hunks) { + const newSide: string[] = []; + const oldSide: string[] = []; + for (const line of hunk.lines) { + const prefix = line[0]; + const body = line.slice(1); + if (prefix === '+') newSide.push(body); + else if (prefix === '-') oldSide.push(body); + else if (prefix === ' ') { + newSide.push(body); + oldSide.push(body); + } + } + const newCode = newSide.join('\n'); + const oldCode = oldSide.join('\n'); + let newTokens: ThemedToken[][] | null = null; + let oldTokens: ThemedToken[][] | null = null; + if ( + highlighter && + !isTooLargeToHighlight(newCode) && + !isTooLargeToHighlight(oldCode) + ) { + // resolvedLang is a real Shiki language id here ('text' was filtered out + // before the highlighter was loaded). + const lang = resolvedLang as BundledLanguage; + try { + newTokens = highlighter.codeToTokens(newCode, { lang, theme }).tokens; + oldTokens = highlighter.codeToTokens(oldCode, { lang, theme }).tokens; + } catch { + newTokens = null; + oldTokens = null; + } + } + + let ni = 0; + let oi = 0; + let oldNo = hunk.oldStart; + let newNo = hunk.newStart; + for (const line of hunk.lines) { + const prefix = line[0]; + const body = line.slice(1); + if (prefix === '+') { + rows.push({ + type: 'add', + oldNo: null, + newNo, + text: body, + tokens: newTokens?.[ni] ?? null, + }); + ni++; + newNo++; + } else if (prefix === '-') { + rows.push({ + type: 'del', + oldNo, + newNo: null, + text: body, + tokens: oldTokens?.[oi] ?? null, + }); + oi++; + oldNo++; + } else if (prefix === ' ') { + rows.push({ + type: 'context', + oldNo, + newNo, + text: body, + tokens: newTokens?.[ni] ?? null, + }); + ni++; + oi++; + oldNo++; + newNo++; + } else { + // e.g. "\ No newline at end of file" — a neutral marker, no line number. + rows.push({ + type: 'meta', + oldNo: null, + newNo: null, + text: line, + tokens: null, + }); + } + } + } + return rows; +} + +function renderContent(row: DiffRow): ReactNode { + if (!row.tokens || row.tokens.length === 0) return row.text; + return row.tokens.map((token, index) => ( + + {token.content} + + )); +} + +function DiffHunks({ hunks, path }: { hunks: DaemonDiffHunk[]; path: string }) { + const theme = useTheme(); + const shikiTheme = shikiThemeFor(theme); + const [rows, setRows] = useState(null); + + useEffect(() => { + let cancelled = false; + setRows(null); + void buildRows(hunks, path, shikiTheme).then((built) => { + if (!cancelled) setRows(built); + }); + return () => { + cancelled = true; + }; + }, [hunks, path, shikiTheme]); + + return ( +
+ {(rows ?? []).map((row, index) => ( +
+ {row.oldNo ?? ''} + {row.newNo ?? ''} + + {row.type === 'add' + ? '+' + : row.type === 'del' + ? '-' + : row.type === 'meta' + ? '' + : ' '} + + {renderContent(row)} +
+ ))} +
+ ); +} + +function DiffFileRow({ + workspaceCwd, + file, +}: { + workspaceCwd: string; + file: DaemonWorkspaceGitDiffFile; +}) { + const { t } = useI18n(); + const { client } = useWorkspace(); + const [open, setOpen] = useState(false); + const [hunks, setHunks] = useState(null); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(false); + + const toggle = () => { + const next = !open; + setOpen(next); + if (next && hunks === null && !loading && !file.isBinary) { + setLoading(true); + setError(false); + client + .workspaceByCwd(workspaceCwd) + .workspaceGitDiffFile(file.path) + .then((result) => { + setHunks(result.hunks); + }) + .catch(() => { + setError(true); + }) + .finally(() => { + setLoading(false); + }); + } + }; + + const displayName = sanitizeControlChars(file.path); + + return ( +
+ + {open && ( +
+ {file.isBinary ? ( +
{t('gitDiff.binary')}
+ ) : loading ? ( +
{t('gitDiff.loading')}
+ ) : error ? ( +
+ {t('gitDiff.fileError')} +
+ ) : hunks && hunks.length > 0 ? ( + + ) : ( +
{t('gitDiff.noDiff')}
+ )} +
+ )} +
+ ); +} + +export function GitDiffDialog({ + workspaceCwd, + onClose, +}: { + workspaceCwd: string; + onClose: () => void; +}) { + const { t } = useI18n(); + const { client } = useWorkspace(); + const [diff, setDiff] = useState(null); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(false); + + useEffect(() => { + let cancelled = false; + setLoading(true); + setError(false); + client + .workspaceByCwd(workspaceCwd) + .workspaceGitDiff() + .then((result) => { + if (!cancelled) setDiff(result); + }) + .catch(() => { + if (!cancelled) setError(true); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [client, workspaceCwd]); + + const subtitle = + diff && diff.available + ? t('gitDiff.summary', { + count: diff.filesCount, + added: diff.linesAdded, + removed: diff.linesRemoved, + }) + : undefined; + + let body: ReactNode; + if (loading) { + body =
{t('gitDiff.loading')}
; + } else if (error) { + body =
{t('gitDiff.error')}
; + } else if (!diff || !diff.available) { + body =
{t('gitDiff.unavailable')}
; + } else if (diff.files.length === 0) { + body =
{t('gitDiff.empty')}
; + } else { + body = ( +
+ {diff.files.map((file) => ( + + ))} + {diff.hiddenCount > 0 && ( +
+ {t('gitDiff.hidden', { count: diff.hiddenCount })} +
+ )} +
+ ); + } + + return ( + + {body} + + ); +} diff --git a/packages/web-shell/client/components/sidebar/WebShellSidebar.tsx b/packages/web-shell/client/components/sidebar/WebShellSidebar.tsx index 3fbf96f9583..187e1379f4c 100644 --- a/packages/web-shell/client/components/sidebar/WebShellSidebar.tsx +++ b/packages/web-shell/client/components/sidebar/WebShellSidebar.tsx @@ -235,6 +235,11 @@ interface WebShellSidebarProps { */ selectedWorkspaceCwd?: string; onSelectWorkspace?: (workspaceCwd: string | undefined) => void; + /** + * Open the working-tree Changes dialog for a workspace. Forwarded to each + * trusted workspace's folder header, where a live git chip fires it on click. + */ + onOpenGitDiff?: (workspaceCwd: string) => void; workspaces?: DaemonWorkspaceCapability[]; lockedWorkspaceCwd?: string; lockedWorkspace?: WebShellSidebarLockedWorkspace; @@ -422,6 +427,7 @@ export function WebShellSidebar({ sessionListReloadToken, selectedWorkspaceCwd, onSelectWorkspace, + onOpenGitDiff, workspaces: providedWorkspaces, lockedWorkspaceCwd, lockedWorkspace: lockedWorkspaceOptions, @@ -3498,6 +3504,7 @@ export function WebShellSidebar({ deleteGroupLabel={t('sidebar.groupDelete')} groupActionsDisabled={groupBusy} excludePinned + onOpenGitDiff={onOpenGitDiff} formatTime={(iso) => formatRelativeTime(iso, t)} searchQuery={searchQuery} expanded={ws.primary ? projectExpanded : undefined} diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.module.css b/packages/web-shell/client/components/sidebar/WorkspaceSection.module.css index 3d3e6da4b69..bfdd67b914b 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.module.css +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.module.css @@ -76,6 +76,25 @@ border-radius: 4px; } +/* Live git chip in the folder header — icon-only (the chip's `compact` form): a + status dot on the branch icon conveys dirty / conflict / in-progress at a + glance, while the branch name + ahead/behind live in the hover tooltip. + Rendered as a sibling of the header button (buttons can't nest), kept snug + after the folder name. The chip is a fixed 28px box, so the pill grows only to + carry the spare width to the hover actions on the right. */ +.gitPill { + display: inline-flex; + flex: 1 1 auto; + min-width: 0; +} + +/* With a chip present, let the pill — not the folder name — own the spare width, + so the icon sits right after the name and the hover actions stay pinned to the + right edge. Without a chip the header keeps growing as before. */ +.headerRow:has(.gitPill) .header { + flex-grow: 0; +} + /* Trusted sessions use the sidebar's shared row with its own left indent; read-only rows apply the same indent below. */ .sessions { diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx new file mode 100644 index 00000000000..c44cd5dd899 --- /dev/null +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx @@ -0,0 +1,194 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import type { ReactNode } from 'react'; +import type { + DaemonClient, + DaemonSessionSummary, + DaemonWorkspaceCapability, + DaemonWorkspaceGitStatus, +} from '@qwen-code/sdk/daemon'; +import gitStyles from '../ChatEditor.module.css'; + +const { workspaceGit } = vi.hoisted(() => ({ + workspaceGit: vi.fn(), +})); + +// A stable client whose `workspaceByCwd` always returns the same `workspaceGit` +// mock, so call assertions accumulate regardless of how often the component +// re-resolves the workspace handle. +function makeClient(): DaemonClient { + return { + workspaceByCwd: vi.fn(() => ({ + workspaceGit, + listWorkspaceSessions: vi.fn().mockResolvedValue([]), + listSessionGroups: vi.fn().mockResolvedValue({ groups: [] }), + })), + } as unknown as DaemonClient; +} + +const { I18nProvider } = await import('../../i18n'); +const { WorkspaceSection } = await import('./WorkspaceSection'); + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; +if (!globalThis.PointerEvent) { + globalThis.PointerEvent = MouseEvent as typeof PointerEvent; +} +if (!Element.prototype.hasPointerCapture) { + Element.prototype.hasPointerCapture = () => false; +} +if (!Element.prototype.setPointerCapture) { + Element.prototype.setPointerCapture = () => {}; +} +if (!Element.prototype.releasePointerCapture) { + Element.prototype.releasePointerCapture = () => {}; +} + +const trustedWorkspace: DaemonWorkspaceCapability = { + id: 'primary', + cwd: '/tmp/project', + primary: true, + trusted: true, + removable: false, +}; + +const untrustedWorkspace: DaemonWorkspaceCapability = { + id: 'danger', + cwd: '/tmp/danger', + primary: false, + trusted: false, + removable: true, +}; + +let root: Root; +let container: HTMLDivElement; + +function renderSection( + overrides: Partial<{ + workspace: DaemonWorkspaceCapability; + onOpenGitDiff: (cwd: string) => void; + }> = {}, +): void { + act(() => { + root.render( + + ''} + renderSession={(session: DaemonSessionSummary): ReactNode => ( +
{session.displayName}
+ )} + onOpenGitDiff={overrides.onOpenGitDiff} + /> + , + ); + }); +} + +async function flush(): Promise { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +function gitChip(): HTMLElement | null { + return container.querySelector('[data-web-shell-git-branch]'); +} + +beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + workspaceGit.mockReset(); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + vi.restoreAllMocks(); +}); + +describe('WorkspaceSection git chip', () => { + it('renders a clickable git chip for a trusted repo and opens its diff', async () => { + const status: DaemonWorkspaceGitStatus = { + v: 2, + workspaceCwd: '/tmp/project', + branch: 'main', + unstaged: 1, + }; + workspaceGit.mockResolvedValue(status); + const onOpenGitDiff = vi.fn(); + + renderSection({ onOpenGitDiff }); + await flush(); + + const chip = gitChip(); + expect(chip).not.toBeNull(); + expect(chip?.tagName).toBe('BUTTON'); + expect(chip?.getAttribute('data-dirty')).toBe('true'); + // Icon-only (compact) form: the branch name is not shown as inline text but + // stays reachable via the accessible name (the hover tooltip). + expect(chip?.className).toContain(gitStyles.gitBranchChipCompact); + expect(chip?.getAttribute('aria-label')).toContain('main'); + + act(() => { + chip?.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + expect(onOpenGitDiff).toHaveBeenCalledWith('/tmp/project'); + }); + + it('hides the chip for an untrusted workspace and never queries git', async () => { + workspaceGit.mockResolvedValue({ + v: 2, + workspaceCwd: '/tmp/danger', + branch: 'main', + }); + + renderSection({ + workspace: untrustedWorkspace, + onOpenGitDiff: vi.fn(), + }); + await flush(); + + expect(gitChip()).toBeNull(); + expect(workspaceGit).not.toHaveBeenCalled(); + }); + + it('hides the chip when the workspace is not a git repo (null branch)', async () => { + workspaceGit.mockResolvedValue({ + v: 2, + workspaceCwd: '/tmp/project', + branch: null, + }); + + renderSection({ onOpenGitDiff: vi.fn() }); + await flush(); + + expect(workspaceGit).toHaveBeenCalled(); + expect(gitChip()).toBeNull(); + }); + + it('omits the chip when no diff handler is provided', async () => { + workspaceGit.mockResolvedValue({ + v: 2, + workspaceCwd: '/tmp/project', + branch: 'main', + }); + + renderSection({ onOpenGitDiff: undefined }); + await flush(); + + expect(gitChip()).toBeNull(); + }); +}); diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx index f35c5ae1878..da69cec949a 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx @@ -10,8 +10,10 @@ import type { DaemonSessionGroup, DaemonSessionSummary, DaemonWorkspaceCapability, + DaemonWorkspaceGitStatus, } from '@qwen-code/sdk/daemon'; import { FolderClosedIcon, FolderOpenIcon } from 'lucide-react'; +import { GitBranchIndicator } from '../GitBranchIndicator'; import { SESSION_LIST_PAGE_SIZE } from '../../constants/sessions'; import { readWorkspaceCollapsedGroupIds, @@ -79,6 +81,12 @@ interface WorkspaceSectionProps { deleteGroupLabel?: string; groupActionsDisabled?: boolean; excludePinned?: boolean; + /** + * Open the working-tree Changes dialog for this workspace. When provided, the + * folder header shows a live git chip (branch + dirty/ahead-behind state) that + * fires this on click. Omitted for untrusted workspaces (no git surface). + */ + onOpenGitDiff?: (workspaceCwd: string) => void; } export function WorkspaceSection({ @@ -108,6 +116,7 @@ export function WorkspaceSection({ deleteGroupLabel, groupActionsDisabled, excludePinned = false, + onOpenGitDiff, }: WorkspaceSectionProps) { const [sessions, setSessions] = useState([]); const [groups, setGroups] = useState([]); @@ -117,6 +126,7 @@ export function WorkspaceSection({ readWorkspaceCollapsedGroupIds(workspace.id), ); const [actionsVisible, setActionsVisible] = useState(false); + const [gitStatus, setGitStatus] = useState(); const expanded = controlledExpanded ?? internalExpanded; const readOnly = !workspace.primary && !workspace.trusted; const disabled = workspace.primary && !workspace.trusted; @@ -203,6 +213,39 @@ export function WorkspaceSection({ searchQuery, ]); + const loadGitStatus = useCallback(async () => { + if (!onOpenGitDiff || !workspace.trusted) return; + try { + const status = await client.workspaceByCwd(workspace.cwd).workspaceGit(); + setGitStatus(status); + } catch (err) { + console.warn('[WorkspaceSection] git status poll failed:', err); + setGitStatus(undefined); + } + }, [client, onOpenGitDiff, workspace.cwd, workspace.trusted]); + + // The git chip lives in the always-visible folder header, so it polls + // independently of session expansion: on mount/trust, on window focus, and on + // a visibility-gated 60s tick (the daemon recomputes the working-tree summary + // per call, so the cadence stays gentle). Skipped entirely when no diff + // handler is wired, since the chip — its only consumer — would not render. + useEffect(() => { + if (!onOpenGitDiff || !workspace.trusted) { + setGitStatus(undefined); + return; + } + void loadGitStatus(); + const onFocus = () => void loadGitStatus(); + window.addEventListener('focus', onFocus); + const timer = window.setInterval(() => { + if (document.visibilityState === 'visible') void loadGitStatus(); + }, 60_000); + return () => { + window.removeEventListener('focus', onFocus); + window.clearInterval(timer); + }; + }, [loadGitStatus, onOpenGitDiff, reloadToken, workspace.trusted]); + const visibleSessions = useMemo(() => { const query = searchQuery.trim().toLowerCase(); return sessions.filter((session) => { @@ -283,6 +326,16 @@ export function WorkspaceSection({ )} + {onOpenGitDiff && workspace.trusted && gitStatus?.branch && ( + + onOpenGitDiff(workspace.cwd)} + /> + + )} {headerActions?.(actionsVisible)} {renderSessions && diff --git a/packages/web-shell/client/constants/localCommands.ts b/packages/web-shell/client/constants/localCommands.ts index 00516e672bf..909b39f943d 100644 --- a/packages/web-shell/client/constants/localCommands.ts +++ b/packages/web-shell/client/constants/localCommands.ts @@ -89,6 +89,7 @@ export function getLocalCommands(t: Translate): CommandInfo[] { description: t('local.branch'), argumentHint: '[]', }, + { name: 'diff', description: t('local.diff') }, { name: 'fork', description: t('local.fork'), diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 13c94a86b41..8a4af3bdf36 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -17,6 +17,35 @@ type Messages = Record; const EN: Messages = { 'git.currentBranch': (v) => `Current Git branch: ${v?.branch ?? ''}`, + 'git.detached': 'Detached HEAD', + 'git.clean': 'Working tree clean', + 'git.operation.merge': 'Merging', + 'git.operation.rebase': 'Rebasing', + 'git.operation.cherry-pick': 'Cherry-picking', + 'git.operation.revert': 'Reverting', + 'git.operation.bisect': 'Bisecting', + 'git.conflicted': (v) => `${v?.count ?? 0} conflicted`, + 'git.staged': (v) => `${v?.count ?? 0} staged`, + 'git.unstaged': (v) => `${v?.count ?? 0} modified`, + 'git.untracked': (v) => `${v?.count ?? 0} untracked`, + 'git.ahead': (v) => `${v?.count ?? 0} ahead`, + 'git.behind': (v) => `${v?.count ?? 0} behind`, + 'git.stash': (v) => `${v?.count ?? 0} stashed`, + 'gitDiff.title': 'Changes', + 'gitDiff.summary': (v) => + `${v?.count ?? 0} files · +${v?.added ?? 0} −${v?.removed ?? 0}`, + 'gitDiff.loading': 'Loading changes…', + 'gitDiff.empty': 'No changes in the working tree', + 'gitDiff.unavailable': 'Git is not available for this workspace', + 'gitDiff.error': 'Failed to load changes', + 'gitDiff.binary': 'Binary', + 'gitDiff.untracked': 'Untracked', + 'gitDiff.deleted': 'Deleted', + 'gitDiff.noDiff': 'No changes to display', + 'gitDiff.fileError': 'Failed to load this diff', + 'gitDiff.hidden': (v) => `${v?.count ?? 0} more file(s) not shown`, + 'gitDiff.expand': 'Show file changes', + 'gitDiff.collapse': 'Hide file changes', 'workspace.paneLabel': (v) => `Workspace: ${v?.name ?? ''}`, 'about.auth': 'Auth', 'about.baseUrl': 'Base URL', @@ -2019,6 +2048,35 @@ const EN: Messages = { const ZH: Messages = { ...EN, 'git.currentBranch': (v) => `当前 Git 分支：${v?.branch ?? ''}`, + 'git.detached': '游离 HEAD', + 'git.clean': '工作区干净', + 'git.operation.merge': '合并中', + 'git.operation.rebase': '变基中', + 'git.operation.cherry-pick': '拣选中', + 'git.operation.revert': '回退中', + 'git.operation.bisect': '二分中', + 'git.conflicted': (v) => `${v?.count ?? 0} 个冲突`, + 'git.staged': (v) => `${v?.count ?? 0} 已暂存`, + 'git.unstaged': (v) => `${v?.count ?? 0} 已修改`, + 'git.untracked': (v) => `${v?.count ?? 0} 未跟踪`, + 'git.ahead': (v) => `领先 ${v?.count ?? 0}`, + 'git.behind': (v) => `落后 ${v?.count ?? 0}`, + 'git.stash': (v) => `${v?.count ?? 0} 个 stash`, + 'gitDiff.title': '变更', + 'gitDiff.summary': (v) => + `${v?.count ?? 0} 个文件 · +${v?.added ?? 0} −${v?.removed ?? 0}`, + 'gitDiff.loading': '加载变更中…', + 'gitDiff.empty': '工作区无变更', + 'gitDiff.unavailable': '此工作区无 Git', + 'gitDiff.error': '加载变更失败', + 'gitDiff.binary': '二进制', + 'gitDiff.untracked': '未跟踪', + 'gitDiff.deleted': '已删除', + 'gitDiff.noDiff': '无差异可显示', + 'gitDiff.fileError': '加载此差异失败', + 'gitDiff.hidden': (v) => `还有 ${v?.count ?? 0} 个文件未显示`, + 'gitDiff.expand': '显示文件变更', + 'gitDiff.collapse': '隐藏文件变更', 'workspace.paneLabel': (v) => `工作区：${v?.name ?? ''}`, // Tool display names (chat-stream badge labels). Keyed by `toolName.`; // a wire name with no entry here falls back to the English display name via From 895a682d61b440fb6b1d3cbb4118ba871364df2d Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 04:31:13 +0800 Subject: [PATCH 02/24] fix(web-shell): themed tooltips and git-chip review follow-ups Tooltips now render on the themed popover surface (bg-popover / text-popover-foreground / border + fill-popover arrow) instead of the inverted bg-foreground default, so they read dark-on-dark rather than a bright box on the dark theme. Fixing the shared primitive corrects the git branch tooltip in the composer toolbar and sidebar, plus every other tooltip, at once. Also addressing review feedback on the git integration: - Replace the hand-drawn detached/conflict/stash SVG icons with lucide-react (CircleDot / TriangleAlert / Layers) per the web-shell icon convention. - Gate the tooltip "Working tree clean" message on an enriched status (computedAt) so a branch-only status no longer asserts clean. - Include the file path in the diff dialog row aria-label so screen readers can distinguish files. - Reset the toolbar git chip on workspace switch so it never shows the previous repo's branch/counts while the new fetch resolves. - Log a sidebar git poll failure only on the success->failure transition to avoid spamming a long-lived tab. - Correct the SDK doc for DaemonWorkspaceGitDiffFile.added/removed (0, not undefined, for binary files). --- packages/sdk-typescript/src/daemon/types.ts | 4 +- packages/web-shell/client/App.tsx | 10 ++++ .../client/components/GitBranchIndicator.tsx | 56 ++----------------- .../components/dialogs/GitDiffDialog.tsx | 4 +- .../components/sidebar/WorkspaceSection.tsx | 10 +++- .../client/components/ui/tooltip.tsx | 4 +- packages/web-shell/client/i18n.tsx | 8 +-- 7 files changed, 35 insertions(+), 61 deletions(-) diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 5b8c9d9029a..e4eda78dec9 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -102,9 +102,9 @@ export interface DaemonWorkspaceGitStatus { export interface DaemonWorkspaceGitDiffFile { /** Repo-root-relative path (render after sanitizing — git allows odd bytes). */ path: string; - /** Lines added; `undefined` for binary files. */ + /** Lines added (`0` for binary files). */ added?: number; - /** Lines removed; `undefined` for binary files. */ + /** Lines removed (`0` for binary files). */ removed?: number; isBinary: boolean; isUntracked: boolean; diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 3026dde775b..b63dfecc089 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -1222,6 +1222,11 @@ export function App({ const [selectedWorkspaceGitStatus, setSelectedWorkspaceGitStatus] = useState< DaemonWorkspaceGitStatus | undefined >(undefined); + // The workspace the chip's status was last fetched for. On a workspace switch + // we clear the status immediately so the chip never shows the previous repo's + // branch/dirty counts while the new fetch is in flight; same-workspace + // re-runs (branch change, focus, poll) keep the live value to avoid flicker. + const gitStatusWorkspaceCwdRef = useRef(undefined); useEffect(() => { // Active workspace: the connected session's workspace, else the workspace // picked for the next session (locked / selected / primary). @@ -1231,9 +1236,14 @@ export function App({ selectedWorkspaceCwd ?? workspaces.find((entry) => entry.primary)?.cwd); if (!activeWorkspaceCwd) { + gitStatusWorkspaceCwdRef.current = undefined; setSelectedWorkspaceGitStatus(undefined); return; } + if (gitStatusWorkspaceCwdRef.current !== activeWorkspaceCwd) { + gitStatusWorkspaceCwdRef.current = activeWorkspaceCwd; + setSelectedWorkspaceGitStatus(undefined); + } let cancelled = false; const fetchStatus = () => { void workspace.client diff --git a/packages/web-shell/client/components/GitBranchIndicator.tsx b/packages/web-shell/client/components/GitBranchIndicator.tsx index 5d86a3af5e8..cbb21f73517 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.tsx @@ -5,6 +5,7 @@ */ import type { DaemonWorkspaceGitStatus } from '@qwen-code/sdk/daemon'; +import { CircleDotIcon, LayersIcon, TriangleAlertIcon } from 'lucide-react'; import { useI18n } from '../i18n'; import styles from './ChatEditor.module.css'; import { @@ -30,53 +31,6 @@ function GitBranchIcon() { ); } -function GitDetachedIcon() { - return ( - - ); -} - -function GitWarningIcon() { - return ( - - ); -} - -function GitStashIcon() { - return ( - - ); -} - /** Tone of the compact badge dot, by descending severity. */ type BadgeTone = 'error' | 'warning' | 'accent'; @@ -171,7 +125,7 @@ export function GitBranchIndicator({ <> - {s.detached ? : } + {s.detached ? : } {compact && tone && ( 0 && ( - + {s.conflicted} )} @@ -204,7 +158,7 @@ export function GitBranchIndicator({ )} {s.stashCount > 0 && ( - + {s.stashCount} )} @@ -248,7 +202,7 @@ export function GitBranchIndicator({ {phrase} )) - ) : status ? ( + ) : status?.computedAt !== undefined ? (
{t('git.clean')}
) : null} diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index a84f42ce0f5..50ca1e50432 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -250,7 +250,9 @@ function DiffFileRow({ className={styles.fileHeader} onClick={toggle} aria-expanded={open} - aria-label={t(open ? 'gitDiff.collapse' : 'gitDiff.expand')} + aria-label={t(open ? 'gitDiff.collapse' : 'gitDiff.expand', { + path: displayName, + })} > {file.isBinary ? ( diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx index da69cec949a..12d9bc23cea 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo, + useRef, useState, type ReactNode, } from 'react'; @@ -213,13 +214,20 @@ export function WorkspaceSection({ searchQuery, ]); + // Log a poll failure only on the success→failure transition, not on every + // 60s/focus tick, so an unreachable workspace doesn't spam a long-lived tab. + const gitPollFailed = useRef(false); const loadGitStatus = useCallback(async () => { if (!onOpenGitDiff || !workspace.trusted) return; try { const status = await client.workspaceByCwd(workspace.cwd).workspaceGit(); + gitPollFailed.current = false; setGitStatus(status); } catch (err) { - console.warn('[WorkspaceSection] git status poll failed:', err); + if (!gitPollFailed.current) { + console.warn('[WorkspaceSection] git status poll failed:', err); + gitPollFailed.current = true; + } setGitStatus(undefined); } }, [client, onOpenGitDiff, workspace.cwd, workspace.trusted]); diff --git a/packages/web-shell/client/components/ui/tooltip.tsx b/packages/web-shell/client/components/ui/tooltip.tsx index f7819dd10f6..fc8b6e485df 100644 --- a/packages/web-shell/client/components/ui/tooltip.tsx +++ b/packages/web-shell/client/components/ui/tooltip.tsx @@ -42,13 +42,13 @@ function TooltipContent({ data-slot="tooltip-content" sideOffset={sideOffset} className={cn( - 'z-50 inline-flex w-fit max-w-xs origin-(--radix-tooltip-content-transform-origin) items-center gap-1.5 rounded-md bg-foreground px-3 py-1.5 text-xs text-background has-data-[slot=kbd]:pr-1.5 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 **:data-[slot=kbd]:relative **:data-[slot=kbd]:isolate **:data-[slot=kbd]:z-50 **:data-[slot=kbd]:rounded-sm data-[state=delayed-open]:animate-in data-[state=delayed-open]:fade-in-0 data-[state=delayed-open]:zoom-in-95 data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95', + 'z-50 inline-flex w-fit max-w-xs origin-(--radix-tooltip-content-transform-origin) items-center gap-1.5 rounded-md border border-border bg-popover px-3 py-1.5 text-xs text-popover-foreground has-data-[slot=kbd]:pr-1.5 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 **:data-[slot=kbd]:relative **:data-[slot=kbd]:isolate **:data-[slot=kbd]:z-50 **:data-[slot=kbd]:rounded-sm data-[state=delayed-open]:animate-in data-[state=delayed-open]:fade-in-0 data-[state=delayed-open]:zoom-in-95 data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95', className, )} {...props} > {children} - + ); diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 8a4af3bdf36..96cc7c8891f 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -44,8 +44,8 @@ const EN: Messages = { 'gitDiff.noDiff': 'No changes to display', 'gitDiff.fileError': 'Failed to load this diff', 'gitDiff.hidden': (v) => `${v?.count ?? 0} more file(s) not shown`, - 'gitDiff.expand': 'Show file changes', - 'gitDiff.collapse': 'Hide file changes', + 'gitDiff.expand': (v) => `Show changes for ${v?.path ?? 'file'}`, + 'gitDiff.collapse': (v) => `Hide changes for ${v?.path ?? 'file'}`, 'workspace.paneLabel': (v) => `Workspace: ${v?.name ?? ''}`, 'about.auth': 'Auth', 'about.baseUrl': 'Base URL', @@ -2075,8 +2075,8 @@ const ZH: Messages = { 'gitDiff.noDiff': '无差异可显示', 'gitDiff.fileError': '加载此差异失败', 'gitDiff.hidden': (v) => `还有 ${v?.count ?? 0} 个文件未显示`, - 'gitDiff.expand': '显示文件变更', - 'gitDiff.collapse': '隐藏文件变更', + 'gitDiff.expand': (v) => `显示 ${v?.path ?? '文件'} 的变更`, + 'gitDiff.collapse': (v) => `隐藏 ${v?.path ?? '文件'} 的变更`, 'workspace.paneLabel': (v) => `工作区：${v?.name ?? ''}`, // Tool display names (chat-stream badge labels). Keyed by `toolName.`; // a wire name with no entry here falls back to the English display name via From c25ec116b7c2234b1824701ee0df7d4af4f48d1a Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 08:09:45 +0800 Subject: [PATCH 03/24] fix(web-shell): address git-integration review suggestions Follow-ups from the /review pass on the git integration: - GitBranchIndicator: include the short SHA in the detached-HEAD tooltip title, and add the "Working tree clean" status to the aria-label (gated on an enriched status, matching the tooltip) so the two never drift. - WorkspaceSection: keep the last known git status on a transient poll failure instead of blanking the chip for a whole interval. - App: surface a toast for `/diff` when no workspace is available instead of silently consuming the composer input. - Tests: cover the diff dialog's list-load and per-file load error paths, and detectGitOperation's revert/bisect branches. - Design doc: align the getGitWorkingTreeStatus spec text with the decision (transient states return status with `operation`; null is reserved for non-repo / git failure). --- .../2026-07-16-webshell-git-status-diff.md | 6 +++-- packages/core/src/utils/gitDiff.test.ts | 23 ++++++++++++++++ packages/web-shell/client/App.tsx | 6 ++++- .../client/components/GitBranchIndicator.tsx | 6 +++-- .../components/dialogs/GitDiffDialog.test.tsx | 27 +++++++++++++++++++ .../components/sidebar/WorkspaceSection.tsx | 4 ++- packages/web-shell/client/i18n.tsx | 3 +++ 7 files changed, 69 insertions(+), 6 deletions(-) diff --git a/docs/design/2026-07-16-webshell-git-status-diff.md b/docs/design/2026-07-16-webshell-git-status-diff.md index c6df81e85ff..2eee323e0bc 100644 --- a/docs/design/2026-07-16-webshell-git-status-diff.md +++ b/docs/design/2026-07-16-webshell-git-status-diff.md @@ -348,8 +348,10 @@ daemon 端只需把 `Map` 里对应文件的 hunk 数组序列化即可，前端 / 公有构件，避免跨文件暴露内部函数： - `getGitWorkingTreeStatus(cwd): Promise` - - 复用 `findGitRoot` 判断是否仓库、`isInTransientGitState` 判断 transient - state（与 `fetchGitDiff` 一致），非仓库 / transient 返回 `null`。 + - 复用 `findGitRoot` 判断是否仓库；非仓库 / git 失败返回 `null`。transient + state（merge/rebase/cherry-pick/…）期间仍返回状态，并通过 `operation` + 字段标记操作类型（见上方"transient state 处理变更"），故不调用 + `isInTransientGitState`。 - 一次 `git --no-optional-locks status --porcelain=v1 --branch -z` 调用， 解析 branch header（branch / detached / `...upstream` / `[ahead N, behind M]`）和 porcelain 行（统计 staged / unstaged / untracked）。解析逻辑可参考 diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index 45ee51f171a..05cdf82ec55 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -1717,4 +1717,27 @@ describe('getGitWorkingTreeStatus', () => { const status = await getGitWorkingTreeStatus(repo); expect(status).toMatchObject({ operation: 'cherry-pick' }); }); + + it('detects an in-progress revert', async () => { + await fs.writeFile(path.join(repo, 'a.txt'), 'one\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + const sha = ( + await execFileAsync('git', ['rev-parse', 'HEAD'], { cwd: repo }) + ).stdout.trim(); + await fs.writeFile(path.join(repo, '.git', 'REVERT_HEAD'), `${sha}\n`); + + const status = await getGitWorkingTreeStatus(repo); + expect(status).toMatchObject({ operation: 'revert' }); + }); + + it('detects an in-progress bisect', async () => { + await fs.writeFile(path.join(repo, 'a.txt'), 'one\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + await fs.writeFile(path.join(repo, '.git', 'BISECT_LOG'), 'bisect\n'); + + const status = await getGitWorkingTreeStatus(repo); + expect(status).toMatchObject({ operation: 'bisect' }); + }); }); diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index b63dfecc089..e8c3094d469 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -4200,7 +4200,11 @@ export function App({ if (cmd === 'diff') { // Local intercept: open the working-tree Changes dialog instead of // forwarding `/diff` to the agent. Targets the current workspace. - if (gitDiffWorkspaceCwd) setDiffWorkspaceCwd(gitDiffWorkspaceCwd); + if (!gitDiffWorkspaceCwd) { + pushToast('info', t('localCommand.diffNoWorkspace')); + return true; + } + setDiffWorkspaceCwd(gitDiffWorkspaceCwd); return true; } if (cmd === 'tasks') { diff --git a/packages/web-shell/client/components/GitBranchIndicator.tsx b/packages/web-shell/client/components/GitBranchIndicator.tsx index cbb21f73517..3c49c33d320 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.tsx @@ -105,7 +105,9 @@ export function GitBranchIndicator({ const ariaLabel = phrases.length > 0 ? `${t('git.currentBranch', { branch })} — ${phrases.join(', ')}` - : t('git.currentBranch', { branch }); + : status?.computedAt !== undefined + ? `${t('git.currentBranch', { branch })} — ${t('git.clean')}` + : t('git.currentBranch', { branch }); const tone = badgeTone(s); @@ -194,7 +196,7 @@ export function GitBranchIndicator({
- {s.detached ? t('git.detached') : branch} + {s.detached ? `${t('git.detached')} (${branch})` : branch}
{phrases.length > 0 ? ( phrases.map((phrase) => ( diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx index 92c457e3ff0..4351617c18b 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -201,4 +201,31 @@ describe('GitDiffDialog', () => { expect(document.body.textContent).toContain('Untracked'); expect(document.body.textContent).toContain('Binary'); }); + + it('shows an error placeholder when the diff list fails to load', async () => { + workspaceGitDiff.mockRejectedValue(new Error('network down')); + mount(); + await flush(); + + expect(document.body.textContent).toContain('Failed to load changes'); + }); + + it('shows a per-file error when a file diff fails to load', async () => { + workspaceGitDiff.mockResolvedValue(diffPayload()); + workspaceGitDiffFile.mockRejectedValue(new Error('file fetch failed')); + mount(); + await flush(); + + const header = document.body.querySelector( + 'button[aria-expanded="false"]', + ) as HTMLButtonElement; + expect(header).not.toBeNull(); + await act(async () => { + header.click(); + }); + await flush(); + + expect(workspaceGitDiffFile).toHaveBeenCalledWith('src/a.ts'); + expect(document.body.textContent).toContain('Failed to load this diff'); + }); }); diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx index 12d9bc23cea..a95fa4f85b3 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx @@ -224,11 +224,13 @@ export function WorkspaceSection({ gitPollFailed.current = false; setGitStatus(status); } catch (err) { + // Keep the last known status on a transient failure so a brief network + // or daemon blip doesn't blank the chip for a whole poll interval; log + // only on the success→failure transition. if (!gitPollFailed.current) { console.warn('[WorkspaceSection] git status poll failed:', err); gitPollFailed.current = true; } - setGitStatus(undefined); } }, [client, onOpenGitDiff, workspace.cwd, workspace.trusted]); diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 96cc7c8891f..f34bde05812 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -1092,6 +1092,8 @@ const EN: Messages = { 'language.usage': 'Usage: /language ui [en|zh-CN]', 'localCommand.noSession': 'No active session yet. Send your first message before using this command.', + 'localCommand.diffNoWorkspace': + 'No workspace is available yet to show changes for.', 'local.agents': 'Manage subagents', 'local.bug': 'Submit a bug report', 'local.compress': 'Compress the context into a summary', @@ -3122,6 +3124,7 @@ const ZH: Messages = { 'language.usage': '用法：/language ui [en|zh-CN]', 'localCommand.noSession': '当前还没有会话。请先发送第一条消息，再使用这个命令。', + 'localCommand.diffNoWorkspace': '当前还没有可用于查看变更的工作区。', 'local.agents': '管理智能体', 'local.bug': '提交错误报告', 'local.compress': '将上下文压缩为摘要', From 52e4df2d9a85d39357dea367c42bfad4b4cdadce Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 09:00:38 +0800 Subject: [PATCH 04/24] fix(web-shell): focus-visible ring for git chip button; align doc poll interval - Add a :focus-visible outline to .gitBranchChipButton so keyboard users get a visible focus indicator (the chip resets UA button chrome). - Design doc: align the active-workspace poll-interval references at 30s to match the implementation. --- docs/design/2026-07-16-webshell-git-status-diff.md | 4 ++-- packages/web-shell/client/components/ChatEditor.module.css | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/design/2026-07-16-webshell-git-status-diff.md b/docs/design/2026-07-16-webshell-git-status-diff.md index 2eee323e0bc..6230929a352 100644 --- a/docs/design/2026-07-16-webshell-git-status-diff.md +++ b/docs/design/2026-07-16-webshell-git-status-diff.md @@ -452,7 +452,7 @@ M]`）和 porcelain 行（统计 staged / unstaged / untracked）。解析逻辑 1. 用户打开 status popover 或 `GitDiffDialog` 时（按需，权威）。 2. 收到 `git_branch_changed`（commit / reset / 切分支都会同时改变这些值）。 3. 标签页 `visibilitychange` 重新可见时。 - 4. 仅对**当前选中 / 可见的 workspace**做一次低速轮询（如 15s，可配置）， + 4. 仅对**当前选中 / 可见的 workspace**做一次低速轮询（如 30s，可配置）， 保证 dirty 点在编辑后“足够新”。**不**对所有 workspace 轮询。 - 明确取舍：不对工作区文件树建立 watcher，dirty 不会逐键实时刷新。这是为了 避免昂贵的全树监听；对“编辑后立刻想看 dirty”的场景，focus / 轮询 / 打开弹窗 @@ -604,7 +604,7 @@ computedAt`），`v: 1 | 2`；新增 `DaemonGitOperation` 类型并从 - 重字段（dirty/ahead/behind/stash/operation/conflicted）在以下时机重拉 `workspaceGit()`：① 打开 status popover / diff 弹窗；② 收到 `git_branch_changed`；③ 标签页 `visibilitychange` 重新可见；④ 仅对**当前活跃 - workspace** 低速轮询（如 15s，可配置）。 + workspace** 低速轮询（如 30s，可配置）。 - 不对工作区文件树建 watcher，dirty 不逐键实时（成本取舍）。 ### Task 5 · 验证 ✅ 已完成 diff --git a/packages/web-shell/client/components/ChatEditor.module.css b/packages/web-shell/client/components/ChatEditor.module.css index b04ce34a2cb..badb3895ba8 100644 --- a/packages/web-shell/client/components/ChatEditor.module.css +++ b/packages/web-shell/client/components/ChatEditor.module.css @@ -964,6 +964,11 @@ background: var(--subtle-bg); } +.gitBranchChipButton:focus-visible { + outline: 2px solid var(--chat-editor-accent-color); + outline-offset: -2px; +} + .gitBranchIcon { display: inline-flex; width: 16px; From 192db5f9d6ccc10f6aa9ef5c1f739f1384b198e5 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 17 Jul 2026 09:38:24 +0800 Subject: [PATCH 05/24] fix(web-shell): surface capped diffs, catch row-build failures, cover degradation paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the remaining review findings on the git integration: - Truncation is no longer silent: fetchGitDiffHunksForFile now returns { hunks, truncated } — the parser records files that actually lost lines to MAX_LINES_PER_FILE (tracked path), and the untracked synthesis reports its byte/line caps. The route forwards an additive `truncated` flag on the hunks response (absent when not truncated, so older clients and daemons are unaffected), and the Changes dialog renders a "Diff truncated" note under the visible window. - DiffHunks catches an unexpected buildRows rejection (e.g. malformed hunk lines) and shows the per-file error instead of leaving an unhandled rejection and a silently empty diff area. - New tests: untracked and tracked truncation at the core caps, the route's truncated passthrough (and its absence when clean), the branch-only degradation when the working-tree summary throws, the malformed-hunks error path, and the Shiki success path (a fake tokenizer proving add rows pull new-side tokens and del rows pull old-side tokens, not the plain-text fallback). --- .../serve/routes/workspace-git-diff.test.ts | 54 +++++-- .../src/serve/routes/workspace-git-diff.ts | 15 +- .../cli/src/serve/workspace-git-state.test.ts | 23 +++ packages/core/src/utils/gitDiff.test.ts | 67 ++++++-- packages/core/src/utils/gitDiff.ts | 78 ++++++--- packages/sdk-typescript/src/daemon/types.ts | 6 + .../components/dialogs/GitDiffDialog.test.tsx | 152 ++++++++++++++++-- .../components/dialogs/GitDiffDialog.tsx | 34 +++- packages/web-shell/client/i18n.tsx | 2 + 9 files changed, 365 insertions(+), 66 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-git-diff.test.ts b/packages/cli/src/serve/routes/workspace-git-diff.test.ts index fe178818cf6..f324d3a8cc8 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.test.ts @@ -122,15 +122,18 @@ describe('workspace Git diff routes', () => { }); it('returns single-file hunks for the bound workspace', async () => { - fetchGitDiffHunksForFileMock.mockResolvedValue([ - { - oldStart: 1, - oldLines: 2, - newStart: 1, - newLines: 2, - lines: ['-one', '+ONE', ' two'], - }, - ]); + fetchGitDiffHunksForFileMock.mockResolvedValue({ + hunks: [ + { + oldStart: 1, + oldLines: 2, + newStart: 1, + newLines: 2, + lines: ['-one', '+ONE', ' two'], + }, + ], + truncated: false, + }); const app = express(); registerWorkspaceGitDiffRoutes(app, { boundWorkspace: '/work/main', @@ -142,6 +145,7 @@ describe('workspace Git diff routes', () => { ); expect(response.status).toBe(200); + // `truncated` is intentionally ABSENT (not false) on an untruncated diff. expect(response.body).toEqual({ v: 1, workspaceCwd: '/work/main', @@ -163,6 +167,33 @@ describe('workspace Git diff routes', () => { ); }); + it('surfaces the truncated flag when the diff was capped', async () => { + fetchGitDiffHunksForFileMock.mockResolvedValue({ + hunks: [ + { + oldStart: 0, + oldLines: 0, + newStart: 1, + newLines: 1, + lines: ['+head'], + }, + ], + truncated: true, + }); + const app = express(); + registerWorkspaceGitDiffRoutes(app, { + boundWorkspace: '/work/main', + sendBridgeError, + }); + + const response = await request(app).get( + '/workspace/git/diff/file?path=big.txt', + ); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ available: true, truncated: true }); + }); + it('reports available=false when the file has no diff', async () => { fetchGitDiffHunksForFileMock.mockResolvedValue(null); const app = express(); @@ -194,7 +225,10 @@ describe('workspace Git diff routes', () => { }); it('uses the selected trusted workspace runtime for the file route', async () => { - fetchGitDiffHunksForFileMock.mockResolvedValue([]); + fetchGitDiffHunksForFileMock.mockResolvedValue({ + hunks: [], + truncated: false, + }); const app = express(); const primary = runtime('primary', '/work/main', true); const secondary = runtime('secondary', '/work/secondary', true); diff --git a/packages/cli/src/serve/routes/workspace-git-diff.ts b/packages/cli/src/serve/routes/workspace-git-diff.ts index 06748e4d4ae..9906a0a4ff6 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.ts @@ -8,7 +8,7 @@ import type { Application, Request, Response } from 'express'; import { fetchGitDiff, fetchGitDiffHunksForFile, - type GitDiffHunk, + type GitDiffFileHunks, type GitDiffResult, } from '@qwen-code/qwen-code-core'; import type { SendBridgeError } from '../server/error-response.js'; @@ -78,20 +78,23 @@ function buildDiffList( function buildFileHunks( workspaceCwd: string, queryPath: string, - hunks: GitDiffHunk[] | null, + result: GitDiffFileHunks | null, ): Record { return { v: 1, workspaceCwd, path: queryPath, - available: hunks !== null && hunks.length > 0, - hunks: (hunks ?? []).map((h) => ({ + available: result !== null && result.hunks.length > 0, + hunks: (result?.hunks ?? []).map((h) => ({ oldStart: h.oldStart, oldLines: h.oldLines, newStart: h.newStart, newLines: h.newLines, lines: h.lines, })), + // Only present when the per-file caps actually cut content, so the client + // can label the diff incomplete; absent otherwise (additive to v=1). + ...(result?.truncated ? { truncated: true } : {}), }; } @@ -129,9 +132,9 @@ async function handleDiffFile( return; } try { - const hunks = await fetchGitDiffHunksForFile(workspaceCwd, queryPath); + const result = await fetchGitDiffHunksForFile(workspaceCwd, queryPath); applyReadHeaders(res); - res.status(200).json(buildFileHunks(workspaceCwd, queryPath, hunks)); + res.status(200).json(buildFileHunks(workspaceCwd, queryPath, result)); } catch (err) { sendBridgeError(res, err, { route }); } diff --git a/packages/cli/src/serve/workspace-git-state.test.ts b/packages/cli/src/serve/workspace-git-state.test.ts index fbb8c8aca66..deb61eba79c 100644 --- a/packages/cli/src/serve/workspace-git-state.test.ts +++ b/packages/cli/src/serve/workspace-git-state.test.ts @@ -73,6 +73,29 @@ describe('WorkspaceGitState', () => { await vi.waitFor(() => expect(dispose).toHaveBeenCalledOnce()); }); + it('degrades to the branch-only shape when the working-tree summary throws', async () => { + resolveBranchNameMock.mockResolvedValue('main'); + watchRepoBranchMock.mockResolvedValue(() => {}); + // Not a graceful `null` — an actual rejection (e.g. an unexpected + // child_process failure). The `.catch(() => null)` in getStatus must + // convert it into the branch-only v2 response instead of a thrown error + // that would 500 every `GET /workspace/git`. + getGitWorkingTreeStatusMock.mockRejectedValueOnce( + new Error('git exploded'), + ); + const state = new WorkspaceGitState(); + + await expect( + state.getStatus('/workspace', { + publishWorkspaceEvent: vi.fn(), + } as unknown as AcpSessionBridge), + ).resolves.toEqual({ + v: 2, + workspaceCwd: '/workspace', + branch: 'main', + }); + }); + it('returns null and keeps the watcher shared for a non-git workspace', async () => { resolveBranchNameMock.mockResolvedValue(undefined); watchRepoBranchMock.mockResolvedValue(() => {}); diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index 05cdf82ec55..f91db9d8759 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -484,10 +484,11 @@ describe('fetchGitDiffHunksForFile', () => { await git(repo, 'commit', '-q', '-m', 'init'); await fs.writeFile(path.join(repo, 'a.txt'), 'one\nTWO\nthree\n'); - const hunks = await fetchGitDiffHunksForFile(repo, 'a.txt'); - expect(hunks).not.toBeNull(); - expect(hunks![0].lines.some((l) => l === '-two')).toBe(true); - expect(hunks![0].lines.some((l) => l === '+TWO')).toBe(true); + const result = await fetchGitDiffHunksForFile(repo, 'a.txt'); + expect(result).not.toBeNull(); + expect(result!.truncated).toBe(false); + expect(result!.hunks[0].lines.some((l) => l === '-two')).toBe(true); + expect(result!.hunks[0].lines.some((l) => l === '+TWO')).toBe(true); }); it('scopes the diff to the requested file only', async () => { @@ -498,10 +499,10 @@ describe('fetchGitDiffHunksForFile', () => { await fs.writeFile(path.join(repo, 'a.txt'), 'A\n'); await fs.writeFile(path.join(repo, 'b.txt'), 'B\n'); - const hunks = await fetchGitDiffHunksForFile(repo, 'a.txt'); - expect(hunks![0].lines.some((l) => l === '+A')).toBe(true); + const result = await fetchGitDiffHunksForFile(repo, 'a.txt'); + expect(result!.hunks[0].lines.some((l) => l === '+A')).toBe(true); // b.txt's change must not leak into a.txt's hunks. - expect(hunks![0].lines.some((l) => l === '+B')).toBe(false); + expect(result!.hunks[0].lines.some((l) => l === '+B')).toBe(false); }); it('returns null for an unchanged tracked file', async () => { @@ -518,16 +519,52 @@ describe('fetchGitDiffHunksForFile', () => { await git(repo, 'commit', '-q', '-m', 'init'); await fs.writeFile(path.join(repo, 'new.txt'), 'x\ny\n'); - const hunks = await fetchGitDiffHunksForFile(repo, 'new.txt'); - expect(hunks).not.toBeNull(); - expect(hunks).toHaveLength(1); - expect(hunks![0]).toMatchObject({ + const result = await fetchGitDiffHunksForFile(repo, 'new.txt'); + expect(result).not.toBeNull(); + expect(result!.truncated).toBe(false); + expect(result!.hunks).toHaveLength(1); + expect(result!.hunks[0]).toMatchObject({ oldStart: 0, oldLines: 0, newStart: 1, newLines: 2, }); - expect(hunks![0].lines).toEqual(['+x', '+y']); + expect(result!.hunks[0].lines).toEqual(['+x', '+y']); + }); + + it('reports truncation for an untracked file past the line cap', async () => { + await fs.writeFile(path.join(repo, 'a.txt'), 'a\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + const body = Array.from( + { length: MAX_LINES_PER_FILE + 5 }, + (_, i) => `line-${i}`, + ).join('\n'); + await fs.writeFile(path.join(repo, 'big.txt'), body + '\n'); + + const result = await fetchGitDiffHunksForFile(repo, 'big.txt'); + expect(result).not.toBeNull(); + expect(result!.truncated).toBe(true); + expect(result!.hunks[0].lines).toHaveLength(MAX_LINES_PER_FILE); + // The capped window is the file's head, all-added. + expect(result!.hunks[0].lines[0]).toBe('+line-0'); + }); + + it('reports truncation for a tracked diff past the parser line cap', async () => { + await fs.writeFile(path.join(repo, 'a.txt'), 'seed\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + const body = Array.from( + { length: MAX_LINES_PER_FILE + 5 }, + (_, i) => `line-${i}`, + ).join('\n'); + await fs.writeFile(path.join(repo, 'a.txt'), body + '\n'); + + const result = await fetchGitDiffHunksForFile(repo, 'a.txt'); + expect(result).not.toBeNull(); + expect(result!.truncated).toBe(true); + const total = result!.hunks.reduce((n, h) => n + h.lines.length, 0); + expect(total).toBe(MAX_LINES_PER_FILE); }); it('returns null for a binary untracked file', async () => { @@ -561,12 +598,12 @@ describe('fetchGitDiffHunksForFile', () => { await git(repo, 'commit', '-q', '-m', 'init'); await fs.writeFile(path.join(repo, 'a.txt'), 'one\nTWO\n'); - const hunks = await fetchGitDiffHunksForFile( + const result = await fetchGitDiffHunksForFile( repo, path.join(repo, 'a.txt'), ); - expect(hunks).not.toBeNull(); - expect(hunks![0].lines.some((l) => l === '+TWO')).toBe(true); + expect(result).not.toBeNull(); + expect(result!.hunks[0].lines.some((l) => l === '+TWO')).toBe(true); // An absolute path outside the git root is rejected. const outside = path.join(os.tmpdir(), 'elsewhere.txt'); diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index a34ae9b51ea..f71d1dc8572 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -22,6 +22,16 @@ import { findGitRoot } from './gitUtils.js'; /** Re-export so consumers don't need to depend on `diff` directly. */ export type GitDiffHunk = Hunk; +/** + * A single file's diff hunks plus whether the per-file caps + * (`MAX_DIFF_SIZE_BYTES` / `MAX_LINES_PER_FILE`) actually cut content — so the + * viewer can label the diff as incomplete instead of silently under-reporting. + */ +export interface GitDiffFileHunks { + hunks: Hunk[]; + truncated: boolean; +} + const execFileAsync = promisify(execFile); export interface GitDiffStats { @@ -320,14 +330,16 @@ export async function fetchGitDiffHunks( * * Untracked files (which `git diff HEAD` omits) are synthesized as a single * all-added hunk by reading the file, so the viewer can show new files like any - * other addition. Returns `null` for non-repos, transient states, paths outside - * the repo, binary or unreadable untracked files, and tracked files with no - * changes. + * other addition. `truncated` is set whenever the per-file caps cut content on + * either path (parser cap for tracked diffs, byte/line caps for synthesized + * untracked ones). Returns `null` for non-repos, transient states, paths + * outside the repo, binary or unreadable untracked files, and tracked files + * with no changes. */ export async function fetchGitDiffHunksForFile( cwd: string, filePath: string, -): Promise { +): Promise { const gitRoot = findGitRoot(cwd); if (!gitRoot) return null; const relPath = toRepoRelativePath(gitRoot, filePath); @@ -347,10 +359,14 @@ export async function fetchGitDiffHunksForFile( gitRoot, ); if (diffOut == null) return null; - const parsed = parseGitDiff(diffOut); + const truncatedPaths = new Set(); + const parsed = parseGitDiff(diffOut, truncatedPaths); // A single-file diff yields at most one entry; return its hunks regardless of // the exact header key (which may carry rename / C-style-quote formatting). - if (parsed.size > 0) return parsed.values().next().value ?? []; + if (parsed.size > 0) { + const [key, hunks] = parsed.entries().next().value as [string, Hunk[]]; + return { hunks: hunks ?? [], truncated: truncatedPaths.has(key) }; + } // No tracked diff: synthesize an all-added hunk only for a genuinely // untracked (and not ignored) file, matching the `--exclude-standard` listing @@ -388,13 +404,15 @@ function toRepoRelativePath(gitRoot: string, filePath: string): string | null { /** * Build a single all-added hunk from an untracked file's content, capped by - * `MAX_DIFF_SIZE_BYTES` / `MAX_LINES_PER_FILE`. Binary or unreadable files - * return `null` so the caller surfaces them without an inline diff. + * `MAX_DIFF_SIZE_BYTES` / `MAX_LINES_PER_FILE` — `truncated` reports when + * either cap actually cut content, so the caller can say so instead of + * presenting a silently incomplete file. Binary or unreadable files return + * `null` so the caller surfaces them without an inline diff. */ async function synthesizeUntrackedHunk( gitRoot: string, filePath: string, -): Promise { +): Promise { let fh; try { fh = await open(path.join(gitRoot, filePath), getUntrackedOpenFlags()); @@ -421,16 +439,21 @@ async function synthesizeUntrackedHunk( // Drop the trailing empty element produced by a final newline. if (lines.length > 0 && lines[lines.length - 1] === '') lines.pop(); const capped = lines.slice(0, MAX_LINES_PER_FILE); - if (capped.length === 0) return []; - return [ - { - oldStart: 0, - oldLines: 0, - newStart: 1, - newLines: capped.length, - lines: capped.map((line) => '+' + line), - }, - ]; + const truncated = + st.size > MAX_DIFF_SIZE_BYTES || lines.length > capped.length; + if (capped.length === 0) return { hunks: [], truncated }; + return { + hunks: [ + { + oldStart: 0, + oldLines: 0, + newStart: 1, + newLines: capped.length, + lines: capped.map((line) => '+' + line), + }, + ], + truncated, + }; } catch { return null; } finally { @@ -541,9 +564,15 @@ export function parseGitNumstat(stdout: string): GitDiffResult { * Limits applied: * - Stop once `MAX_FILES` files have been collected. * - Skip files whose raw diff exceeds `MAX_DIFF_SIZE_BYTES`. - * - Truncate per-file content at `MAX_LINES_PER_FILE` lines. + * - Truncate per-file content at `MAX_LINES_PER_FILE` lines; when + * `truncatedPaths` is provided, every file that actually lost lines to that + * cap is recorded there so callers can surface the truncation instead of + * presenting a silently incomplete diff. */ -export function parseGitDiff(stdout: string): Map { +export function parseGitDiff( + stdout: string, + truncatedPaths?: Set, +): Map { const result = new Map(); if (!stdout.trim()) return result; @@ -599,7 +628,12 @@ export function parseGitDiff(stdout: string): Map { line.startsWith('-') || line.startsWith(' ') ) { - if (lineCount >= MAX_LINES_PER_FILE) break; + if (lineCount >= MAX_LINES_PER_FILE) { + // A content line exists beyond the cap, so this file's hunks are + // genuinely incomplete (an exactly-at-cap diff never reaches here). + truncatedPaths?.add(filePath); + break; + } // Force a flat string copy to break V8 sliced-string references so the // whole raw diff can be GC'd once parsing finishes. currentHunk.lines.push('' + line); diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index e4eda78dec9..4b4a6844a3c 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -146,6 +146,12 @@ export interface DaemonWorkspaceGitDiffHunks { /** `false` when the file has no diff (unchanged / binary / untracked-empty). */ available: boolean; hunks: DaemonDiffHunk[]; + /** + * Present (and `true`) when the daemon's per-file caps cut content from + * `hunks`, so the viewer can label the diff incomplete. Absent from older + * daemons and untruncated responses (additive to v=1). + */ + truncated?: boolean; } /** Capabilities envelope returned from `GET /capabilities`. */ diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx index 4351617c18b..d09a4a06242 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -12,25 +12,46 @@ if (!Element.prototype.scrollIntoView) { // A STABLE client object: the dialog's fetch effect depends on `client`, so a // fresh object per render (as a naive mock returns) would re-fire it in a loop. -const { workspaceGitDiff, workspaceGitDiffFile, workspaceClient } = vi.hoisted( - () => { +const { workspaceGitDiff, workspaceGitDiffFile, workspaceClient, shikiState } = + vi.hoisted(() => { const workspaceGitDiff = vi.fn(); const workspaceGitDiffFile = vi.fn(); const workspaceClient = { workspaceByCwd: () => ({ workspaceGitDiff, workspaceGitDiffFile }), }; - return { workspaceGitDiff, workspaceGitDiffFile, workspaceClient }; - }, -); + // Per-test switch for the highlighter path: `resolvedLang` steers whether + // buildRows even asks for a highlighter ('text' skips it), `highlighter` + // (when set) makes getCodeHighlighter resolve instead of reject. + const shikiState = { + resolvedLang: 'text', + highlighter: null as { + codeToTokens: ( + code: string, + opts: { lang: string; theme: string }, + ) => { tokens: Array> }; + } | null, + }; + return { + workspaceGitDiff, + workspaceGitDiffFile, + workspaceClient, + shikiState, + }; + }); vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ useWorkspace: () => ({ client: workspaceClient }), })); -// Shiki's WASM engine isn't available under jsdom; the dialog must degrade to -// plain text. Stub the highlighter so buildRows takes the plain-text path. +// Shiki's WASM engine isn't available under jsdom; by default the stub rejects +// so buildRows takes the plain-text path. A test can install a fake +// highlighter via `shikiState` to exercise the token-interleaving success path. vi.mock('../messages/codeHighlighter', () => ({ - getCodeHighlighter: vi.fn().mockRejectedValue(new Error('no shiki in tests')), + getCodeHighlighter: vi.fn(() => + shikiState.highlighter + ? Promise.resolve(shikiState.highlighter) + : Promise.reject(new Error('no shiki in tests')), + ), isTooLargeToHighlight: () => false, })); @@ -38,7 +59,7 @@ vi.mock('../messages/Markdown', () => ({ resolveFenceLanguage: (lang: string) => ({ label: lang, lang, - resolvedLang: 'text', + resolvedLang: shikiState.resolvedLang, }), })); @@ -74,6 +95,8 @@ afterEach(() => { act(() => root.unmount()); container.remove(); vi.clearAllMocks(); + shikiState.resolvedLang = 'text'; + shikiState.highlighter = null; }); function diffPayload( @@ -228,4 +251,115 @@ describe('GitDiffDialog', () => { expect(workspaceGitDiffFile).toHaveBeenCalledWith('src/a.ts'); expect(document.body.textContent).toContain('Failed to load this diff'); }); + + it('labels a capped file diff as truncated', async () => { + workspaceGitDiff.mockResolvedValue(diffPayload()); + workspaceGitDiffFile.mockResolvedValue({ + v: 1, + workspaceCwd: '/repo', + path: 'src/a.ts', + available: true, + hunks: [ + { + oldStart: 0, + oldLines: 0, + newStart: 1, + newLines: 1, + lines: ['+the visible head of a capped file'], + }, + ], + truncated: true, + }); + mount(); + await flush(); + + const header = document.body.querySelector( + 'button[aria-expanded="false"]', + ) as HTMLButtonElement; + await act(async () => { + header.click(); + }); + await flush(); + + expect(document.body.textContent).toContain('Diff truncated'); + // The visible window still renders above the note. + expect(document.body.textContent).toContain('visible head'); + }); + + it('shows the per-file error when row building rejects on malformed hunks', async () => { + workspaceGitDiff.mockResolvedValue(diffPayload()); + workspaceGitDiffFile.mockResolvedValue({ + v: 1, + workspaceCwd: '/repo', + path: 'src/a.ts', + available: true, + // `lines: null` makes buildRows throw while iterating — the shape a + // buggy daemon could emit. Without the .catch this is an unhandled + // rejection and the diff area silently stays empty. + hunks: [ + { oldStart: 1, oldLines: 1, newStart: 1, newLines: 1, lines: null }, + ], + }); + mount(); + await flush(); + + const header = document.body.querySelector( + 'button[aria-expanded="false"]', + ) as HTMLButtonElement; + await act(async () => { + header.click(); + }); + await flush(); + + expect(document.body.textContent).toContain('Failed to load this diff'); + }); + + it('renders Shiki tokens per side when highlighting succeeds', async () => { + // Steer buildRows onto the highlighter path with a fake tokenizer that + // emits one colored token per line, so the add row pulls from the new-side + // tokens and the del row from the old-side tokens. + shikiState.resolvedLang = 'ts'; + shikiState.highlighter = { + codeToTokens: (code: string) => ({ + tokens: code + .split('\n') + .map((line) => [{ content: line, color: '#ff0000' }]), + }), + }; + workspaceGitDiff.mockResolvedValue(diffPayload()); + workspaceGitDiffFile.mockResolvedValue({ + v: 1, + workspaceCwd: '/repo', + path: 'src/a.ts', + available: true, + hunks: [ + { + oldStart: 1, + oldLines: 1, + newStart: 1, + newLines: 1, + lines: ['-const a = 1', '+const a = 2'], + }, + ], + }); + mount(); + await flush(); + + const header = document.body.querySelector( + 'button[aria-expanded="false"]', + ) as HTMLButtonElement; + await act(async () => { + header.click(); + }); + await flush(); + + const colored = Array.from( + document.body.querySelectorAll('span[style]'), + ).filter((el) => (el as HTMLElement).style.color !== ''); + const texts = colored.map((el) => el.textContent); + // Both sides tokenized: the del row from the old side, the add row from + // the new side — not the plain-text fallback. + expect(texts).toContain('const a = 1'); + expect(texts).toContain('const a = 2'); + }); }); diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index 50ca1e50432..7aa60255039 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -166,21 +166,38 @@ function renderContent(row: DiffRow): ReactNode { } function DiffHunks({ hunks, path }: { hunks: DaemonDiffHunk[]; path: string }) { + const { t } = useI18n(); const theme = useTheme(); const shikiTheme = shikiThemeFor(theme); const [rows, setRows] = useState(null); + const [failed, setFailed] = useState(false); useEffect(() => { let cancelled = false; setRows(null); - void buildRows(hunks, path, shikiTheme).then((built) => { - if (!cancelled) setRows(built); - }); + setFailed(false); + buildRows(hunks, path, shikiTheme) + .then((built) => { + if (!cancelled) setRows(built); + }) + // Highlighter failures degrade to plain text inside buildRows; this + // catches the unexpected (e.g. malformed hunk lines), which would + // otherwise be an unhandled rejection leaving `rows` stuck at null with + // no feedback. + .catch(() => { + if (!cancelled) setFailed(true); + }); return () => { cancelled = true; }; }, [hunks, path, shikiTheme]); + if (failed) { + return ( +
{t('gitDiff.fileError')}
+ ); + } + return (
{(rows ?? []).map((row, index) => ( @@ -217,6 +234,7 @@ function DiffFileRow({ const { client } = useWorkspace(); const [open, setOpen] = useState(false); const [hunks, setHunks] = useState(null); + const [truncated, setTruncated] = useState(false); const [loading, setLoading] = useState(false); const [error, setError] = useState(false); @@ -231,6 +249,7 @@ function DiffFileRow({ .workspaceGitDiffFile(file.path) .then((result) => { setHunks(result.hunks); + setTruncated(result.truncated === true); }) .catch(() => { setError(true); @@ -285,7 +304,14 @@ function DiffFileRow({ {t('gitDiff.fileError')}
) : hunks && hunks.length > 0 ? ( - + <> + + {truncated && ( +
+ {t('gitDiff.truncated')} +
+ )} + ) : (
{t('gitDiff.noDiff')}
)} diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index f34bde05812..25a9a3a0c79 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -43,6 +43,7 @@ const EN: Messages = { 'gitDiff.deleted': 'Deleted', 'gitDiff.noDiff': 'No changes to display', 'gitDiff.fileError': 'Failed to load this diff', + 'gitDiff.truncated': 'Diff truncated — the file is too large to show in full', 'gitDiff.hidden': (v) => `${v?.count ?? 0} more file(s) not shown`, 'gitDiff.expand': (v) => `Show changes for ${v?.path ?? 'file'}`, 'gitDiff.collapse': (v) => `Hide changes for ${v?.path ?? 'file'}`, @@ -2076,6 +2077,7 @@ const ZH: Messages = { 'gitDiff.deleted': '已删除', 'gitDiff.noDiff': '无差异可显示', 'gitDiff.fileError': '加载此差异失败', + 'gitDiff.truncated': '差异已截断——文件过大，未完整显示', 'gitDiff.hidden': (v) => `还有 ${v?.count ?? 0} 个文件未显示`, 'gitDiff.expand': (v) => `显示 ${v?.path ?? '文件'} 的变更`, 'gitDiff.collapse': (v) => `隐藏 ${v?.path ?? '文件'} 的变更`, From d3a0bff6bf1a490e3743c70c987f446c088fad19 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 10:18:54 +0800 Subject: [PATCH 06/24] fix(web-shell): drop dialog backdrop-blur that froze the page on open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dialog and alert-dialog overlays applied `backdrop-blur-xs`, which forces the browser to rasterize and blur the entire content behind the overlay when a dialog opens. With a long transcript behind it, that main-thread paint+blur froze the whole page — e.g. clicking the git branch chip to open the Changes dialog. Keep the bg-black/10 scrim for separation and drop the blur. --- packages/web-shell/client/components/ui/alert-dialog.tsx | 4 +++- packages/web-shell/client/components/ui/dialog.tsx | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/web-shell/client/components/ui/alert-dialog.tsx b/packages/web-shell/client/components/ui/alert-dialog.tsx index 88699e1b5b7..8bc2ac754ea 100644 --- a/packages/web-shell/client/components/ui/alert-dialog.tsx +++ b/packages/web-shell/client/components/ui/alert-dialog.tsx @@ -42,7 +42,9 @@ const AlertDialogOverlay = React.forwardRef< ref={ref} data-slot="alert-dialog-overlay" className={cn( - 'fixed inset-0 z-50 bg-black/10 duration-100 supports-backdrop-filter:backdrop-blur-xs data-open:animate-in data-open:fade-in-0 data-closed:animate-out data-closed:fade-out-0', + // No backdrop-blur: see DialogOverlay — blurring the whole backdrop on + // open freezes the page when a long transcript sits behind it. + 'fixed inset-0 z-50 bg-black/10 duration-100 data-open:animate-in data-open:fade-in-0 data-closed:animate-out data-closed:fade-out-0', className, )} {...props} diff --git a/packages/web-shell/client/components/ui/dialog.tsx b/packages/web-shell/client/components/ui/dialog.tsx index 26360c9e2f4..acee8c4cc42 100644 --- a/packages/web-shell/client/components/ui/dialog.tsx +++ b/packages/web-shell/client/components/ui/dialog.tsx @@ -49,7 +49,11 @@ const DialogOverlay = React.forwardRef< ref={ref} data-slot="dialog-overlay" className={cn( - 'fixed inset-0 isolate z-[var(--web-shell-dialog-backdrop-z-index,50)] bg-black/10 duration-100 supports-backdrop-filter:backdrop-blur-xs data-open:animate-in data-open:fade-in-0 data-closed:animate-out data-closed:fade-out-0', + // No backdrop-blur: it forces the browser to rasterize and blur the + // entire content behind the overlay on open, which freezes the page + // when a long transcript sits behind it. The bg-black/10 scrim keeps + // the visual separation without that cost. + 'fixed inset-0 isolate z-[var(--web-shell-dialog-backdrop-z-index,50)] bg-black/10 duration-100 data-open:animate-in data-open:fade-in-0 data-closed:animate-out data-closed:fade-out-0', className, )} {...props} From 0c9edbd6d68cd77bc2259081882baae1b3678cac Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 11:37:43 +0800 Subject: [PATCH 07/24] fix(core): guard synthesizeUntrackedHunk against non-regular files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit synthesizeUntrackedHunk opened an untracked path before checking its type, so an untracked FIFO (listed by `ls-files --others`) would block on open() forever waiting on a writer — hanging the daemon's event loop and leaving the Web Shell Changes dialog stuck on a permanent loading state. lstat-gate on regular files before opening, matching the existing guard in countUntrackedLines. Adds a FIFO regression test. --- packages/core/src/utils/gitDiff.test.ts | 15 +++++++++++++++ packages/core/src/utils/gitDiff.ts | 12 +++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index f91db9d8759..d53a52842a4 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -576,6 +576,21 @@ describe('fetchGitDiffHunksForFile', () => { expect(await fetchGitDiffHunksForFile(repo, 'blob.bin')).toBeNull(); }); + it.skipIf(process.platform === 'win32')( + 'returns null for an untracked FIFO without hanging', + async () => { + // `ls-files --others` can list a FIFO; open() on it blocks forever + // waiting on a writer. synthesizeUntrackedHunk must lstat-gate so + // expanding it in the diff dialog can't hang the daemon's event loop. + await fs.writeFile(path.join(repo, 'a.txt'), 'a\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + await execFileAsync('mkfifo', [path.join(repo, 'pipe')]); + + expect(await fetchGitDiffHunksForFile(repo, 'pipe')).toBeNull(); + }, + ); + it('returns null for an ignored file', async () => { await fs.writeFile(path.join(repo, 'a.txt'), 'a\n'); await fs.writeFile(path.join(repo, '.gitignore'), 'ignored.log\n'); diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index f71d1dc8572..964f12a5f99 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -413,9 +413,19 @@ async function synthesizeUntrackedHunk( gitRoot: string, filePath: string, ): Promise { + const absPath = path.join(gitRoot, filePath); + // lstat before open: `ls-files --others` can list FIFOs whose open() blocks + // forever waiting on a writer. Gate on regular files (as countUntrackedLines + // does) so expanding an untracked FIFO can't hang the daemon's event loop. + try { + const lst = await lstat(absPath); + if (!lst.isFile()) return null; + } catch { + return null; + } let fh; try { - fh = await open(path.join(gitRoot, filePath), getUntrackedOpenFlags()); + fh = await open(absPath, getUntrackedOpenFlags()); } catch { return null; } From 9786618432200f5eb60e7b88819dd105e8f239ad Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 15:04:49 +0800 Subject: [PATCH 08/24] fix(web-shell,core): rename expansion, no-newline marker, chip measurement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 review Criticals: - core: key renamed diff entries by the real (post-rename) path and carry the old path for display, so renamed rows can be expanded — the synthetic `old => new` key was sent to git as a nonexistent literal path. The diff dialog renders the rename as `old → new`. - core: preserve Git's `\ No newline at end of file` marker through the hunk parser so a trailing-newline-only edit isn't shown as identical removed/added lines (the viewer already renders it as a meta row). - web-shell: the toolbar's hidden git-chip measurement replica now renders the full chip content via the extracted GitBranchChipContent, so the expanded width includes the status indicators and the compact/expanded toggle no longer oscillates near the responsive threshold. --- .../src/serve/routes/workspace-git-diff.ts | 1 + packages/core/src/utils/gitDiff.test.ts | 41 +++++-- packages/core/src/utils/gitDiff.ts | 18 ++- packages/sdk-typescript/src/daemon/types.ts | 3 + .../client/components/ChatEditor.tsx | 16 ++- .../client/components/GitBranchIndicator.tsx | 103 +++++++++++------- .../dialogs/GitDiffDialog.module.css | 4 + .../components/dialogs/GitDiffDialog.tsx | 12 +- 8 files changed, 139 insertions(+), 59 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-git-diff.ts b/packages/cli/src/serve/routes/workspace-git-diff.ts index 9906a0a4ff6..0f16f4da28e 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.ts @@ -53,6 +53,7 @@ function buildDiffList( } const files = [...result.perFileStats.entries()].map(([path, s]) => ({ path, + oldPath: s.oldPath, added: s.added, removed: s.removed, isBinary: s.isBinary, diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index d53a52842a4..168655e8242 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -105,12 +105,15 @@ describe('parseGitNumstat', () => { }); }); - it('combines rename-pair tokens into a single entry', () => { + it('combines rename-pair tokens into a single entry keyed by the new path', () => { // `-z` rename format: `\t\t\0\0\0`. const out = '0\t0\t\0' + 'src/old.ts\0' + 'src/new.ts\0'; const { stats, perFileStats } = parseGitNumstat(out); expect(stats.filesCount).toBe(1); - expect(perFileStats.has('src/old.ts => src/new.ts')).toBe(true); + // Keyed by the current (new) path so the single-file endpoint can address + // it; the old path is carried for display. + expect(perFileStats.has('src/new.ts')).toBe(true); + expect(perFileStats.get('src/new.ts')?.oldPath).toBe('src/old.ts'); }); }); @@ -214,6 +217,23 @@ index 0000000..3333333 expect(bHunks[0].lines).toEqual(['+hello', '+world']); }); + it('preserves the "\\ No newline at end of file" marker', () => { + const diff = `diff --git a/f.txt b/f.txt +--- a/f.txt ++++ b/f.txt +@@ -1 +1 @@ +-line +\\ No newline at end of file ++line +`; + const result = parseGitDiff(diff); + expect(result.get('f.txt')![0].lines).toEqual([ + '-line', + '\\ No newline at end of file', + '+line', + ]); + }); + it('returns empty map on empty input', () => { expect(parseGitDiff('').size).toBe(0); expect(parseGitDiff(' \n').size).toBe(0); @@ -960,7 +980,7 @@ describe('fetchGitDiff tracked-file filename robustness', () => { } }); - it('combines a rename into a single "old => new" per-file entry', async () => { + it('keys a rename by its new path and carries the old path', async () => { const repo = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-gitdiff-mv-')); try { await execFileAsync('git', ['init', '-q', '-b', 'main'], { cwd: repo }); @@ -978,10 +998,10 @@ describe('fetchGitDiff tracked-file filename robustness', () => { const result = await fetchGitDiff(repo); expect(result).not.toBeNull(); - // Rename detection is the git default with -M; we preserve that display - // shape rather than splitting into delete + add rows. - const keys = [...result!.perFileStats.keys()]; - expect(keys.some((k) => k.includes('old.txt => new.txt'))).toBe(true); + // Keyed by the current path so the row can expand; the old path is + // carried for display instead of the synthetic `old => new` string + // (which git cannot address). + expect(result!.perFileStats.get('new.txt')?.oldPath).toBe('old.txt'); } finally { await fs.rm(repo, { recursive: true, force: true }); } @@ -1324,10 +1344,9 @@ describe('fetchGitDiff deletion detection', () => { const result = await fetchGitDiff(repo); expect(result).not.toBeNull(); - // The rename collapses to a single "old => new" entry; it must not - // be flagged as deleted. - const keys = [...result!.perFileStats.keys()]; - expect(keys.some((k) => k.includes('=>'))).toBe(true); + // The rename collapses to a single entry keyed by the new path (old path + // carried for display); it must not be flagged as deleted. + expect(result!.perFileStats.get('new.txt')?.oldPath).toBe('old.txt'); for (const s of result!.perFileStats.values()) { expect(s.isDeleted).toBeFalsy(); } diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index 964f12a5f99..fcda1d74ffb 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -54,6 +54,10 @@ export interface PerFileStats { /** Only meaningful for untracked files: `true` when the file exceeded the * line-counting read cap and `added` is therefore a lower bound. */ truncated?: boolean; + /** For a rename detected by `git diff --numstat -z`, the pre-rename path. + * The map key (and wire `path`) is the current post-rename path so the + * single-file endpoint can address it; this carries the old path for display. */ + oldPath?: string; } export interface GitDiffResult { @@ -508,11 +512,16 @@ export function parseGitNumstat(stdout: string): GitDiffResult { renameOld = token; continue; } + // Key by the current (post-rename) path so the single-file endpoint can + // address it; carry the old path for display. Keying by the synthetic + // `old => new` string sent a nonexistent literal path to git, so renamed + // rows could never expand. commitEntry( - `${renameOld} => ${token}`, + token, pending.added, pending.removed, pending.isBinary, + renameOld, ); pending = null; renameOld = null; @@ -545,6 +554,7 @@ export function parseGitNumstat(stdout: string): GitDiffResult { fileAdded: number, fileRemoved: number, isBinary: boolean, + oldPath?: string, ): void { validFileCount++; added += fileAdded; @@ -554,6 +564,7 @@ export function parseGitNumstat(stdout: string): GitDiffResult { added: fileAdded, removed: fileRemoved, isBinary, + ...(oldPath ? { oldPath } : {}), }); } } @@ -648,6 +659,11 @@ export function parseGitDiff( // whole raw diff can be GC'd once parsing finishes. currentHunk.lines.push('' + line); lineCount++; + } else if (line.startsWith('\\')) { + // "\ No newline at end of file" — metadata the viewer renders as a + // marker. Keep it so a trailing-newline-only edit isn't shown as + // identical removed/added lines. Not counted against the content cap. + currentHunk.lines.push('' + line); } } diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 4b4a6844a3c..103b0abba8a 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -102,6 +102,9 @@ export interface DaemonWorkspaceGitStatus { export interface DaemonWorkspaceGitDiffFile { /** Repo-root-relative path (render after sanitizing — git allows odd bytes). */ path: string; + /** Pre-rename path when this entry is a rename; absent otherwise. `path` is + * the current (post-rename) path used to fetch the per-file diff. */ + oldPath?: string; /** Lines added (`0` for binary files). */ added?: number; /** Lines removed (`0` for binary files). */ diff --git a/packages/web-shell/client/components/ChatEditor.tsx b/packages/web-shell/client/components/ChatEditor.tsx index 76b837b6d58..e4e07562dfb 100644 --- a/packages/web-shell/client/components/ChatEditor.tsx +++ b/packages/web-shell/client/components/ChatEditor.tsx @@ -47,7 +47,7 @@ import { ModeIcon } from './ModeIcon'; import { planSlashSectionRows } from '../utils/slashSectionPlan'; import { getModelDisplayName } from '../utils/modelDisplay'; import { VoiceButton } from '../voice/VoiceButton'; -import { GitBranchIndicator } from './GitBranchIndicator'; +import { GitBranchChipContent, GitBranchIndicator } from './GitBranchIndicator'; import { WorkspaceIndicator } from './WorkspaceIndicator'; import { ChevronDownIcon, FolderClosedIcon } from 'lucide-react'; import { @@ -2440,15 +2440,21 @@ export const ChatEditor = memo( data-toolbar-measure="gitBranch:collapsed" className={`${styles.gitBranchChip} ${styles.gitBranchChipCompact}`} > - - {gitBranch} + - - {gitBranch} + )} diff --git a/packages/web-shell/client/components/GitBranchIndicator.tsx b/packages/web-shell/client/components/GitBranchIndicator.tsx index 3c49c33d320..262e85f66de 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.tsx @@ -74,56 +74,25 @@ function badgeTone(s: DerivedStatus): BadgeTone | null { return null; } -export function GitBranchIndicator({ +/** + * The chip's inner content (icon + branch + status indicators), shared by the + * interactive {@link GitBranchIndicator} and the toolbar's hidden measurement + * replica. The replica must render the same indicators or it under-measures the + * expanded chip, which makes the responsive compact/expanded toggle oscillate. + */ +export function GitBranchChipContent({ branch, status, - compact = false, - onOpenDiff, + compact, }: { branch: string; status?: DaemonWorkspaceGitStatus; - compact?: boolean; - onOpenDiff?: () => void; + compact: boolean; }) { const { t } = useI18n(); const s = deriveStatus(status); - - // Localized state phrases drive both the accessible label and the tooltip, - // so the two never drift apart. - const phrases: string[] = []; - if (s.operation) phrases.push(t(`git.operation.${s.operation}`)); - if (s.detached) phrases.push(t('git.detached')); - if (s.conflicted > 0) - phrases.push(t('git.conflicted', { count: s.conflicted })); - if (s.staged > 0) phrases.push(t('git.staged', { count: s.staged })); - if (s.unstaged > 0) phrases.push(t('git.unstaged', { count: s.unstaged })); - if (s.untracked > 0) phrases.push(t('git.untracked', { count: s.untracked })); - if (s.ahead > 0) phrases.push(t('git.ahead', { count: s.ahead })); - if (s.behind > 0) phrases.push(t('git.behind', { count: s.behind })); - if (s.stashCount > 0) phrases.push(t('git.stash', { count: s.stashCount })); - - const ariaLabel = - phrases.length > 0 - ? `${t('git.currentBranch', { branch })} — ${phrases.join(', ')}` - : status?.computedAt !== undefined - ? `${t('git.currentBranch', { branch })} — ${t('git.clean')}` - : t('git.currentBranch', { branch }); - const tone = badgeTone(s); - - const chipClassName = `${styles.gitBranchChip} ${ - compact ? styles.gitBranchChipCompact : '' - } ${onOpenDiff ? styles.gitBranchChipButton : ''}`; - - const chipDataAttrs = { - 'data-web-shell-git-branch': true, - 'data-detached': s.detached ? 'true' : undefined, - 'data-dirty': s.dirty ? 'true' : undefined, - 'data-operation': s.operation ?? undefined, - 'data-clickable': onOpenDiff ? 'true' : undefined, - } as const; - - const chipInner = ( + return ( <> @@ -168,6 +137,58 @@ export function GitBranchIndicator({ )} ); +} + +export function GitBranchIndicator({ + branch, + status, + compact = false, + onOpenDiff, +}: { + branch: string; + status?: DaemonWorkspaceGitStatus; + compact?: boolean; + onOpenDiff?: () => void; +}) { + const { t } = useI18n(); + const s = deriveStatus(status); + + // Localized state phrases drive both the accessible label and the tooltip, + // so the two never drift apart. + const phrases: string[] = []; + if (s.operation) phrases.push(t(`git.operation.${s.operation}`)); + if (s.detached) phrases.push(t('git.detached')); + if (s.conflicted > 0) + phrases.push(t('git.conflicted', { count: s.conflicted })); + if (s.staged > 0) phrases.push(t('git.staged', { count: s.staged })); + if (s.unstaged > 0) phrases.push(t('git.unstaged', { count: s.unstaged })); + if (s.untracked > 0) phrases.push(t('git.untracked', { count: s.untracked })); + if (s.ahead > 0) phrases.push(t('git.ahead', { count: s.ahead })); + if (s.behind > 0) phrases.push(t('git.behind', { count: s.behind })); + if (s.stashCount > 0) phrases.push(t('git.stash', { count: s.stashCount })); + + const ariaLabel = + phrases.length > 0 + ? `${t('git.currentBranch', { branch })} — ${phrases.join(', ')}` + : status?.computedAt !== undefined + ? `${t('git.currentBranch', { branch })} — ${t('git.clean')}` + : t('git.currentBranch', { branch }); + + const chipClassName = `${styles.gitBranchChip} ${ + compact ? styles.gitBranchChipCompact : '' + } ${onOpenDiff ? styles.gitBranchChipButton : ''}`; + + const chipDataAttrs = { + 'data-web-shell-git-branch': true, + 'data-detached': s.detached ? 'true' : undefined, + 'data-dirty': s.dirty ? 'true' : undefined, + 'data-operation': s.operation ?? undefined, + 'data-clickable': onOpenDiff ? 'true' : undefined, + } as const; + + const chipInner = ( + + ); return ( diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.module.css b/packages/web-shell/client/components/dialogs/GitDiffDialog.module.css index 676f316c6c4..97e0fb94dad 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.module.css +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.module.css @@ -62,6 +62,10 @@ white-space: nowrap; } +.fileOldPath { + color: var(--muted-foreground); +} + .fileTag { flex-shrink: 0; padding: 0 6px; diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index 7aa60255039..27e92ac3017 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -270,7 +270,9 @@ function DiffFileRow({ onClick={toggle} aria-expanded={open} aria-label={t(open ? 'gitDiff.collapse' : 'gitDiff.expand', { - path: displayName, + path: file.oldPath + ? `${sanitizeControlChars(file.oldPath)} → ${displayName}` + : displayName, })} > @@ -284,6 +286,14 @@ function DiffFileRow({ )} + {file.oldPath ? ( + <> + + {sanitizeControlChars(file.oldPath)} + + {' → '} + + ) : null} {displayName} {file.isUntracked && ( From a27840e37463d29c9f05dec393a2e957053bd764 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 15:35:29 +0800 Subject: [PATCH 09/24] fix(build): generate git-commit info even when prepare build is skipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review tooling runs `npm ci` with QWEN_SKIP_PREPARE=1 (to skip the heavy prepare build) and then builds only the changed workspaces. Because `prepare` exited before generating the gitignored git-commit.ts, a per-workspace build of packages/cli failed at the unchanged systemInfo.ts on the missing `../generated/git-commit.js` module. Generate the git-commit info in the skip path too — it is cheap and never fails hard — so a later per-workspace build or typecheck finds the module. The non-skip path still generates it via `npm run build`. --- scripts/prepare.js | 10 +++- scripts/tests/package-scripts.test.js | 66 +++++++++++++++++++++------ 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/scripts/prepare.js b/scripts/prepare.js index e55623afb81..ddd8d9af844 100644 --- a/scripts/prepare.js +++ b/scripts/prepare.js @@ -13,7 +13,15 @@ const skipPrepare = ['1', 'true'].includes( ); if (skipPrepare) { - console.log('Skipping prepare because QWEN_SKIP_PREPARE is set.'); + // The heavy build/bundle/husky are skipped, but git-commit.ts (gitignored, + // imported by e.g. cli's systemInfo) is still required to build or typecheck + // the packages that import it. Generate it here so a later per-workspace + // build/typecheck — such as the review tooling's — doesn't fail on the + // missing module. The non-skip path generates it via `npm run build`. + run('npm', ['run', 'generate']); + console.log( + 'Skipping prepare build/bundle/husky because QWEN_SKIP_PREPARE is set.', + ); process.exit(0); } diff --git a/scripts/tests/package-scripts.test.js b/scripts/tests/package-scripts.test.js index 5236a4160d4..e04387f7005 100644 --- a/scripts/tests/package-scripts.test.js +++ b/scripts/tests/package-scripts.test.js @@ -104,26 +104,64 @@ describe('package scripts', () => { expect(vscodePackageJson.scripts['test:ci']).toContain('--coverage'); }); - it('can skip root prepare work for CI installs that build explicitly', () => { + it('skips build/bundle/husky but still generates git-commit info when CI builds explicitly', () => { const packageJson = readPackageJson(); expect(packageJson.scripts.prepare).toBe('node scripts/prepare.js'); - const result = spawnSync( - process.execPath, - [path.join(root, 'scripts/prepare.js')], - { - cwd: root, - encoding: 'utf8', - env: { - ...process.env, - QWEN_SKIP_PREPARE: '1', + const binDir = mkdtempSync(path.join(tmpdir(), 'qwen-prepare-skip-')); + const logFile = path.join(binDir, 'commands.log'); + writeFileSync(logFile, ''); + + try { + if (process.platform === 'win32') { + writeFileSync( + path.join(binDir, 'husky.cmd'), + '@echo husky >> "%PREPARE_LOG_FILE%"\r\n', + ); + writeFileSync( + path.join(binDir, 'npm.cmd'), + '@echo npm %* >> "%PREPARE_LOG_FILE%"\r\n', + ); + } else { + writeFileSync( + path.join(binDir, 'husky'), + '#!/bin/sh\necho husky >> "$PREPARE_LOG_FILE"\n', + ); + writeFileSync( + path.join(binDir, 'npm'), + '#!/bin/sh\necho "npm $*" >> "$PREPARE_LOG_FILE"\n', + ); + chmodSync(path.join(binDir, 'husky'), 0o755); + chmodSync(path.join(binDir, 'npm'), 0o755); + } + + const result = spawnSync( + process.execPath, + [path.join(root, 'scripts/prepare.js')], + { + cwd: root, + encoding: 'utf8', + env: { + ...process.env, + PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ''}`, + PREPARE_LOG_FILE: logFile, + QWEN_SKIP_PREPARE: '1', + }, }, - }, - ); + ); - expect(result.status).toBe(0); - expect(result.stdout).toContain('Skipping prepare'); + expect(result.status).toBe(0); + expect(result.stdout).toContain('Skipping prepare'); + // git-commit info is still generated so a later per-workspace build or + // typecheck (e.g. the review tooling's) doesn't fail on the missing + // module; the heavy build/bundle/husky are skipped. + expect(readFileSync(logFile, 'utf8').trim().split(/\r?\n/)).toEqual([ + 'npm run generate', + ]); + } finally { + rmSync(binDir, { recursive: true, force: true }); + } }); it('runs prepare steps in order when CI does not skip prepare', () => { From 39e971424df0f34619df24b4921633b74701a63a Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 17 Jul 2026 23:00:31 +0800 Subject: [PATCH 10/24] fix(web-shell,cli): address round-6 review suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cli: carry the pre-rename path (oldPath) through DiffRenderRow and show renamed files as `old → new` in both the Ink and plain-text renderers. The rename-keying fix updated the daemon and web-shell dialog but not the CLI `/diff` renderer, which silently dropped the old path. - web-shell: key DiffFileRow by workspace + path so switching workspace remounts the row instead of reusing another workspace's hunks/open state for a path both workspaces share. - web-shell: show a loading placeholder in DiffHunks while rows are (re)built (e.g. after a theme switch) instead of an empty, jumpily-resized box. - web-shell: cover the /diff local intercept in App.test.tsx (opens the Changes dialog and is not forwarded to the agent). --- .../cli/src/ui/commands/diffCommand.test.ts | 22 +++++++++++++++ packages/cli/src/ui/commands/diffCommand.ts | 11 ++++++-- .../components/messages/DiffStatsDisplay.tsx | 6 ++++ packages/cli/src/ui/types.ts | 3 ++ packages/web-shell/client/App.test.tsx | 28 +++++++++++++++++++ .../components/dialogs/GitDiffDialog.tsx | 11 +++++++- 6 files changed, 78 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/ui/commands/diffCommand.test.ts b/packages/cli/src/ui/commands/diffCommand.test.ts index 4900000b423..52f1deefcd8 100644 --- a/packages/cli/src/ui/commands/diffCommand.test.ts +++ b/packages/cli/src/ui/commands/diffCommand.test.ts @@ -154,6 +154,28 @@ describe('diffCommand', () => { expect(content).toContain('src/b.ts'); }); + it('shows renamed files as `old → new`', async () => { + if (!diffCommand.action) throw new Error('Command has no action'); + mockFetchGitDiff.mockResolvedValue({ + stats: { filesCount: 1, linesAdded: 2, linesRemoved: 1 }, + perFileStats: new Map([ + [ + 'src/new-name.ts', + { + added: 2, + removed: 1, + isBinary: false, + oldPath: 'src/old-name.ts', + }, + ], + ]), + } satisfies GitDiffResult); + const result = await diffCommand.action(mockContext, ''); + const content = (result as { content: string }).content; + const row = content.split('\n').find((l) => l.includes('new-name.ts'))!; + expect(row).toContain('src/old-name.ts → src/new-name.ts'); + }); + it('shows untracked text files with their line count and a (new) marker', async () => { if (!diffCommand.action) throw new Error('Command has no action'); mockFetchGitDiff.mockResolvedValue({ diff --git a/packages/cli/src/ui/commands/diffCommand.ts b/packages/cli/src/ui/commands/diffCommand.ts index e1bcc1d5f5f..3b39a298be2 100644 --- a/packages/cli/src/ui/commands/diffCommand.ts +++ b/packages/cli/src/ui/commands/diffCommand.ts @@ -118,6 +118,7 @@ function toRow(filename: string, s: PerFileStats): DiffRenderRow { if (s.isBinary) { return { filename, + oldPath: s.oldPath, isBinary: true, isUntracked: Boolean(s.isUntracked), isDeleted: Boolean(s.isDeleted), @@ -126,6 +127,7 @@ function toRow(filename: string, s: PerFileStats): DiffRenderRow { } return { filename, + oldPath: s.oldPath, added: s.added, removed: s.isUntracked ? 0 : s.removed, isBinary: false, @@ -212,13 +214,18 @@ function formatRowsText(rows: DiffRenderRow[]): string[] { // moves, full screen clears, or layout-breaking newlines into CI logs and // any consumer's terminal. const safeName = sanitizeFilenameForDisplay(r.filename); + // Renames carry the pre-rename path; show `old → new` so the move is + // visible (the row is still addressed by the new path). + const displayName = r.oldPath + ? `${sanitizeFilenameForDisplay(r.oldPath)} → ${safeName}` + : safeName; if (r.isBinary) { const suffix = r.isUntracked ? ` ${t('(binary, new)')}` : r.isDeleted ? ` ${t('(binary, deleted)')}` : ` ${t('(binary)')}`; - out.push(` ${padMarker('~', statColumnWidth)} ${safeName}${suffix}`); + out.push(` ${padMarker('~', statColumnWidth)} ${displayName}${suffix}`); continue; } const added = `+${String(r.added ?? 0).padStart(addWidth)}`; @@ -229,7 +236,7 @@ function formatRowsText(rows: DiffRenderRow[]): string[] { } else if (r.isDeleted) { suffix = ` ${t('(deleted)')}`; } - out.push(` ${added} ${removed} ${safeName}${suffix}`); + out.push(` ${added} ${removed} ${displayName}${suffix}`); } return out; } diff --git a/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx b/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx index 05c4aeb066c..3b49f1cf143 100644 --- a/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx +++ b/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx @@ -94,6 +94,9 @@ const DiffRow: React.FC = ({ {' '} {marker} {' '} + {row.oldPath ? ( + {row.oldPath} → + ) : null} {row.filename} {suffix} @@ -116,6 +119,9 @@ const DiffRow: React.FC = ({ -{removed} {' '} + {row.oldPath ? ( + {row.oldPath} → + ) : null} {row.filename} {suffix && {suffix}} diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index ef88bd09826..d5f88c11d91 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -227,6 +227,9 @@ export type HistoryItemStats = HistoryItemBase & { */ export interface DiffRenderRow { filename: string; + /** Pre-rename path when this row is a rename; absent otherwise. `filename` + * is the current (post-rename) path used to address the file. */ + oldPath?: string; /** `undefined` for binary files; a line count (lower bound if `truncated`) * otherwise. */ added?: number; diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 1b3bad25138..0207795e8ae 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -391,6 +391,18 @@ vi.mock('./components/dialogs/ModelDialog', async () => { }; }); +// The /diff intercept opens this dialog; render it through the (mocked) +// DialogShell so tests can detect it via [data-testid="dialog-shell"] without +// exercising the dialog's diff-fetching hooks. +vi.mock('./components/dialogs/GitDiffDialog', async () => { + const React = await import('react'); + const { DialogShell } = await import('./components/dialogs/DialogShell'); + return { + GitDiffDialog: () => + React.createElement(DialogShell, null, 'changes dialog'), + }; +}); + // Render DialogShell as an observable container so tests can detect an open // sub-dialog (model picker, approval-mode picker) via [data-testid="dialog-shell"]. vi.mock('./components/dialogs/DialogShell', async () => { @@ -3620,6 +3632,22 @@ describe('App session callbacks', () => { expect(container.querySelector('[data-testid="dialog-shell"]')).toBeNull(); }); + it('opens the Changes dialog for /diff and does not forward it to the agent', async () => { + // /diff is intercepted locally — it opens the working-tree Changes dialog + // rather than being forwarded to the daemon/agent as a prompt. + const { container } = renderApp(); + await flush(); + + testState.prompt = '/diff'; + await clickSubmit(container); + await flush(); + + expect( + container.querySelector('[data-testid="dialog-shell"]'), + ).not.toBeNull(); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + }); + it('moves focus to the approval overlay when it appears', async () => { const { rerender } = renderApp(); await flush(); diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index 27e92ac3017..d9bd0b6c2e9 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -198,6 +198,12 @@ function DiffHunks({ hunks, path }: { hunks: DaemonDiffHunk[]; path: string }) { ); } + // null while the rows are first built and again while re-tokenizing after a + // theme switch; show a placeholder instead of an empty, jumpily-resized box. + if (rows === null) { + return
{t('gitDiff.loading')}
; + } + return (
{(rows ?? []).map((row, index) => ( @@ -388,7 +394,10 @@ export function GitDiffDialog({
{diff.files.map((file) => ( From 62379e95e7a16d3969bb30ffd426c2f9893c8585 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 00:19:11 +0800 Subject: [PATCH 11/24] fix(web-shell,cli,core): address round-7 review suggestions - cli: sanitize the rendered filename (and pre-rename oldPath) in the Ink DiffStatsDisplay via sanitizeFilenameForDisplay, matching the plain-text renderer so a crafted path can't inject into the interactive view. - cli: apply the read headers before awaiting the per-file diff fetch (as handleDiffList does) so error responses also carry no-store/nosniff. - cli + web-shell: strip Unicode bidi embedding/isolate controls (U+202A-202E, U+2066-2069) in the filename/control-char sanitizers so a crafted filename can't visually spoof its extension. - core: guard countStashEntries with an lstat type check before readFile, so a symlink-to-FIFO at logs/refs/stash can't block the event loop (the same hazard already guarded in the untracked-file readers). - core: cover fetchGitDiffHunksForFile's transient-state guard with a test (the sibling helpers already had one). --- .../src/serve/routes/workspace-git-diff.ts | 5 ++++- .../components/messages/DiffStatsDisplay.tsx | 20 +++++++++++++------ packages/cli/src/ui/utils/textUtils.ts | 8 ++++++-- packages/core/src/utils/gitDiff.test.ts | 14 +++++++++++++ packages/core/src/utils/gitDiff.ts | 15 ++++++++++---- .../components/messages/toolFormatting.ts | 10 +++++++--- 6 files changed, 56 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-git-diff.ts b/packages/cli/src/serve/routes/workspace-git-diff.ts index 0f16f4da28e..a310bad0acf 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.ts @@ -133,8 +133,11 @@ async function handleDiffFile( return; } try { - const result = await fetchGitDiffHunksForFile(workspaceCwd, queryPath); + // Apply the read headers before the await (as handleDiffList does) so the + // no-store/nosniff headers are also present on the error response if the + // fetch throws. applyReadHeaders(res); + const result = await fetchGitDiffHunksForFile(workspaceCwd, queryPath); res.status(200).json(buildFileHunks(workspaceCwd, queryPath, result)); } catch (err) { sendBridgeError(res, err, { route }); diff --git a/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx b/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx index 3b49f1cf143..e4fe23de57b 100644 --- a/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx +++ b/packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx @@ -9,6 +9,7 @@ import { Box, Text } from 'ink'; import { theme } from '../../semantic-colors.js'; import type { DiffRenderModel, DiffRenderRow } from '../../types.js'; import { computeDiffColumnWidths } from '../../commands/diffCommand.js'; +import { sanitizeFilenameForDisplay } from '../../utils/textUtils.js'; import { t } from '../../../i18n/index.js'; interface DiffStatsDisplayProps { @@ -81,6 +82,13 @@ const DiffRow: React.FC = ({ remWidth, statColumnWidth, }) => { + // Sanitize hostile filenames (control chars / ANSI) the same way the + // plain-text renderer does, so the interactive view can't be injected via a + // crafted path. + const safeName = sanitizeFilenameForDisplay(row.filename); + const safeOldPath = row.oldPath + ? sanitizeFilenameForDisplay(row.oldPath) + : null; if (row.isBinary) { const marker = padRight('~', statColumnWidth); const suffix = row.isUntracked @@ -94,10 +102,10 @@ const DiffRow: React.FC = ({ {' '} {marker} {' '} - {row.oldPath ? ( - {row.oldPath} → + {safeOldPath ? ( + {safeOldPath} → ) : null} - {row.filename} + {safeName} {suffix} @@ -119,10 +127,10 @@ const DiffRow: React.FC = ({ -{removed} {' '} - {row.oldPath ? ( - {row.oldPath} → + {safeOldPath ? ( + {safeOldPath} → ) : null} - {row.filename} + {safeName} {suffix && {suffix}} diff --git a/packages/cli/src/ui/utils/textUtils.ts b/packages/cli/src/ui/utils/textUtils.ts index 6968c0ac280..e6bf1050560 100644 --- a/packages/cli/src/ui/utils/textUtils.ts +++ b/packages/cli/src/ui/utils/textUtils.ts @@ -459,8 +459,12 @@ export function sanitizeSensitiveText( // `escapeAnsiCtrlCodes` only neutralizes multi-byte ANSI sequences; raw single // bytes like `\n`, `\r`, BEL, BS slip past it and can still break layouts or // inject terminal effects when rendered as part of a git-supplied filename. -// eslint-disable-next-line no-control-regex -const FILENAME_CONTROL_CHARS_REGEX = /[\x00-\x1f\x7f-\x9f]/g; +// The Unicode bidi embedding/isolate controls (U+202A–202E, U+2066–2069) are +// also stripped so a crafted filename can't visually spoof its extension. +/* eslint-disable no-control-regex */ +const FILENAME_CONTROL_CHARS_REGEX = + /[\x00-\x1f\x7f-\x9f\u202a-\u202e\u2066-\u2069]/g; +/* eslint-enable no-control-regex */ // Same as FILENAME_CONTROL_CHARS_REGEX minus `\n` (row separator) and `\t` // (benign indentation), which multi-line display treats as layout. diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index 168655e8242..f9639f56cfa 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -533,6 +533,20 @@ describe('fetchGitDiffHunksForFile', () => { expect(await fetchGitDiffHunksForFile(repo, 'a.txt')).toBeNull(); }); + it('returns null during a transient merge state', async () => { + await fs.writeFile(path.join(repo, 'a.txt'), 'one\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + await fs.writeFile(path.join(repo, 'a.txt'), 'TWO\n'); + // Fake a merge in progress; the single-file endpoint must decline just + // like fetchGitDiff/fetchGitDiffHunks do. + await fs.writeFile( + path.join(repo, '.git', 'MERGE_HEAD'), + '0000000000000000000000000000000000000000\n', + ); + expect(await fetchGitDiffHunksForFile(repo, 'a.txt')).toBeNull(); + }); + it('synthesizes an all-added hunk for an untracked file', async () => { await fs.writeFile(path.join(repo, 'a.txt'), 'a\n'); await git(repo, 'add', '.'); diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index fcda1d74ffb..25b25e5003a 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -1344,11 +1344,18 @@ async function detectGitOperation( async function countStashEntries(gitRoot: string): Promise { const gitDir = await resolveGitDirFromRoot(gitRoot); if (!gitDir) return 0; + const stashLog = path.join(gitDir, 'logs', 'refs', 'stash'); + // lstat before read: a symlink-to-FIFO would block readFile forever (the + // same hazard the untracked-file readers guard against). Only count a + // regular file. try { - const content = await readFile( - path.join(gitDir, 'logs', 'refs', 'stash'), - 'utf8', - ); + const st = await lstat(stashLog); + if (!st.isFile()) return 0; + } catch { + return 0; + } + try { + const content = await readFile(stashLog, 'utf8'); return content.split('\n').filter((line) => line.trim().length > 0).length; } catch { return 0; diff --git a/packages/web-shell/client/components/messages/toolFormatting.ts b/packages/web-shell/client/components/messages/toolFormatting.ts index 89c2ba36b88..49b1fb2ade4 100644 --- a/packages/web-shell/client/components/messages/toolFormatting.ts +++ b/packages/web-shell/client/components/messages/toolFormatting.ts @@ -69,9 +69,13 @@ export const TOOL_DISPLAY_NAMES: Record = { * collapse whitespace before rendering single-line labels. */ // Matches bare C0/C1 control bytes but not `\n`/`\t` (mirrors the CLI's -// MULTILINE_CONTROL_CHARS_REGEX). -// eslint-disable-next-line no-control-regex -const CONTROL_CHARS_REGEX = /[\x00-\x08\x0b-\x1f\x7f-\x9f]/g; +// MULTILINE_CONTROL_CHARS_REGEX), plus the Unicode bidi embedding/isolate +// controls (U+202A–202E, U+2066–2069) so a crafted filename can't visually +// reorder or spoof its extension (bidi/"trojan source" style attacks). +/* eslint-disable no-control-regex */ +const CONTROL_CHARS_REGEX = + /[\x00-\x08\x0b-\x1f\x7f-\x9f\u202a-\u202e\u2066-\u2069]/g; +/* eslint-enable no-control-regex */ export function sanitizeControlChars(text: string): string { return text.replace(CONTROL_CHARS_REGEX, (ch) => { From ab3f3e858815ae7b638f290f977d94fc8f9d400c Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 01:17:57 +0800 Subject: [PATCH 12/24] fix(web-shell,cli,core): address round-8 review suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - core: pass --no-optional-locks to the ls-files call in fetchGitDiffHunksForFile, matching the other runGit calls so it doesn't contend for an optional index-refresh lock alongside concurrent git add/commit. - cli: add a route test asserting a rename's oldPath survives serialization end-to-end (keyed by the new path, old path carried alongside). - web-shell: add a GitDiffDialog test for the hiddenCount>0 "N more files not shown" note (every payload previously used hiddenCount: 0). - web-shell: drop the nonexistent primaryLabel prop from the WorkspaceSection test (it is not a WorkspaceSectionProps member). - docs: correct the plan doc — large-diff virtual scrolling was explicitly descoped (core caps + per-file lazy loading), not implemented in Phase 2. --- .../2026-07-16-webshell-git-integration.md | 16 +++++---- .../serve/routes/workspace-git-diff.test.ts | 36 +++++++++++++++++++ packages/core/src/utils/gitDiff.ts | 9 ++++- .../components/dialogs/GitDiffDialog.test.tsx | 11 +++++- .../sidebar/WorkspaceSection.test.tsx | 1 - 5 files changed, 64 insertions(+), 9 deletions(-) diff --git a/docs/plans/2026-07-16-webshell-git-integration.md b/docs/plans/2026-07-16-webshell-git-integration.md index 8eb9d5f2f1a..704b662f824 100644 --- a/docs/plans/2026-07-16-webshell-git-integration.md +++ b/docs/plans/2026-07-16-webshell-git-integration.md @@ -11,7 +11,8 @@ > Web Shell 只做展示与受控操作。 > > **Tech Stack:** TypeScript、React、Node `child_process`（git/gh）、Vitest、 -> Shiki（diff 高亮）、`virtual-viewport`（大 diff 虚拟化）。 +> Shiki（diff 高亮）。大 diff 靠 core 端上限 + 单文件懒加载控制，本期不引入 +> 虚拟滚动（见设计文档"本期不引入虚拟滚动"）。 第一层、第二层的接口与组件细节见设计文档 `docs/design/2026-07-16-webshell-git-status-diff.md`；本文档是全阶段路线图与 @@ -83,8 +84,9 @@ branch chip 从“分支名”升级为“实时状态条”。 `GET /workspace/git/diff/file?path=`（单文件 hunk）。 - SDK：`DaemonWorkspaceGitDiff` / `DaemonWorkspaceGitDiffHunks` 类型 + `workspaceGitDiff()` / `workspaceGitDiffFile(path)`。 -- Web Shell：`GitDiffDialog`（文件列表 → 点开按需加载行级 diff，Shiki 高亮， - 大 diff 虚拟滚动）；`/diff` 本地化打开该弹窗；dirty chip 点击联动。 +- Web Shell：`GitDiffDialog`（文件列表 → 点开按需加载行级 diff，Shiki 高亮； + 大 diff 靠 core 上限 + 单文件懒加载控制，本期不引入虚拟滚动）；`/diff` + 本地化打开该弹窗；dirty chip 点击联动。 详见设计文档第二层。 @@ -174,8 +176,9 @@ daemon 封装 `gh` CLI（需 workspace 内 `gh` 已认证），暴露 REST： 调用（in-flight coalescing）。 - **diff 缓存**：diff 只在文件变化时变 → 缓存 + 在 `git_branch_changed` / focus / index 变化时失效。 -- **大 diff 虚拟化**：复用 `docs/design/virtual-viewport` 的虚拟滚动；单文件 - 按需加载（Phase 2 已设计）；hunk 分页。 +- **大 diff 控制**：本期靠 core 端文件数/行数上限 + 单文件按需加载（Phase 2 + 已落地）+ hunk 分页；虚拟滚动（`virtual-viewport`）已明确不在本期范围，留待 + 后续。 - **离屏解析**：diff 解析 / Shiki 高亮可放 web worker，避免阻塞主线程。 - **请求取消 / 背压**：切走 workspace / 关闭弹窗时 abort 在途 git/gh 子进程； 高频刷新（focus、连续 SSE）做 debounce + 丢弃过期响应（last-write-wins，按 @@ -252,7 +255,8 @@ Phase 7 远程同步 - Phase 3 依赖 Phase 2 的 diff 查看器 UI（复用同一弹窗，加视图切换）。 - Phase 4/5 的写操作依赖 Phase 1-3 的只读基础与确认弹窗模式。 - Phase 6 相对独立（依赖 `gh` 认证），可在 Phase 3 之后并行推进。 -- 横切优化按需在对应阶段落地（如虚拟滚动随 Phase 2、index watch 随 Phase 1）。 +- 横切优化按需在对应阶段落地（如 index watch 随 Phase 1；虚拟滚动已降级为 + 后续项，不随 Phase 2）。 - **信任门控是写操作的安全网**：Phase 4（commit/丢弃）、Phase 6（gh，可能泄露 仓库信息）、Phase 7（push）落地时必须接入 workspace 信任级别（复用 `requireTrustedWorkspaceRuntime`）；读操作（status/diff）可放开。这应在对应 diff --git a/packages/cli/src/serve/routes/workspace-git-diff.test.ts b/packages/cli/src/serve/routes/workspace-git-diff.test.ts index f324d3a8cc8..573bce5ce97 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.test.ts @@ -107,6 +107,42 @@ describe('workspace Git diff routes', () => { expect(fetchGitDiffMock).toHaveBeenCalledWith('/work/main'); }); + it('carries the pre-rename oldPath through the file list', async () => { + fetchGitDiffMock.mockResolvedValue({ + stats: { filesCount: 1, linesAdded: 2, linesRemoved: 1 }, + perFileStats: new Map([ + [ + 'src/new.ts', + { added: 2, removed: 1, isBinary: false, oldPath: 'src/old.ts' }, + ], + ]), + }); + const app = express(); + registerWorkspaceGitDiffRoutes(app, { + boundWorkspace: '/work/main', + sendBridgeError, + }); + + const response = await request(app).get('/workspace/git/diff'); + + expect(response.status).toBe(200); + // The rename must survive serialization keyed by the new path with the old + // path carried alongside, so both the Web Shell dialog and CLI can render + // `old → new`. + expect(response.body.files).toEqual([ + { + path: 'src/new.ts', + oldPath: 'src/old.ts', + added: 2, + removed: 1, + isBinary: false, + isUntracked: false, + isDeleted: false, + truncated: false, + }, + ]); + }); + it('reports available=false when the bound workspace is not a repo', async () => { fetchGitDiffMock.mockResolvedValue(null); const app = express(); diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index 25b25e5003a..f422208f5dc 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -377,7 +377,14 @@ export async function fetchGitDiffHunksForFile( // that drives the diff file list. A tracked-but-unchanged or ignored file // yields nothing here and returns null. const untrackedOut = await runGit( - ['ls-files', '--others', '--exclude-standard', '--', relPath], + [ + '--no-optional-locks', + 'ls-files', + '--others', + '--exclude-standard', + '--', + relPath, + ], gitRoot, ); if (untrackedOut && untrackedOut.trim().length > 0) { diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx index d09a4a06242..c89c77b10ce 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -103,6 +103,7 @@ function diffPayload( overrides: Partial<{ available: boolean; files: Array>; + hiddenCount: number; }> = {}, ) { const files = overrides.files ?? [ @@ -124,7 +125,7 @@ function diffPayload( linesAdded: 2, linesRemoved: 1, files, - hiddenCount: 0, + hiddenCount: overrides.hiddenCount ?? 0, }; } @@ -140,6 +141,14 @@ describe('GitDiffDialog', () => { expect(document.body.textContent).toContain('-1'); }); + it('shows a truncation note when more files are hidden', async () => { + workspaceGitDiff.mockResolvedValue(diffPayload({ hiddenCount: 3 })); + mount(); + await flush(); + + expect(document.body.textContent).toContain('3 more file(s) not shown'); + }); + it('loads and renders a file diff when expanded', async () => { workspaceGitDiff.mockResolvedValue(diffPayload()); workspaceGitDiffFile.mockResolvedValue({ diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx index c44cd5dd899..f3c9f670b61 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx @@ -77,7 +77,6 @@ function renderSection( workspace={overrides.workspace ?? trustedWorkspace} client={makeClient()} reloadToken={0} - primaryLabel="Primary" untrustedLabel="Untrusted" readOnlyLabel="Read-only" trustToOpenLabel="Trust to open" From 6c690c56a1a8825316e90a8dff78aac35ec81d74 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 05:55:33 +0800 Subject: [PATCH 13/24] fix(cli,web-shell): address round-9 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cli: propagate the pre-rename oldPath through DiffDialog's perFileToUnified and render renamed files as `old → new` in the interactive diff viewer (the rename-keying fix had updated the daemon, the web-shell dialog, and the /diff stats, but not this viewer). - cli: cover DiffStatsDisplay's rename (`old → new`) rendering and the sanitizeFilenameForDisplay path for hostile filenames carrying control characters. - web-shell: guard the GitBranchIndicator test afterEach against double-unmounting an already-unmounted root (the localization tests assert on getTranslator without calling render()). --- packages/cli/src/ui/components/DiffDialog.tsx | 18 +++++++ .../messages/DiffStatsDisplay.test.tsx | 53 +++++++++++++++++++ .../components/GitBranchIndicator.test.tsx | 10 +++- 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/ui/components/DiffDialog.tsx b/packages/cli/src/ui/components/DiffDialog.tsx index b8d27a380db..08baede19ac 100644 --- a/packages/cli/src/ui/components/DiffDialog.tsx +++ b/packages/cli/src/ui/components/DiffDialog.tsx @@ -42,6 +42,9 @@ type UnifiedFile = { path: string; /** Sanitized version of `path` safe to drop into a `` node. */ displayPath: string; + /** Sanitized pre-rename path for renames; absent otherwise. `displayPath` + * is the current (post-rename) path used to address the file. */ + oldDisplayPath?: string; added: number; removed: number; isBinary: boolean; @@ -479,6 +482,16 @@ function FileRow({ bold={selected} > {pointer} + + {file.oldDisplayPath ? ( + + {truncatePathStart(file.oldDisplayPath, maxPathChars)} →{' '} + + ) : null} + {path} {tag} @@ -623,6 +636,11 @@ function perFileToUnified( return { path, displayPath: sanitizeFilenameForDisplay(path), + // Carry the pre-rename path so the interactive viewer can show `old → new` + // for renames (keyed/addressed by the new path). + oldDisplayPath: s.oldPath + ? sanitizeFilenameForDisplay(s.oldPath) + : undefined, added: s.added ?? 0, removed: s.isUntracked ? 0 : (s.removed ?? 0), isBinary: !!s.isBinary, diff --git a/packages/cli/src/ui/components/messages/DiffStatsDisplay.test.tsx b/packages/cli/src/ui/components/messages/DiffStatsDisplay.test.tsx index e36e6862a4a..05275094056 100644 --- a/packages/cli/src/ui/components/messages/DiffStatsDisplay.test.tsx +++ b/packages/cli/src/ui/components/messages/DiffStatsDisplay.test.tsx @@ -177,4 +177,57 @@ describe('DiffStatsDisplay', () => { expect(visible).toContain('60 files changed'); expect(visible).toMatch(/59 more/); }); + + it('renders renamed files as `old → new`', () => { + const model: DiffRenderModel = { + filesCount: 1, + linesAdded: 2, + linesRemoved: 1, + hiddenCount: 0, + rows: [ + { + filename: 'src/new-name.ts', + oldPath: 'src/old-name.ts', + added: 2, + removed: 1, + isBinary: false, + isUntracked: false, + isDeleted: false, + truncated: false, + }, + ], + }; + const visible = stripAnsi( + render().lastFrame() ?? '', + ); + const row = visible.split('\n').find((l) => l.includes('new-name.ts'))!; + expect(row).toContain('src/old-name.ts → src/new-name.ts'); + }); + + it('sanitizes hostile filenames carrying control characters', () => { + const model: DiffRenderModel = { + filesCount: 1, + linesAdded: 1, + linesRemoved: 0, + hiddenCount: 0, + rows: [ + { + // A crafted filename with a BEL — sanitizeFilenameForDisplay must + // escape it to inert text rather than injecting a control byte. + filename: 'evil\x07.txt', + added: 1, + removed: 0, + isBinary: false, + isUntracked: false, + isDeleted: false, + truncated: false, + }, + ], + }; + const visible = stripAnsi( + render().lastFrame() ?? '', + ); + expect(visible).toContain('evil\\u0007.txt'); + expect(visible).not.toContain('\x07'); + }); }); diff --git a/packages/web-shell/client/components/GitBranchIndicator.test.tsx b/packages/web-shell/client/components/GitBranchIndicator.test.tsx index 7005d43b4b6..010e4102a4c 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.test.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.test.tsx @@ -9,7 +9,7 @@ import { getTranslator, I18nProvider } from '../i18n'; import { GitBranchIndicator } from './GitBranchIndicator'; let container: HTMLDivElement; -let root: Root; +let root: Root | undefined; function render( props: { @@ -33,7 +33,13 @@ function render( } afterEach(() => { - act(() => root.unmount()); + // The localization tests below assert on getTranslator() without render(), + // so `root` may already be unmounted by a preceding test's afterEach — guard + // instead of unconditionally double-unmounting (a TypeError under React 19). + if (root) { + act(() => root.unmount()); + root = undefined; + } container.remove(); }); From 76bfedb8432b0a1868ee08ed78d54555dd1b2c31 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 06:58:50 +0800 Subject: [PATCH 14/24] =?UTF-8?q?fix(core,cli,web-shell):=20rename-aware?= =?UTF-8?q?=20single-file=20diff=20(old=E2=86=92new)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetchGitDiffHunksForFile pathspec-limited the diff to the new path, which defeats git's rename detection — a renamed file was reported as fully added (every line +) instead of its actual edit. Thread an optional pre-rename path through the single-file endpoint (core → route → SDK → dialog) and diff old→new with -M when it is present, so expanding a renamed file shows its real content change. --- .../serve/routes/workspace-git-diff.test.ts | 2 ++ .../src/serve/routes/workspace-git-diff.ts | 13 ++++++++- packages/core/src/utils/gitDiff.test.ts | 20 +++++++++++++ packages/core/src/utils/gitDiff.ts | 29 +++++++++++-------- .../sdk-typescript/src/daemon/DaemonClient.ts | 16 ++++++++-- .../components/dialogs/GitDiffDialog.test.tsx | 4 +-- .../components/dialogs/GitDiffDialog.tsx | 4 ++- 7 files changed, 69 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-git-diff.test.ts b/packages/cli/src/serve/routes/workspace-git-diff.test.ts index 573bce5ce97..fa3c23d0665 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.test.ts @@ -200,6 +200,7 @@ describe('workspace Git diff routes', () => { expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( '/work/main', 'src/a.ts', + undefined, ); }); @@ -281,6 +282,7 @@ describe('workspace Git diff routes', () => { expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( '/work/secondary', 'b.ts', + undefined, ); }); diff --git a/packages/cli/src/serve/routes/workspace-git-diff.ts b/packages/cli/src/serve/routes/workspace-git-diff.ts index a310bad0acf..fdfed82f69c 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.ts @@ -132,12 +132,23 @@ async function handleDiffFile( }); return; } + // Optional pre-rename path: when present the diff is computed old→new with + // rename detection so a renamed file shows its actual edit, not all-added. + const queryOldPath = req.query['oldPath']; + const oldPath = + typeof queryOldPath === 'string' && queryOldPath.length > 0 + ? queryOldPath + : undefined; try { // Apply the read headers before the await (as handleDiffList does) so the // no-store/nosniff headers are also present on the error response if the // fetch throws. applyReadHeaders(res); - const result = await fetchGitDiffHunksForFile(workspaceCwd, queryPath); + const result = await fetchGitDiffHunksForFile( + workspaceCwd, + queryPath, + oldPath, + ); res.status(200).json(buildFileHunks(workspaceCwd, queryPath, result)); } catch (err) { sendBridgeError(res, err, { route }); diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index f9639f56cfa..5dce8a8c6e8 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -547,6 +547,26 @@ describe('fetchGitDiffHunksForFile', () => { expect(await fetchGitDiffHunksForFile(repo, 'a.txt')).toBeNull(); }); + it('diffs a renamed file old→new when oldPath is provided', async () => { + await fs.writeFile(path.join(repo, 'old.txt'), 'one\ntwo\nthree\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + // Rename old.txt → new.txt and edit one line. + await fs.rm(path.join(repo, 'old.txt')); + await fs.writeFile(path.join(repo, 'new.txt'), 'one\nTWO\nthree\n'); + await git(repo, 'add', '-A'); + + // With the pre-rename path, rename detection yields the actual edit + // (-two/+TWO with one/three as context) instead of new.txt as fully added. + const result = await fetchGitDiffHunksForFile(repo, 'new.txt', 'old.txt'); + expect(result).not.toBeNull(); + const lines = result!.hunks.flatMap((h) => h.lines); + expect(lines).toContain('-two'); + expect(lines).toContain('+TWO'); + expect(lines).toContain(' one'); + expect(lines).not.toContain('+one'); + }); + it('synthesizes an all-added hunk for an untracked file', async () => { await fs.writeFile(path.join(repo, 'a.txt'), 'a\n'); await git(repo, 'add', '.'); diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index f422208f5dc..835ac36ddb6 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -343,6 +343,7 @@ export async function fetchGitDiffHunks( export async function fetchGitDiffHunksForFile( cwd: string, filePath: string, + oldPath?: string, ): Promise { const gitRoot = findGitRoot(cwd); if (!gitRoot) return null; @@ -350,18 +351,22 @@ export async function fetchGitDiffHunksForFile( if (relPath === null) return null; if (await isInTransientGitState(gitRoot)) return null; - const diffOut = await runGit( - [ - '--no-optional-locks', - 'diff', - '--no-ext-diff', - '--no-textconv', - 'HEAD', - '--', - relPath, - ], - gitRoot, - ); + // For a rename, include the pre-rename path with rename detection so git + // diffs old→new content instead of reporting the new path as fully added + // (a single-path pathspec defeats rename detection). + const oldRelPath = + oldPath != null ? toRepoRelativePath(gitRoot, oldPath) : null; + const diffArgs = [ + '--no-optional-locks', + 'diff', + '--no-ext-diff', + '--no-textconv', + ]; + if (oldRelPath != null) diffArgs.push('-M'); + diffArgs.push('HEAD', '--'); + if (oldRelPath != null) diffArgs.push(oldRelPath); + diffArgs.push(relPath); + const diffOut = await runGit(diffArgs, gitRoot); if (diffOut == null) return null; const truncatedPaths = new Set(); const parsed = parseGitDiff(diffOut, truncatedPaths); diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 66dfcfdc265..855f86f7c95 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -1015,9 +1015,13 @@ export class DaemonClient { async workspaceGitDiffFile( path: string, + oldPath?: string, ): Promise { + const query = + `/workspace/git/diff/file?path=${urlEncode(path)}` + + (oldPath != null ? `&oldPath=${urlEncode(oldPath)}` : ''); return await this.jsonRequest( - `/workspace/git/diff/file?path=${urlEncode(path)}`, + query, 'GET /workspace/git/diff/file', { mode: 'rest' }, ); @@ -4180,10 +4184,16 @@ export class WorkspaceDaemonClient { ); } - workspaceGitDiffFile(path: string): Promise { + workspaceGitDiffFile( + path: string, + oldPath?: string, + ): Promise { + const query = + `/git/diff/file?path=${urlEncode(path)}` + + (oldPath != null ? `&oldPath=${urlEncode(oldPath)}` : ''); return this.client.workspaceJsonRequest( this.workspaceSelector, - `/git/diff/file?path=${urlEncode(path)}`, + query, 'GET /workspaces/:workspace/git/diff/file', { mode: 'rest' }, ); diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx index c89c77b10ce..943afeda836 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -178,7 +178,7 @@ describe('GitDiffDialog', () => { }); await flush(); - expect(workspaceGitDiffFile).toHaveBeenCalledWith('src/a.ts'); + expect(workspaceGitDiffFile).toHaveBeenCalledWith('src/a.ts', undefined); // Plain-text fallback: the line bodies render without the +/- prefix // (the marker is a separate column). expect(document.body.textContent).toContain('const a = 2'); @@ -257,7 +257,7 @@ describe('GitDiffDialog', () => { }); await flush(); - expect(workspaceGitDiffFile).toHaveBeenCalledWith('src/a.ts'); + expect(workspaceGitDiffFile).toHaveBeenCalledWith('src/a.ts', undefined); expect(document.body.textContent).toContain('Failed to load this diff'); }); diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index d9bd0b6c2e9..16c195218e4 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -252,7 +252,9 @@ function DiffFileRow({ setError(false); client .workspaceByCwd(workspaceCwd) - .workspaceGitDiffFile(file.path) + // Pass the pre-rename path so a renamed file diffs old→new (rename + // detection) instead of showing the new path as fully added. + .workspaceGitDiffFile(file.path, file.oldPath) .then((result) => { setHunks(result.hunks); setTruncated(result.truncated === true); From 70dbc9a4a6ea2ca510b2359fced4f55b39a1e254 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 08:14:19 +0800 Subject: [PATCH 15/24] fix(cli): address round-10 review suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DiffDialog: split the path-width budget between old and new paths for a rename (reserving the " → " separator) so the combined width stays within maxPathChars instead of overflowing the row layout. - textUtils: extend MULTILINE_CONTROL_CHARS_REGEX with the Unicode bidi ranges (matching FILENAME_CONTROL_CHARS_REGEX) and add a test that sanitizeFilenameForDisplay strips bidi embedding/isolate controls. - workspace-git-diff route: add a test that ?oldPath= is parsed and forwarded to fetchGitDiffHunksForFile. --- .../serve/routes/workspace-git-diff.test.ts | 25 +++++++++++++++++++ packages/cli/src/ui/components/DiffDialog.tsx | 9 +++++-- packages/cli/src/ui/utils/textUtils.test.ts | 10 ++++++++ packages/cli/src/ui/utils/textUtils.ts | 6 +++-- 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-git-diff.test.ts b/packages/cli/src/serve/routes/workspace-git-diff.test.ts index fa3c23d0665..32315919cb5 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.test.ts @@ -204,6 +204,31 @@ describe('workspace Git diff routes', () => { ); }); + it('forwards the oldPath query to fetchGitDiffHunksForFile', async () => { + fetchGitDiffHunksForFileMock.mockResolvedValue({ + hunks: [], + truncated: false, + }); + const app = express(); + registerWorkspaceGitDiffRoutes(app, { + boundWorkspace: '/work/main', + sendBridgeError, + }); + + const response = await request(app).get( + '/workspace/git/diff/file?path=src/new.ts&oldPath=src/old.ts', + ); + + expect(response.status).toBe(200); + // The route must parse ?oldPath= and forward it so the core diff is + // computed old→new (rename detection) instead of new-path-as-added. + expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( + '/work/main', + 'src/new.ts', + 'src/old.ts', + ); + }); + it('surfaces the truncated flag when the diff was capped', async () => { fetchGitDiffHunksForFileMock.mockResolvedValue({ hunks: [ diff --git a/packages/cli/src/ui/components/DiffDialog.tsx b/packages/cli/src/ui/components/DiffDialog.tsx index 08baede19ac..207aba41b6e 100644 --- a/packages/cli/src/ui/components/DiffDialog.tsx +++ b/packages/cli/src/ui/components/DiffDialog.tsx @@ -474,7 +474,12 @@ function FileRow({ : ''; // Head-truncate so the basename (the part users actually read) is kept. // Use the sanitized displayPath — `file.path` may carry raw control bytes. - const path = truncatePathStart(file.displayPath, maxPathChars); + // For a rename, split the budget between old and new (reserving the " → " + // separator) so the combined width stays within maxPathChars. + const pathBudget = file.oldDisplayPath + ? Math.max(8, Math.floor((maxPathChars - 3) / 2)) + : maxPathChars; + const path = truncatePathStart(file.displayPath, pathBudget); return ( {file.oldDisplayPath ? ( - {truncatePathStart(file.oldDisplayPath, maxPathChars)} →{' '} + {truncatePathStart(file.oldDisplayPath, pathBudget)} →{' '} ) : null} { expect(sanitizeFilenameForDisplay('a\x9fb')).toBe('a\\u009fb'); }); + it('escapes Unicode bidi embedding/isolate controls', () => { + // RLO/LRE (U+202A–202E) and LRI/PDI (U+2066–2069) can visually reorder + // a filename to spoof its extension (e.g. "report‮fdp.exe" reads as + // "reportexe.pdf"). + expect(sanitizeFilenameForDisplay('a\u202eb')).toBe('a\\u202eb'); + expect(sanitizeFilenameForDisplay('a\u202ab')).toBe('a\\u202ab'); + expect(sanitizeFilenameForDisplay('a\u2066b')).toBe('a\\u2066b'); + expect(sanitizeFilenameForDisplay('a\u2069b')).toBe('a\\u2069b'); + }); + it('strips multi-byte ANSI CSI sequences', () => { // SGR color/reset and cursor movement should not survive to the // terminal — `escapeAnsiCtrlCodes` neutralizes the ESC byte, then diff --git a/packages/cli/src/ui/utils/textUtils.ts b/packages/cli/src/ui/utils/textUtils.ts index e6bf1050560..a38598c330d 100644 --- a/packages/cli/src/ui/utils/textUtils.ts +++ b/packages/cli/src/ui/utils/textUtils.ts @@ -468,8 +468,10 @@ const FILENAME_CONTROL_CHARS_REGEX = // Same as FILENAME_CONTROL_CHARS_REGEX minus `\n` (row separator) and `\t` // (benign indentation), which multi-line display treats as layout. -// eslint-disable-next-line no-control-regex -const MULTILINE_CONTROL_CHARS_REGEX = /[\x00-\x08\x0b-\x1f\x7f-\x9f]/g; +/* eslint-disable no-control-regex */ +const MULTILINE_CONTROL_CHARS_REGEX = + /[\x00-\x08\x0b-\x1f\x7f-\x9f\u202a-\u202e\u2066-\u2069]/g; +/* eslint-enable no-control-regex */ function escapeControlChar(ch: string): string { switch (ch) { From fa6c4e056ae0aa01d070505f67492b41d2ad2428 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 09:16:48 +0800 Subject: [PATCH 16/24] test(sdk),docs: cover diff client methods; align design doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - sdk: add DaemonClient unit tests for workspaceGitDiff() and workspaceGitDiffFile(path, oldPath?) — URL construction (incl. urlEncode on path/oldPath, with and without oldPath, plus the workspace-qualified route) and response deserialization, mirroring the existing workspaceGit() test. - docs: add the oldPath? param to the workspaceGitDiffFile API spec; record that the diff client methods now have unit tests (correcting the claim that workspaceGit() had none); attribute the bundle-limit bump to packages/sdk-typescript/scripts/build.js; clarify ahead/behind are relative to upstream (0, and ↑N/↓N not shown, without one). --- .../2026-07-16-webshell-git-status-diff.md | 20 +++--- .../test/unit/DaemonClient.test.ts | 69 +++++++++++++++++++ 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/docs/design/2026-07-16-webshell-git-status-diff.md b/docs/design/2026-07-16-webshell-git-status-diff.md index 6230929a352..2928c0c75cb 100644 --- a/docs/design/2026-07-16-webshell-git-status-diff.md +++ b/docs/design/2026-07-16-webshell-git-status-diff.md @@ -264,9 +264,10 @@ interface DaemonWorkspaceGitStatus { conflicted?: number; /** 是否配置了 upstream。 */ hasUpstream?: boolean; - /** 领先 upstream 的 commit 数。 */ + /** 领先 upstream 的 commit 数；仅当 `hasUpstream` 为 true 时有意义，无 + * upstream 时为 0（此时 UI 不显示 ↑N）。 */ ahead?: number; - /** 落后 upstream 的 commit 数。 */ + /** 落后 upstream 的 commit 数；同 `ahead`，仅有 upstream 时有意义。 */ behind?: number; /** stash 数量。 */ stashCount?: number; @@ -397,9 +398,10 @@ M]`）和 porcelain 行（统计 staged / unstaged / untracked）。解析逻辑 `sdk-typescript/src/index.ts` 与 `src/daemon/index.ts` 导出。 - `DaemonClient` 新增： - `workspaceGitDiff(): Promise` - - `workspaceGitDiffFile(path: string): Promise` + - `workspaceGitDiffFile(path: string, oldPath?: string): Promise` （`path` 作为 query 参数需 `urlEncode`，对齐现有 `workspaceMcpTools` 等 - 方法的写法）。 + 方法的写法；`oldPath` 可选，传入时服务端按 rename 检测计算 old→new 的 + diff，否则重命名文件会显示为整文件新增；`oldPath` 同样需 `urlEncode`）。 - 事件：可选新增 `git_status_changed`（携带 enriched status）。本期更倾向于 **不新增推送事件**，而是复用现有 `git_branch_changed` 作为“需要重新拉取 status”的信号——见“刷新策略”。是否新增 `git_status_changed` 留作实施时权衡， @@ -732,10 +734,12 @@ core fetchGitDiff(cwd) / fetchGitDiffHunksForFile(cwd, path) - [x] `DaemonClient`：`workspaceGitDiff()` / `workspaceGitDiffFile(path)` （path 作为 query，`urlEncode`，对齐 `workspaceMcpTools` 写法）； bound 与 workspace-qualified 两个 client 类各加一对方法。 -- [x] 浏览器 bundle 上限 160KB→165KB（`scripts/build.js`，含说明注释）。 - 未加 client 方法单测：SDK 现有 `workspaceGit()` 亦无对应单测，遵循既有 - 约定不补一次性测试；契约由 cli 路由单测 + typecheck + web-shell 消费侧 - 测试覆盖。 +- [x] 浏览器 bundle 上限 160KB→165KB（`packages/sdk-typescript/scripts/build.js`， + 含说明注释）。 + 已补 client 方法单测：`DaemonClient.test.ts` 覆盖 `workspaceGitDiff()` / + `workspaceGitDiffFile(path, oldPath?)` 的 URL 构造（含 path/oldPath 的 + `urlEncode`）与响应反序列化，与既有 `workspaceGit()` 单测同一模式；契约 + 另由 cli 路由单测 + typecheck + web-shell 消费侧测试覆盖。 ### Task 4 · Web Shell ✅ 已完成 diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index 7381cd7c807..51906819036 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -747,6 +747,75 @@ describe('DaemonClient', () => { expect(transportFetch).not.toHaveBeenCalled(); }); + it('reads Git diff list and per-file hunks (incl. rename oldPath) over REST', async () => { + const diffList = { + v: 1 as const, + workspaceCwd: '/work/main', + available: true, + filesCount: 1, + linesAdded: 2, + linesRemoved: 1, + files: [ + { + path: 'src/new.ts', + oldPath: 'src/old.ts', + added: 2, + removed: 1, + isBinary: false, + isUntracked: false, + isDeleted: false, + truncated: false, + }, + ], + hiddenCount: 0, + }; + const hunks = { + v: 1 as const, + workspaceCwd: '/work/main', + path: 'src/new.ts', + available: true, + hunks: [ + { + oldStart: 1, + oldLines: 1, + newStart: 1, + newLines: 1, + lines: ['-a', '+b'], + }, + ], + }; + const { fetch, calls } = recordingFetch((req) => + jsonResponse(200, req.url.includes('/diff/file') ? hunks : diffList), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + await expect(client.workspaceGitDiff()).resolves.toEqual(diffList); + // Without oldPath: no &oldPath= query segment. + await expect(client.workspaceGitDiffFile('src/new.ts')).resolves.toEqual( + hunks, + ); + // With oldPath: urlEncoded into the query for rename detection. + await expect( + client.workspaceGitDiffFile('src/new.ts', 'src/old.ts'), + ).resolves.toEqual(hunks); + // Workspace-qualified variant routes through /workspaces/:cwd/git/diff/file. + await expect( + client.workspaceByCwd('/work/secondary').workspaceGitDiffFile('a.ts'), + ).resolves.toEqual(hunks); + expect(calls.map((call) => [call.method, call.url])).toEqual([ + ['GET', 'http://daemon/workspace/git/diff'], + ['GET', 'http://daemon/workspace/git/diff/file?path=src%2Fnew.ts'], + [ + 'GET', + 'http://daemon/workspace/git/diff/file?path=src%2Fnew.ts&oldPath=src%2Fold.ts', + ], + [ + 'GET', + 'http://daemon/workspaces/%2Fwork%2Fsecondary/git/diff/file?path=a.ts', + ], + ]); + }); + it('lets ACP preheat wait longer than the client default timeout', async () => { let resolveResponse: ((value: Response) => void) | undefined; const slowFetch = vi.fn( From b122c9f708c6d02c24d5d8001a592ee61c647420 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 10:50:35 +0800 Subject: [PATCH 17/24] fix(web-shell,core): address round-11 review suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GitBranchIndicator: count conflicted entries as dirty — a merge where every changed file is conflicted (staged=unstaged=untracked=0) is still uncommitted, so the expanded chip's dirty dot / data-dirty now reflect it. - core: split the status branch line at the last "..." (the branch/upstream separator) so a dotted branch name isn't truncated at the first "...". - GitDiffDialog: guard DiffFileRow's in-flight fetch against unmount via a cancelled ref, matching DiffHunks / GitDiffDialog. - tests: forward oldPath when expanding a renamed file in the web-shell dialog; bidi-strip coverage for the web-shell sanitizeControlChars; untrusted-guard coverage on the single-file diff route; conflicted-only dirty; branch-line "..." split. --- .../serve/routes/workspace-git-diff.test.ts | 18 +++++++ packages/core/src/utils/gitDiff.test.ts | 13 +++++ packages/core/src/utils/gitDiff.ts | 6 ++- .../components/GitBranchIndicator.test.tsx | 16 ++++++ .../client/components/GitBranchIndicator.tsx | 7 ++- .../components/dialogs/GitDiffDialog.test.tsx | 52 +++++++++++++++++++ .../components/dialogs/GitDiffDialog.tsx | 16 ++++-- .../messages/toolFormatting.test.ts | 9 ++++ 8 files changed, 131 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-git-diff.test.ts b/packages/cli/src/serve/routes/workspace-git-diff.test.ts index 32315919cb5..e6784ca464a 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.test.ts @@ -327,6 +327,24 @@ describe('workspace Git diff routes', () => { expect(fetchGitDiffMock).not.toHaveBeenCalled(); }); + it('rejects an untrusted workspace on the single-file endpoint too', async () => { + const app = express(); + const primary = runtime('primary', '/work/main', true); + const untrusted = runtime('untrusted', '/work/untrusted', false); + registerWorkspaceQualifiedGitDiffRoutes(app, { + workspaceRegistry: registry([primary, untrusted]), + sendBridgeError, + }); + + const response = await request(app).get( + '/workspaces/untrusted/git/diff/file?path=a.ts', + ); + + expect(response.status).toBe(403); + expect(response.body.code).toBe('untrusted_workspace'); + expect(fetchGitDiffHunksForFileMock).not.toHaveBeenCalled(); + }); + it('rejects an unknown workspace', async () => { const app = express(); const primary = runtime('primary', '/work/main', true); diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index 5dce8a8c6e8..c9006b1fa93 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -1541,6 +1541,19 @@ describe('parseStatusBranchLine', () => { }); }); + it('splits the branch from upstream at the last "..."', () => { + // Git forbids `..` in ref names so a real branch can't contain `...`, but + // the split must use the last `...` (the branch/upstream separator) so a + // dotted branch name isn't truncated at the first `...`. + expect(parseStatusBranchLine('## fix...feature...origin/main')).toEqual({ + branch: 'fix...feature', + detached: false, + hasUpstream: true, + ahead: 0, + behind: 0, + }); + }); + it('parses ahead-only and behind-only brackets', () => { expect( parseStatusBranchLine('## main...origin/main [ahead 3]'), diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index 835ac36ddb6..6c5f88aca4a 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -1259,7 +1259,11 @@ export function parseStatusBranchLine(line: string): StatusBranchLine { } const hasUpstream = desc.includes('...'); - const branch = desc.split('...')[0]; + // Split at the last `...` (the branch/upstream separator). Git forbids `..` + // in ref names so a branch can't itself contain `...`, but lastIndexOf is the + // robust split point regardless. + const sepIndex = desc.lastIndexOf('...'); + const branch = sepIndex >= 0 ? desc.slice(0, sepIndex) : desc; return { branch: branch || null, detached: false, diff --git a/packages/web-shell/client/components/GitBranchIndicator.test.tsx b/packages/web-shell/client/components/GitBranchIndicator.test.tsx index 010e4102a4c..6a8a2a3c735 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.test.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.test.tsx @@ -126,6 +126,22 @@ describe('GitBranchIndicator', () => { expect(el.getAttribute('aria-label')).toContain('7 conflicted'); }); + it('treats a conflicted-only working tree as dirty', () => { + // A merge where every changed file is conflicted has staged=unstaged= + // untracked=0 but conflicted>0 — still uncommitted changes, so still dirty. + render({ + branch: 'main', + status: { + v: 2, + workspaceCwd: '/repo', + branch: 'main', + conflicted: 3, + }, + }); + + expect(chip().getAttribute('data-dirty')).toBe('true'); + }); + it('flags a detached HEAD', () => { render({ branch: 'a1b2c3d', diff --git a/packages/web-shell/client/components/GitBranchIndicator.tsx b/packages/web-shell/client/components/GitBranchIndicator.tsx index 262e85f66de..57d02dfd42a 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.tsx @@ -51,17 +51,20 @@ function deriveStatus(status?: DaemonWorkspaceGitStatus): DerivedStatus { const staged = status?.staged ?? 0; const unstaged = status?.unstaged ?? 0; const untracked = status?.untracked ?? 0; + const conflicted = status?.conflicted ?? 0; return { detached: status?.detached ?? false, staged, unstaged, untracked, - conflicted: status?.conflicted ?? 0, + conflicted, ahead: status?.ahead ?? 0, behind: status?.behind ?? 0, stashCount: status?.stashCount ?? 0, operation: status?.operation, - dirty: staged + unstaged + untracked > 0, + // Conflicted entries are uncommitted changes too — a merge where every + // changed file is conflicted (staged=unstaged=untracked=0) is still dirty. + dirty: staged + unstaged + untracked + conflicted > 0, }; } diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx index 943afeda836..83e17dcaeab 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -186,6 +186,58 @@ describe('GitDiffDialog', () => { expect(document.body.textContent).toContain('const a = 1'); }); + it('forwards the pre-rename oldPath when expanding a renamed file', async () => { + workspaceGitDiff.mockResolvedValue( + diffPayload({ + files: [ + { + path: 'src/new.ts', + oldPath: 'src/old.ts', + added: 1, + removed: 1, + isBinary: false, + isUntracked: false, + isDeleted: false, + truncated: false, + }, + ], + }), + ); + workspaceGitDiffFile.mockResolvedValue({ + v: 1, + workspaceCwd: '/repo', + path: 'src/new.ts', + available: true, + hunks: [ + { + oldStart: 1, + oldLines: 1, + newStart: 1, + newLines: 1, + lines: ['-const a = 1', '+const a = 2'], + }, + ], + }); + mount(); + await flush(); + + const header = document.body.querySelector( + 'button[aria-expanded="false"]', + ) as HTMLButtonElement; + expect(header).not.toBeNull(); + await act(async () => { + header.click(); + }); + await flush(); + + // The pre-rename path is forwarded so the daemon diffs old→new (rename + // detection) instead of showing the new path as fully added. + expect(workspaceGitDiffFile).toHaveBeenCalledWith( + 'src/new.ts', + 'src/old.ts', + ); + }); + it('shows a placeholder when git is unavailable', async () => { workspaceGitDiff.mockResolvedValue( diffPayload({ available: false, files: [] }), diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index 16c195218e4..cfc361101e8 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useEffect, useState, type ReactNode } from 'react'; +import { useEffect, useRef, useState, type ReactNode } from 'react'; import { useWorkspace } from '@qwen-code/webui/daemon-react-sdk'; import type { DaemonDiffHunk, @@ -243,6 +243,15 @@ function DiffFileRow({ const [truncated, setTruncated] = useState(false); const [loading, setLoading] = useState(false); const [error, setError] = useState(false); + // Guard the in-flight fetch so closing the dialog before it resolves doesn't + // settle state on an unmounted row (matching DiffHunks / GitDiffDialog). + const cancelledRef = useRef(false); + useEffect( + () => () => { + cancelledRef.current = true; + }, + [], + ); const toggle = () => { const next = !open; @@ -256,14 +265,15 @@ function DiffFileRow({ // detection) instead of showing the new path as fully added. .workspaceGitDiffFile(file.path, file.oldPath) .then((result) => { + if (cancelledRef.current) return; setHunks(result.hunks); setTruncated(result.truncated === true); }) .catch(() => { - setError(true); + if (!cancelledRef.current) setError(true); }) .finally(() => { - setLoading(false); + if (!cancelledRef.current) setLoading(false); }); } }; diff --git a/packages/web-shell/client/components/messages/toolFormatting.test.ts b/packages/web-shell/client/components/messages/toolFormatting.test.ts index c812899a650..eb63714a1b2 100644 --- a/packages/web-shell/client/components/messages/toolFormatting.test.ts +++ b/packages/web-shell/client/components/messages/toolFormatting.test.ts @@ -48,6 +48,15 @@ describe('toolFormatting', () => { ); expect(sanitizeControlChars('a\tb\nc')).toBe('a\tb\nc'); }); + + it('escapes Unicode bidi embedding/isolate controls', () => { + // RLO/LRE (U+202A–202E) and LRI/PDI (U+2066–2069) can visually reorder + // a filename to spoof its extension (mirrors the CLI-side coverage). + expect(sanitizeControlChars('a\u202eb')).toBe('a\\u202eb'); + expect(sanitizeControlChars('a\u202ab')).toBe('a\\u202ab'); + expect(sanitizeControlChars('a\u2066b')).toBe('a\\u2066b'); + expect(sanitizeControlChars('a\u2069b')).toBe('a\\u2069b'); + }); }); it('normalizes web fetch display names', () => { From 0f138bdbd56ff31d24e586a966d6caf5c64c2269 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 12:57:32 +0800 Subject: [PATCH 18/24] fix(web-shell,cli): address round-12 review suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DiffDialog: only render the rename "old → new" when there's room for both sides (≥19 cols, so each gets ≥8); otherwise fall back to the new path alone, so a narrow terminal no longer overflows the row (the Math.max(8,…) floor could exceed maxPathChars). - GitBranchIndicator test: guard afterEach container.remove() for non-render tests run in isolation, and make the compact-mode ↑-suppression assertion non-vacuous by giving the fixture an ahead count. - App: compute the active workspace once (useMemo) and share it between the git-status effect and the Changes-dialog entry point, so the chip and the dialog can't drift onto different repos. --- packages/cli/src/ui/components/DiffDialog.tsx | 14 +++--- packages/web-shell/client/App.tsx | 48 +++++++++---------- .../components/GitBranchIndicator.test.tsx | 16 +++++-- 3 files changed, 43 insertions(+), 35 deletions(-) diff --git a/packages/cli/src/ui/components/DiffDialog.tsx b/packages/cli/src/ui/components/DiffDialog.tsx index 207aba41b6e..a2b6960ea19 100644 --- a/packages/cli/src/ui/components/DiffDialog.tsx +++ b/packages/cli/src/ui/components/DiffDialog.tsx @@ -474,10 +474,12 @@ function FileRow({ : ''; // Head-truncate so the basename (the part users actually read) is kept. // Use the sanitized displayPath — `file.path` may carry raw control bytes. - // For a rename, split the budget between old and new (reserving the " → " - // separator) so the combined width stays within maxPathChars. - const pathBudget = file.oldDisplayPath - ? Math.max(8, Math.floor((maxPathChars - 3) / 2)) + // For a rename, show "old → new" only when there's room for both sides + // (≥19 cols, so each side gets ≥8 without overflowing); otherwise fall back + // to the new path alone so a narrow terminal doesn't break the row layout. + const renameFits = !!file.oldDisplayPath && maxPathChars >= 19; + const pathBudget = renameFits + ? Math.floor((maxPathChars - 3) / 2) : maxPathChars; const path = truncatePathStart(file.displayPath, pathBudget); return ( @@ -488,9 +490,9 @@ function FileRow({ > {pointer} - {file.oldDisplayPath ? ( + {renameFits ? ( - {truncatePathStart(file.oldDisplayPath, pathBudget)} →{' '} + {truncatePathStart(file.oldDisplayPath!, pathBudget)} →{' '} ) : null} (undefined); + // Active workspace: the connected session's workspace, else the workspace + // picked for the next session (locked / selected / primary). Computed once + // and shared by the git-status effect and the Changes-dialog entry point so + // the chip and the dialog always target the same repo. + const activeWorkspaceCwd = useMemo( + () => + connection.sessionId + ? connection.workspaceCwd + : (lockedWorkspaceCwd ?? + selectedWorkspaceCwd ?? + workspaces.find((entry) => entry.primary)?.cwd), + [ + connection.sessionId, + connection.workspaceCwd, + lockedWorkspaceCwd, + selectedWorkspaceCwd, + workspaces, + ], + ); useEffect(() => { - // Active workspace: the connected session's workspace, else the workspace - // picked for the next session (locked / selected / primary). - const activeWorkspaceCwd = connection.sessionId - ? connection.workspaceCwd - : (lockedWorkspaceCwd ?? - selectedWorkspaceCwd ?? - workspaces.find((entry) => entry.primary)?.cwd); if (!activeWorkspaceCwd) { gitStatusWorkspaceCwdRef.current = undefined; setSelectedWorkspaceGitStatus(undefined); @@ -1277,15 +1289,7 @@ export function App({ window.removeEventListener('focus', onFocus); window.clearInterval(poll); }; - }, [ - connection.sessionId, - connection.workspaceCwd, - connection.gitBranch, - lockedWorkspaceCwd, - selectedWorkspaceCwd, - workspaces, - workspace.client, - ]); + }, [activeWorkspaceCwd, connection.gitBranch, workspace.client]); const onToastRef = useRef(onToast); onToastRef.current = onToast; const toastIdRef = useRef(0); @@ -2807,14 +2811,10 @@ export function App({ })), [connection.models], ); - // The workspace the Changes dialog reads: the connected session's workspace, - // else the workspace picked for the next session (mirrors the git-status - // effect above so the chip and the dialog always target the same repo). - const gitDiffWorkspaceCwd = connection.sessionId - ? connection.workspaceCwd - : (lockedWorkspaceCwd ?? - selectedWorkspaceCwd ?? - workspaces.find((entry) => entry.primary)?.cwd); + // The workspace the Changes dialog reads — the same active workspace the + // git-status effect targets (computed once above), so the chip and the + // dialog always target the same repo. + const gitDiffWorkspaceCwd = activeWorkspaceCwd; const dialogOpen = showResumeDialog || showDeleteDialog || diff --git a/packages/web-shell/client/components/GitBranchIndicator.test.tsx b/packages/web-shell/client/components/GitBranchIndicator.test.tsx index 6a8a2a3c735..8a45cc61217 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.test.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.test.tsx @@ -8,7 +8,7 @@ import type { DaemonWorkspaceGitStatus } from '@qwen-code/sdk/daemon'; import { getTranslator, I18nProvider } from '../i18n'; import { GitBranchIndicator } from './GitBranchIndicator'; -let container: HTMLDivElement; +let container: HTMLDivElement | undefined; let root: Root | undefined; function render( @@ -40,11 +40,13 @@ afterEach(() => { act(() => root.unmount()); root = undefined; } - container.remove(); + // container is undefined when a non-render test (e.g. a localization test) + // runs in isolation; guard so remove() doesn't throw a TypeError. + container?.remove(); }); function chip(): HTMLElement { - const el = container.querySelector('[data-web-shell-git-branch]'); + const el = container?.querySelector('[data-web-shell-git-branch]'); if (!el) throw new Error('branch indicator was not rendered'); return el as HTMLElement; } @@ -58,7 +60,7 @@ describe('GitBranchIndicator', () => { expect(el.tagName).toBe('OUTPUT'); expect(el.textContent).toContain(branch); // No interactive control — it is a status chip, not a button. - expect(container.querySelector('button')).toBeNull(); + expect(container?.querySelector('button')).toBeNull(); // A clean repo carries no dirty / operation markers. expect(el.getAttribute('data-dirty')).toBeNull(); expect(el.getAttribute('data-operation')).toBeNull(); @@ -68,7 +70,7 @@ describe('GitBranchIndicator', () => { let opened = 0; render({ branch: 'main', onOpenDiff: () => (opened += 1) }); - const button = container.querySelector('button'); + const button = container?.querySelector('button'); expect(button).not.toBeNull(); expect(button?.getAttribute('data-clickable')).toBe('true'); expect(button?.tagName).toBe('BUTTON'); @@ -163,6 +165,10 @@ describe('GitBranchIndicator', () => { branch: 'main', conflicted: 2, staged: 1, + // ahead would render ↑3 in the expanded chip; compact must suppress it + // (without ahead this assertion would be vacuously true). + hasUpstream: true, + ahead: 3, }, }); From 7fc2656e891793f31bdf99a4f09f9b1a3f5497fd Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 13:02:20 +0800 Subject: [PATCH 19/24] fix(core,docs): address round-13 review suggestions - core: add a rebase-apply detection test (git am / an interrupted `rebase --apply` creates rebase-apply, which detectGitOperation also maps to 'rebase'); previously only rebase-merge was exercised. - docs: correct section 5 to describe the actual diff-dialog mechanism (diffWorkspaceCwd state, not the stale activePanel design). --- docs/design/2026-07-16-webshell-git-status-diff.md | 8 ++++---- packages/core/src/utils/gitDiff.test.ts | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/design/2026-07-16-webshell-git-status-diff.md b/docs/design/2026-07-16-webshell-git-status-diff.md index 2928c0c75cb..e14515272a1 100644 --- a/docs/design/2026-07-16-webshell-git-status-diff.md +++ b/docs/design/2026-07-16-webshell-git-status-diff.md @@ -438,12 +438,12 @@ M]`）和 porcelain 行（统计 staged / unstaged / untracked）。解析逻辑 防止 git 允许的原始控制字节 / 转义注入。 - `available === false` 时显示占位文案（非仓库 / HEAD 缺失 / transient state），对齐 `diffCommand.ts` 的提示语义。 - - 通过 `activePanel` 机制注册（新增一个 panel 值，如 `'diff'`），复用现有 - 打开 / 关闭 / 焦点管理逻辑。 + - 通过 `diffWorkspaceCwd` 状态打开：设为目标 workspace 的 cwd 即打开弹窗， + 设回 `undefined` 关闭（不复用 `activePanel`，见 Phase 2“调研修正”）。 - `/diff` 命令本地化：在 Web Shell 中把 `/diff` 从 ACP 透传改为本地实现—— 打开 `GitDiffDialog`（对齐 CLI 交互模式打开 `DiffDialog` 的行为）。在 - `App.tsx` 的命令分发处识别 `/diff` 并 `setActivePanel('diff')`，不再发给 - daemon。`getLocalCommands` 中补 `diff` 的补全项与 `local.diff` 文案。 + `App.tsx` 的命令分发处识别 `/diff` 并 `setDiffWorkspaceCwd()`， + 不再发给 daemon。`getLocalCommands` 中补 `diff` 的补全项与 `local.diff` 文案。 ### 6. 刷新策略（第一层的新鲜度） diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index c9006b1fa93..3bbedc77639 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -1823,6 +1823,18 @@ describe('getGitWorkingTreeStatus', () => { expect(status).toMatchObject({ operation: 'rebase' }); }); + it('detects an in-progress rebase (rebase-apply dir, e.g. git am)', async () => { + await fs.writeFile(path.join(repo, 'a.txt'), 'one\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + // `git am` (and an interrupted `git rebase --apply`) creates rebase-apply; + // detectGitOperation maps it to 'rebase' too. + await fs.mkdir(path.join(repo, '.git', 'rebase-apply')); + + const status = await getGitWorkingTreeStatus(repo); + expect(status).toMatchObject({ operation: 'rebase' }); + }); + it('detects an in-progress cherry-pick', async () => { await fs.writeFile(path.join(repo, 'a.txt'), 'one\n'); await git(repo, 'add', '.'); From bf2509710cd5f5d9ff2b29f62466ec475b779b41 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 13:45:37 +0800 Subject: [PATCH 20/24] test(core): cover stray no-newline marker before any hunk header parseGitDiff's pre-hunk guard already skips a "\ No newline at end of file" marker that appears before any @@ header, so a malformed/truncated diff can't throw on a null currentHunk and lose subsequent files' hunks; add a regression test pinning that behavior. --- packages/core/src/utils/gitDiff.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index 3bbedc77639..247b9f65481 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -234,6 +234,22 @@ index 0000000..3333333 ]); }); + it('skips a stray no-newline marker before any hunk header without throwing', () => { + // A malformed/truncated diff could carry a `\` line before any `@@` + // header; the pre-hunk guard skips it rather than throwing on a null + // currentHunk (which would lose every subsequent file's hunks). + const diff = `diff --git a/f.txt b/f.txt +--- a/f.txt ++++ b/f.txt +\\ No newline at end of file +@@ -1 +1 @@ +-line ++line +`; + const result = parseGitDiff(diff); + expect(result.get('f.txt')![0].lines).toEqual(['-line', '+line']); + }); + it('returns empty map on empty input', () => { expect(parseGitDiff('').size).toBe(0); expect(parseGitDiff(' \n').size).toBe(0); From d3b4f12ba03781ea7391c10e3fba778a7c582915 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 14:42:44 +0800 Subject: [PATCH 21/24] fix(web-shell): unstick per-file diff loading and skip non-path git poll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DiffFileRow: reset the cancelled-fetch flag on mount so StrictMode's mount/unmount/mount replay no longer leaves it latched at true, which dropped the fetched hunks and froze the row on "Loading changes…" despite a 200 response. - WorkspaceSection: skip the git status poll when the workspace cwd is not an absolute path. A synthetic fallback workspace carries a display name there, which the cwd-qualified route rejects with a 400. --- .../components/dialogs/GitDiffDialog.test.tsx | 51 ++++++++++++++++--- .../components/dialogs/GitDiffDialog.tsx | 13 +++-- .../sidebar/WorkspaceSection.test.tsx | 20 ++++++++ .../components/sidebar/WorkspaceSection.tsx | 30 +++++++++-- 4 files changed, 97 insertions(+), 17 deletions(-) diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx index 83e17dcaeab..926f8fcbcd8 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom import { afterEach, describe, expect, it, vi } from 'vitest'; -import { act } from 'react'; +import { act, StrictMode } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { I18nProvider } from '../../i18n'; @@ -72,16 +72,17 @@ const { GitDiffDialog } = await import('./GitDiffDialog'); let container: HTMLDivElement; let root: Root; -function mount(workspaceCwd = '/repo') { +function mount(workspaceCwd = '/repo', strict = false) { container = document.createElement('div'); document.body.appendChild(container); root = createRoot(container); + const dialog = ( + + + + ); act(() => { - root.render( - - - , - ); + root.render(strict ? {dialog} : dialog); }); } @@ -186,6 +187,42 @@ describe('GitDiffDialog', () => { expect(document.body.textContent).toContain('const a = 1'); }); + it('still loads a file diff under StrictMode (cancelled flag resets on remount)', async () => { + // StrictMode replays mount→unmount→mount and a ref persists across the + // replay, so the row's cancelled flag must reset on mount — otherwise the + // fetched hunks are dropped and the row sticks on "Loading changes…". + workspaceGitDiff.mockResolvedValue(diffPayload()); + workspaceGitDiffFile.mockResolvedValue({ + v: 1, + workspaceCwd: '/repo', + path: 'src/a.ts', + available: true, + hunks: [ + { + oldStart: 1, + oldLines: 1, + newStart: 1, + newLines: 1, + lines: ['-const a = 1', '+const a = 2'], + }, + ], + }); + mount('/repo', true); + await flush(); + + const header = document.body.querySelector( + 'button[aria-expanded="false"]', + ) as HTMLButtonElement; + expect(header).not.toBeNull(); + await act(async () => { + header.click(); + }); + await flush(); + + expect(document.body.textContent).toContain('const a = 2'); + expect(document.body.textContent).not.toContain('Loading changes…'); + }); + it('forwards the pre-rename oldPath when expanding a renamed file', async () => { workspaceGitDiff.mockResolvedValue( diffPayload({ diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index cfc361101e8..65e26d26d51 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -246,12 +246,15 @@ function DiffFileRow({ // Guard the in-flight fetch so closing the dialog before it resolves doesn't // settle state on an unmounted row (matching DiffHunks / GitDiffDialog). const cancelledRef = useRef(false); - useEffect( - () => () => { + useEffect(() => { + // Reset on mount: StrictMode replays mount→unmount→mount and the ref + // persists across the replay, so without this reset the flag would stick at + // true and suppress every post-fetch state update (row stuck on "Loading"). + cancelledRef.current = false; + return () => { cancelledRef.current = true; - }, - [], - ); + }; + }, []); const toggle = () => { const next = !open; diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx index f3c9f670b61..daae67b6728 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx @@ -164,6 +164,26 @@ describe('WorkspaceSection git chip', () => { expect(workspaceGit).not.toHaveBeenCalled(); }); + it('skips the git poll when the workspace cwd is not a real path', async () => { + // A synthetic fallback workspace carries a display name in `cwd`; polling + // would qualify the route with it and 400, so no request fires and the chip + // stays hidden. + workspaceGit.mockResolvedValue({ + v: 2, + workspaceCwd: 'Project', + branch: 'main', + }); + + renderSection({ + workspace: { ...trustedWorkspace, cwd: 'Project' }, + onOpenGitDiff: vi.fn(), + }); + await flush(); + + expect(workspaceGit).not.toHaveBeenCalled(); + expect(gitChip()).toBeNull(); + }); + it('hides the chip when the workspace is not a git repo (null branch)', async () => { workspaceGit.mockResolvedValue({ v: 2, diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx index 4305367aa89..851382fd578 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.tsx @@ -32,6 +32,16 @@ function getWorkspaceName(cwd: string): string { return parts.at(-1) ?? cwd; } +// The cwd-qualified daemon route only accepts a workspace id or absolute path. +// A synthetic fallback workspace (daemon reports no workspaces and the +// connection has no cwd) carries a display name in `cwd`, which is neither, so +// qualifying a request with it would only ever 400. +function isAbsolutePath(cwd: string): boolean { + return ( + cwd.startsWith('/') || cwd.startsWith('\\') || /^[a-zA-Z]:[\\/]/.test(cwd) + ); +} + function getSessionLabel(session: DaemonSessionSummary): string { const displayName = session.displayName?.trim(); return displayName || session.sessionId.slice(0, 8); @@ -212,13 +222,17 @@ export function WorkspaceSection({ searchQuery, ]); + // Undefined when `cwd` is not a real path (synthetic fallback workspace), so + // the poll — which qualifies the route with the cwd — is skipped entirely. + const gitPollCwd = isAbsolutePath(workspace.cwd) ? workspace.cwd : undefined; + // Log a poll failure only on the success→failure transition, not on every // 60s/focus tick, so an unreachable workspace doesn't spam a long-lived tab. const gitPollFailed = useRef(false); const loadGitStatus = useCallback(async () => { - if (!onOpenGitDiff || !workspace.trusted) return; + if (!onOpenGitDiff || !workspace.trusted || !gitPollCwd) return; try { - const status = await client.workspaceByCwd(workspace.cwd).workspaceGit(); + const status = await client.workspaceByCwd(gitPollCwd).workspaceGit(); gitPollFailed.current = false; setGitStatus(status); } catch (err) { @@ -230,7 +244,7 @@ export function WorkspaceSection({ gitPollFailed.current = true; } } - }, [client, onOpenGitDiff, workspace.cwd, workspace.trusted]); + }, [client, gitPollCwd, onOpenGitDiff, workspace.trusted]); // The git chip lives in the always-visible folder header, so it polls // independently of session expansion: on mount/trust, on window focus, and on @@ -238,7 +252,7 @@ export function WorkspaceSection({ // per call, so the cadence stays gentle). Skipped entirely when no diff // handler is wired, since the chip — its only consumer — would not render. useEffect(() => { - if (!onOpenGitDiff || !workspace.trusted) { + if (!onOpenGitDiff || !workspace.trusted || !gitPollCwd) { setGitStatus(undefined); return; } @@ -252,7 +266,13 @@ export function WorkspaceSection({ window.removeEventListener('focus', onFocus); window.clearInterval(timer); }; - }, [loadGitStatus, onOpenGitDiff, reloadToken, workspace.trusted]); + }, [ + gitPollCwd, + loadGitStatus, + onOpenGitDiff, + reloadToken, + workspace.trusted, + ]); const visibleSessions = useMemo(() => { const query = searchQuery.trim().toLowerCase(); From 114dd33cde761d14818a9f75b2164fa2d1c00e1b Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 15:11:25 +0800 Subject: [PATCH 22/24] fix(web-shell,cli): address review suggestions on the git diff surface - GitDiffDialog: highlight each diff side independently so a small side keeps syntax highlighting even when the other side exceeds the size cap (the old guard dropped both as soon as either was too large). - ChatEditor: complete the .gitBranchChipButton reset (font/color/padding/ margin) so the clickable dirty-tree chip matches the read-only output chip instead of picking up UA button styling. - DiffDialog: cover the interactive rename display (old to new on a wide terminal), mirroring the rename tests DiffStatsDisplay and GitDiffDialog already have. --- .../cli/src/ui/components/DiffDialog.test.tsx | 113 +++++++++++++----- .../client/components/ChatEditor.module.css | 4 + .../components/dialogs/GitDiffDialog.tsx | 27 +++-- 3 files changed, 105 insertions(+), 39 deletions(-) diff --git a/packages/cli/src/ui/components/DiffDialog.test.tsx b/packages/cli/src/ui/components/DiffDialog.test.tsx index f96bf45f9fc..a08a317655d 100644 --- a/packages/cli/src/ui/components/DiffDialog.test.tsx +++ b/packages/cli/src/ui/components/DiffDialog.test.tsx @@ -6,16 +6,30 @@ import { render as inkRender } from 'ink'; import { EventEmitter } from 'node:events'; -import { describe, it, expect, vi } from 'vitest'; +import { afterEach, describe, it, expect, vi } from 'vitest'; import { waitFor } from '@testing-library/react'; +import type { Hunk } from 'diff'; +import type { GitDiffResult } from '@qwen-code/qwen-code-core'; import { DiffDialog } from './DiffDialog.js'; import { KeypressProvider } from '../contexts/KeypressContext.js'; import { ShellFocusContext } from '../contexts/ShellFocusContext.js'; -// Keep the dialog hermetic: a clean working tree and no turn diffs, matching -// the "Working tree is clean." state, so no git/filesystem access is needed. +// Tests that need a populated file list set `diffState.result` (reset in +// afterEach); the default null result keeps the dialog hermetic — a clean +// working tree matching the "Working tree is clean." state, so no +// git/filesystem access is needed. +const { diffState } = vi.hoisted(() => ({ + diffState: { + result: null as GitDiffResult | null, + hunks: new Map(), + }, +})); vi.mock('../hooks/useDiffData.js', () => ({ - useDiffData: () => ({ result: null, hunks: new Map(), loading: false }), + useDiffData: () => ({ + result: diffState.result, + hunks: diffState.hunks, + loading: false, + }), })); vi.mock('../hooks/useTurnDiffs.js', () => ({ useTurnDiffs: () => ({ turns: [], loading: false }), @@ -78,39 +92,82 @@ const renderWide = (columns: number) => { return { lastFrame: () => lastFrame, unmount: instance.unmount }; }; +afterEach(() => { + diffState.result = null; + diffState.hunks = new Map(); +}); + +// Render the dialog at a fixed terminal width, restoring the original +// `process.stdout.columns` descriptor afterward (the dialog reads it via +// useTerminalSize, and a leaked override would affect later test files). +async function withDialogAtWidth( + columns: number, + fn: (lastFrame: () => string) => Promise, +): Promise { + const original = Object.getOwnPropertyDescriptor(process.stdout, 'columns'); + Object.defineProperty(process.stdout, 'columns', { + value: columns, + configurable: true, + }); + let unmount: (() => void) | undefined; + try { + const r = renderWide(columns); + unmount = r.unmount; + await fn(r.lastFrame); + } finally { + unmount?.(); + if (original) { + Object.defineProperty(process.stdout, 'columns', original); + } else { + // Non-TTY (CI/piped stdout): `columns` is inherited from the prototype, + // so there was no own-property to restore. Delete the override we added + // so it doesn't leak into later test files via useTerminalSize. + delete (process.stdout as unknown as Record)['columns']; + } + } +} + describe('DiffDialog', () => { it('caps its width on a wide terminal so the right border is not clipped', async () => { // Regression: dialogWidth was Math.min(columns - 4, 110), but the app's // main content area is capped at 100 cols (AppContainer). On a wide // terminal the dialog overflowed its container and its right border was // clipped off-screen. - const original = Object.getOwnPropertyDescriptor(process.stdout, 'columns'); - Object.defineProperty(process.stdout, 'columns', { - value: 200, - configurable: true, - }); - let unmount: (() => void) | undefined; - try { - const r = renderWide(200); - unmount = r.unmount; + await withDialogAtWidth(200, async (lastFrame) => { await waitFor(() => { - expect(stripAnsi(r.lastFrame())).toContain('Working tree vs HEAD'); + expect(stripAnsi(lastFrame())).toContain('Working tree vs HEAD'); }); - const frame = stripAnsi(r.lastFrame()); + const frame = stripAnsi(lastFrame()); const widest = Math.max(...frame.split('\n').map((line) => line.length)); expect(widest).toBeLessThanOrEqual(102); - } finally { - unmount?.(); - if (original) { - Object.defineProperty(process.stdout, 'columns', original); - } else { - // Non-TTY (CI/piped stdout): `columns` is inherited from the prototype, - // so there was no own-property to restore. Delete the override we added - // so it doesn't leak into later test files via useTerminalSize. - delete (process.stdout as unknown as Record)[ - 'columns' - ]; - } - } + }); + }); + + it('shows old → new for a renamed file on a wide terminal', async () => { + // The interactive viewer renders `old → new` for a rename only when there's + // room for both sides (maxPathChars ≥ 19); a wide terminal qualifies, so + // both sanitized paths and the arrow appear in the file row. + diffState.result = { + stats: { filesCount: 1, linesAdded: 1, linesRemoved: 1 }, + perFileStats: new Map([ + [ + 'src/new-name.ts', + { + added: 1, + removed: 1, + isBinary: false, + oldPath: 'src/old-name.ts', + }, + ], + ]), + }; + await withDialogAtWidth(200, async (lastFrame) => { + await waitFor(() => { + expect(stripAnsi(lastFrame())).toContain('→'); + }); + const frame = stripAnsi(lastFrame()); + expect(frame).toContain('src/old-name.ts'); + expect(frame).toContain('src/new-name.ts'); + }); }); }); diff --git a/packages/web-shell/client/components/ChatEditor.module.css b/packages/web-shell/client/components/ChatEditor.module.css index badb3895ba8..fa1def6ce8e 100644 --- a/packages/web-shell/client/components/ChatEditor.module.css +++ b/packages/web-shell/client/components/ChatEditor.module.css @@ -958,6 +958,10 @@ border: 0; background: transparent; cursor: pointer; + font: inherit; + color: inherit; + padding: 0; + margin: 0; } .gitBranchChipButton:hover { diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx index 65e26d26d51..91dbbd57803 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.tsx @@ -85,20 +85,25 @@ async function buildRows( const oldCode = oldSide.join('\n'); let newTokens: ThemedToken[][] | null = null; let oldTokens: ThemedToken[][] | null = null; - if ( - highlighter && - !isTooLargeToHighlight(newCode) && - !isTooLargeToHighlight(oldCode) - ) { + if (highlighter) { // resolvedLang is a real Shiki language id here ('text' was filtered out // before the highlighter was loaded). const lang = resolvedLang as BundledLanguage; - try { - newTokens = highlighter.codeToTokens(newCode, { lang, theme }).tokens; - oldTokens = highlighter.codeToTokens(oldCode, { lang, theme }).tokens; - } catch { - newTokens = null; - oldTokens = null; + // Highlight each side independently so a small side keeps its tokens even + // when the other side exceeds the size cap. + if (!isTooLargeToHighlight(newCode)) { + try { + newTokens = highlighter.codeToTokens(newCode, { lang, theme }).tokens; + } catch { + newTokens = null; + } + } + if (!isTooLargeToHighlight(oldCode)) { + try { + oldTokens = highlighter.codeToTokens(oldCode, { lang, theme }).tokens; + } catch { + oldTokens = null; + } } } From 09e356234eb39eb262e47a33221db80157b7ab9a Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 16:05:08 +0800 Subject: [PATCH 23/24] test(web-shell,cli): cover git chip clean/reload/traversal paths, fix doc - GitDiffDialog: add the missing expect(header).not.toBeNull() guard to the three expand-file tests that lacked it, matching the others in the block. - GitBranchIndicator: cover the known-clean aria-label branch (computedAt set and every change counter zero). - WorkspaceSection: verify a reloadToken change re-fetches git status instead of waiting for the next 60s poll. - workspace-git-diff route: verify a traversal oldPath is forwarded to core and surfaced as available:false rather than escaping the workspace. - Design doc: /diff is handled via setDiffWorkspaceCwd, not setActivePanel. --- .../2026-07-16-webshell-git-status-diff.md | 4 +-- .../serve/routes/workspace-git-diff.test.ts | 25 +++++++++++++++++ .../components/GitBranchIndicator.test.tsx | 14 ++++++++++ .../components/dialogs/GitDiffDialog.test.tsx | 3 +++ .../sidebar/WorkspaceSection.test.tsx | 27 +++++++++++++++++-- 5 files changed, 69 insertions(+), 4 deletions(-) diff --git a/docs/design/2026-07-16-webshell-git-status-diff.md b/docs/design/2026-07-16-webshell-git-status-diff.md index e14515272a1..52b2e098cfe 100644 --- a/docs/design/2026-07-16-webshell-git-status-diff.md +++ b/docs/design/2026-07-16-webshell-git-status-diff.md @@ -500,8 +500,8 @@ stash / detached 的 aria-label 与 tooltip、`GitDiffDialog` 的 header / 列 compact / expanded 形态；可点击 aria。 - `GitDiffDialog`：文件列表渲染（binary / untracked / deleted 标记）；点击展开 按需拉 hunk；`available === false` 占位；文件名 sanitize；hunk 行着色。 -- `/diff` 本地化：`App.tsx` 收到 `/diff` 时 `setActivePanel('diff')` 而非透传 - daemon（参考 `App.test.tsx` 现有 panel 分支用例）。 +- `/diff` 本地化：`App.tsx` 收到 `/diff` 时本地拦截，`setDiffWorkspaceCwd(<当前 cwd>)` + 打开工作区 Changes 弹窗而非透传 daemon（无 cwd 时 toast 提示；参考 `App.test.tsx` 现有用例）。 ### Integration / browser verification diff --git a/packages/cli/src/serve/routes/workspace-git-diff.test.ts b/packages/cli/src/serve/routes/workspace-git-diff.test.ts index e6784ca464a..629cec736b2 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.test.ts @@ -229,6 +229,31 @@ describe('workspace Git diff routes', () => { ); }); + it('surfaces a traversal oldPath as unavailable via core normalization', async () => { + // The route forwards oldPath verbatim; fetchGitDiffHunksForFile rejects `..` + // traversal (returns null) and the route surfaces that as available:false + // rather than erroring or escaping the workspace. + fetchGitDiffHunksForFileMock.mockResolvedValue(null); + const app = express(); + registerWorkspaceGitDiffRoutes(app, { + boundWorkspace: '/work/main', + sendBridgeError, + }); + + const response = await request(app).get( + '/workspace/git/diff/file?path=ok.ts&oldPath=../../etc/passwd', + ); + + expect(response.status).toBe(200); + expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( + '/work/main', + 'ok.ts', + '../../etc/passwd', + ); + expect(response.body.available).toBe(false); + expect(response.body.hunks).toEqual([]); + }); + it('surfaces the truncated flag when the diff was capped', async () => { fetchGitDiffHunksForFileMock.mockResolvedValue({ hunks: [ diff --git a/packages/web-shell/client/components/GitBranchIndicator.test.tsx b/packages/web-shell/client/components/GitBranchIndicator.test.tsx index 8a45cc61217..d869f7574a7 100644 --- a/packages/web-shell/client/components/GitBranchIndicator.test.tsx +++ b/packages/web-shell/client/components/GitBranchIndicator.test.tsx @@ -185,6 +185,20 @@ describe('GitBranchIndicator', () => { expect(chip().querySelector('[data-tone]')).toBeNull(); }); + it('labels a known-clean working tree in the accessible label', () => { + // The clean aria-label branch fires only when a real status snapshot exists + // (computedAt defined) and every change counter is zero — distinguishing + // "known clean" from "no status yet" (which omits the clean phrase). + render({ + branch: 'main', + status: { v: 2, workspaceCwd: '/repo', branch: 'main', computedAt: 1 }, + }); + + const label = chip().getAttribute('aria-label') ?? ''; + expect(label).toContain('Current Git branch: main'); + expect(label).toContain('Working tree clean'); + }); + it('localizes the accessible branch label', () => { expect(getTranslator('en')('git.currentBranch', { branch: 'main' })).toBe( 'Current Git branch: main', diff --git a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx index 926f8fcbcd8..64c1e582eee 100644 --- a/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDiffDialog.test.tsx @@ -374,6 +374,7 @@ describe('GitDiffDialog', () => { const header = document.body.querySelector( 'button[aria-expanded="false"]', ) as HTMLButtonElement; + expect(header).not.toBeNull(); await act(async () => { header.click(); }); @@ -404,6 +405,7 @@ describe('GitDiffDialog', () => { const header = document.body.querySelector( 'button[aria-expanded="false"]', ) as HTMLButtonElement; + expect(header).not.toBeNull(); await act(async () => { header.click(); }); @@ -446,6 +448,7 @@ describe('GitDiffDialog', () => { const header = document.body.querySelector( 'button[aria-expanded="false"]', ) as HTMLButtonElement; + expect(header).not.toBeNull(); await act(async () => { header.click(); }); diff --git a/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx index daae67b6728..278ff271a9f 100644 --- a/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx +++ b/packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx @@ -68,6 +68,8 @@ function renderSection( overrides: Partial<{ workspace: DaemonWorkspaceCapability; onOpenGitDiff: (cwd: string) => void; + client: DaemonClient; + reloadToken: number; }> = {}, ): void { act(() => { @@ -75,8 +77,8 @@ function renderSection( { expect(gitChip()).toBeNull(); }); + it('re-fetches git status when reloadToken changes', async () => { + // reloadToken is in the polling effect's dependency array so agent activity + // (which bumps it) refreshes the chip immediately instead of waiting for the + // next 60s tick. A stable client isolates the re-fetch to the token change. + workspaceGit.mockResolvedValue({ + v: 2, + workspaceCwd: '/tmp/project', + branch: 'main', + }); + const client = makeClient(); + const onOpenGitDiff = vi.fn(); + + renderSection({ client, reloadToken: 0, onOpenGitDiff }); + await flush(); + expect(workspaceGit).toHaveBeenCalledTimes(1); + + renderSection({ client, reloadToken: 1, onOpenGitDiff }); + await flush(); + expect(workspaceGit).toHaveBeenCalledTimes(2); + }); + it('hides the chip when the workspace is not a git repo (null branch)', async () => { workspaceGit.mockResolvedValue({ v: 2, From 7740a9b2f94205638bf0510ca2bcc5f3f93b63df Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sat, 18 Jul 2026 17:21:29 +0800 Subject: [PATCH 24/24] fix(core): allow literal `..foo` paths in diff normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - toRepoRelativePath: reject only a real climb-out (`..` or `../…`), not a literal `..foo` filename at the repo root, which the bare startsWith('..') over-rejected, leaving the diff viewer unable to render such a file. - parseGitDiff: cover the truncatedPaths output set directly (it was only exercised indirectly through fetchGitDiffHunksForFile). --- packages/core/src/utils/gitDiff.test.ts | 35 +++++++++++++++++++++++++ packages/core/src/utils/gitDiff.ts | 10 ++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index 247b9f65481..fcad20b953b 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -270,6 +270,24 @@ index 1111111..2222222 100644 const hunk = result.get('big.ts')![0]; expect(hunk.lines.length).toBe(MAX_LINES_PER_FILE); }); + + it('records the capped file in the provided truncatedPaths set', () => { + const header = `diff --git a/big.ts b/big.ts +index 1111111..2222222 100644 +--- a/big.ts ++++ b/big.ts +@@ -1,${MAX_LINES_PER_FILE + 50} +1,${MAX_LINES_PER_FILE + 50} @@ +`; + const body = Array.from( + { length: MAX_LINES_PER_FILE + 50 }, + (_, i) => ` line${i}`, + ).join('\n'); + const truncatedPaths = new Set(); + parseGitDiff(header + body + '\n', truncatedPaths); + // The caller keys its `truncated` flag off this set, so it must name the + // file that actually lost lines to the cap. + expect(truncatedPaths.has('big.ts')).toBe(true); + }); }); describe('fetchGitDiff', () => { @@ -695,6 +713,23 @@ describe('fetchGitDiffHunksForFile', () => { expect(await fetchGitDiffHunksForFile(repo, outside)).toBeNull(); }); + it('accepts a literal `..foo` filename at the root (not a traversal)', async () => { + // A file literally named `..foo` is not a `..` segment; the absolute-path + // normalization must allow it (a bare startsWith('..') wrongly rejected it, + // so the diff viewer could not render such a file). + await fs.writeFile(path.join(repo, '..foo'), 'one\ntwo\n'); + await git(repo, 'add', '.'); + await git(repo, 'commit', '-q', '-m', 'init'); + await fs.writeFile(path.join(repo, '..foo'), 'one\nTWO\n'); + + const result = await fetchGitDiffHunksForFile( + repo, + path.join(repo, '..foo'), + ); + expect(result).not.toBeNull(); + expect(result!.hunks[0].lines.some((l) => l === '+TWO')).toBe(true); + }); + it('returns null outside a git repo', async () => { const plain = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-plain-')); try { diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index 6c5f88aca4a..88e8c3dc4be 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -414,7 +414,15 @@ function toRepoRelativePath(gitRoot: string, filePath: string): string | null { return filePath; } const rel = path.relative(gitRoot, filePath); - if (rel === '' || rel.startsWith('..') || path.isAbsolute(rel)) return null; + // Reject only a real climb-out (`..` or `../…`), not a literal `..foo` + // filename at the root, which a bare `startsWith('..')` would over-reject. + if ( + rel === '' || + rel === '..' || + rel.startsWith(`..${path.sep}`) || + path.isAbsolute(rel) + ) + return null; return rel; }