fix(sidebar): 二轮独立验证的 9 项缺陷 + 目录级暂存

verifier 二轮报告(F1–F9)全部修复,每条都有「临时回退即变红」的判别性验证:

- F1【高】中文等非 latin1 下载名 → 真实宿主 500:content-disposition 改用
  RFC 5987(ASCII 回退 + filename*=UTF-8'');补真 node:http 用例与根因对照断言
- F2【中】多选同名文件归档重名(解压互相覆盖):冲突项逐级上溯父目录消歧
- F3【中低】maxBytes 在读完之后才判定;budget<=0 的 0 字节 FIFO 会永久阻塞:
  改为先 stat、读取前拒绝(FIFO 探针验证)
- F4【中低】SSH 模式 + 宿主不可用时 reveal 整条消失:保留 kind==='reveal' 目标
- F5【低】图钉嵌套交互语义非法且键盘不可达:改为诚实的鼠标热区,键盘入口归设置面板
- F6【低】变更树空闲 tick 全量重渲染:行 memo + 恒定 props + 内容键派生 +
  共享 store 的 sameStatus 保住快照身份
- F7【低】排序随运行时 locale 漂移:显式 'en' collation + 码点兜底
- F8【低】POSIX 文件名的反斜杠被当成层级:不再替换
- F9【低】打包中重复点击静默、无反馈:busy 行(role=status)+ in-flight 守卫
- 新增:目录行「暂存/取消暂存该目录」(git pathspec 语义,不触发折叠)

验证:typecheck / lint / test(134 文件 1441 通过)/ build 全绿;
DSH_CMD='npx -y --package @deepseek-ai/dsh@0.1.7-rc.1 dsh' pnpm test:mount 7/7 通过
This commit is contained in:
Menghuan1918
2026-09-28 18:20:08 +08:00
parent b011be1e98
commit 13d4bae321
17 changed files with 846 additions and 127 deletions
@@ -476,3 +476,21 @@ ChangesTab
② 变动页 Git 视角以树显示:`src/client/changes`(单子目录链压缩成一行,带计数胶囊 1)→ `GitLens.tsx`(M 徽标),已暂存组 `src` → `app.ts`。
- 单文件页回归:14 个 spec / 91 用例绿(含新增 `file-tree-archive.spec.tsx` 9 例:出现条件、成功链、三条失败路径、in-flight 守卫)。
- 仍未验证:真实浏览器里 ZIP 的落盘行为(jsdom spy 证据)、拖拽/多选手感、Windows 宿主差异。
### 9.5 第三轮(verifier 二轮报告后的修复,全部完成)
| 编号 | 问题 | 修法 | 判别性验证 |
|---|---|---|---|
| F1【高】 | 中文等非 latin1 下载名 → 真实宿主 **500**(`content-disposition` 直接塞原始名,Node `writeHead` 拒绝 >U+00FF) | `archive-route.ts` 新增 `contentDispositionOf`:ASCII 回退 + `filename*=UTF-8''…`(无可用 ASCII stem 时回退 `download.zip`) | 临时回退 → 真 `node:http` 用例 500;对照断言钉住 `validateHeaderValue` 必抛 |
| F2【中】 | 多选同名不同目录 → 归档条目重名(解压互相覆盖) | `disambiguateArchiveNames`:只对冲突项逐级上溯父目录,必要时回退完整路径 | 临时回退 → 条目重名断言变红 |
| F3【中低】 | `maxBytes` 在读完源文件后才判定(与注释相反);且 `budget===0` 时 0 字节 FIFO 仍会 `open()` 永久阻塞 | `prepareFile` 先 `stat`,`budget <= 0 || size > budget` 立即抛,再读 | FIFO 探针:回退后 5s 超时(阻塞),修复后 44ms 通过(Windows 跳过该探针) |
| F4【中低】 | SSH 模式下宿主不可用时 reveal 整条消失 | SSH 过滤保留 `kind === 'reveal'` 的目标,并注明理由 | 更新 open-with.spec 断言(`['explorer','vscode','cursor','custom:e2']`,reveal 恰 1 条) |
| F5【低】 | 图钉是 `<span role="button" tabIndex={-1}>` 嵌在 menuitem 按钮内(语义非法 + 键盘不可达) | 去掉 role/tabIndex,改成诚实的鼠标热区;注释写明键盘入口是设置面板 | 临时加回 role → 用例变红 |
| F6【低】 | 变更树行无 memo,空闲 tick 全量重渲染 | ① 行 `memo` + 恒定 props(图标解析搬进行内当探针)② 内容键 `entriesKey` 派生 tree ③ (Lead)共享 store 的 `sameStatus` 保住快照身份 | 三处各自临时回退 → 探针变红 |
| F7【低】 | 排序依赖运行时 locale collation | `localeCompare(other, 'en', { sensitivity: 'base' })` + 码点兜底 | 反向输入顺序不变 + 瑞典 collation 对照断言 |
| F8【低】 | POSIX 文件名里的 `\` 被静默转成层级 | `archiveName` 不再替换 `\` | 实探条目 `a\b.txt` 保持单条成员 |
| F9【低】 | 打包中重复点击静默吞掉、大选择无反馈 | `archiveBusy` state + 树顶 `Notice kind="loading" role="status"` | deferred fetch 断言忙态行出现/消失、重复点选 fetch 仍 1 次 |
| — | 新增交互:目录行支持「暂存/取消暂存该目录」 | `DirRow` hover IconButton(`gitStage`/`gitUnstage` 均接受目录 pathspec) | 断言 `gitStage(scope,'src/deep',MAIN)` 且点击不触发折叠 |
最终门禁(修复后重跑):`pnpm typecheck` / `pnpm lint` / `pnpm test`(**134 文件 / 1441 通过 / 9 跳过**)/ `pnpm build` 全绿;
`DSH_CMD='npx -y --package @deepseek-ai/dsh@0.1.7-rc.1 dsh' pnpm test:mount` → **7/7 通过**(mount=372ms,p95 帧间隔 16ms,无宽度泄漏)。
+86 -1
View File
@@ -11,7 +11,7 @@
* so it can neither escape the workspace nor cycle.
*/
import { lstat, readdir } from 'node:fs/promises'
import { join } from 'node:path'
import { basename, dirname, join } from 'node:path'
import { SidebarError } from './wire.ts'
import { ZIP_MAX_ENTRIES, type ZipEntry } from './zip.ts'
@@ -44,6 +44,91 @@ export function archiveNameOf(raw: string | null): string {
return cleaned.toLowerCase().endsWith('.zip') ? cleaned : `${cleaned}.zip`
}
/**
* The `content-disposition` value for one archive download.
*
* A non-latin1 name (a Chinese folder → `报告.zip`) CANNOT go into the header
* verbatim: Node's `writeHead` validates header values and rejects anything
* above U+00FF with `Invalid character in header content` — the route turned
* that into a 500. So the name is sent twice, exactly like `/sidebar/file`
* does for a download: an ASCII-only `filename="…"` fallback for old clients
* and the RFC 5987 `filename*=UTF-8''…` form (percent-encoded, therefore pure
* ASCII) for everyone else, which wins in every current browser.
*/
export function contentDispositionOf(name: string): string {
return `attachment; filename="${asciiFallbackOf(name)}"; filename*=UTF-8''${encodeURIComponent(name)}`
}
/**
* The ASCII-only fallback of a download name: characters a quoted header
* string cannot carry (or that no latin1 client could render) drop out, and a
* stray quote or backslash does too. A result with no stem left (`''` or a
* bare extension, as a fully non-latin1 name reduces to) becomes
* `download.zip`, so the header always names a usable file.
*/
function asciiFallbackOf(name: string): string {
const ascii = [...name]
.filter(char => {
const code = char.codePointAt(0)!
return code >= 0x20 && code <= 0x7E && char !== '"' && char !== '\\'
})
.join('')
const stem = ascii.replace(/\.zip$/i, '')
if (stem === '' || /^\.+$/.test(stem)) return 'download.zip'
return ascii
}
/**
* The in-archive name of every selection, disambiguated.
*
* The common case keeps the intuition "one selection is named by its own
* basename" — archive `/ws/src` and the entries live under `src/`. Two
* selections that would collide (multi-select `/ws/a/index.ts` and
* `/ws/b/index.ts` both wanting `index.ts`) are extended with parent
* segments until every name is unique, so an extractor cannot silently
* overwrite one member with another. A path that runs out of ancestors
* (selecting the filesystem root, or `/a` twice) falls back to the full
* '/'-joined path, which is unique by construction.
*
* @param selections - absolute (already fenced) paths, in selection order.
* @returns one '/'-separated archive name per selection, same order.
*/
export function disambiguateArchiveNames(selections: readonly string[]): string[] {
const segmentsOf = (absolute: string, depth: number): string => {
const parts: string[] = []
let current = absolute
for (let level = 0; level < depth; level += 1) {
const part = basename(current)
const parent = dirname(current)
if (part === '' || parent === current) break
parts.unshift(part)
current = parent
}
return parts.join('/')
}
const deepEnough = (absolute: string, name: string): string => {
const full = segmentsOf(absolute, Number.MAX_SAFE_INTEGER)
// Still colliding after every segment is in play (two identical paths, or
// a root-level duplicate): the full path is the best available identity.
return full === name ? absolute.replace(/^\/+/, '') : name
}
const depth = new Map<string, number>(selections.map(path => [path, 1]))
for (let round = 0; round < 64; round += 1) {
const names = selections.map(path => segmentsOf(path, depth.get(path)!))
const counts = new Map<string, number>()
for (const name of names) counts.set(name, (counts.get(name) ?? 0) + 1)
if ([...counts.values()].every(count => count === 1)) return names
const grown = new Set<string>()
for (const [index, path] of selections.entries()) {
if ((counts.get(names[index]!) ?? 0) <= 1) continue
depth.set(path, depth.get(path)! + 1)
grown.add(path)
}
if (grown.size === 0) break
}
return selections.map(path => deepEnough(path, segmentsOf(path, depth.get(path)!)))
}
/**
* Collect one selected path into the archive: a file becomes one entry, a
* directory is walked depth-first and each level contributes its own entry
+35 -11
View File
@@ -1032,16 +1032,20 @@ export function FileTree(props: {
label: (
<span className={css.openWithLabel}>
<span className={css.openWithName}>{openWithLabelOf(target)}</span>
{/* The pushpin: a span (never a button — the Menu row itself is
a button, so a nested interactive element would be invalid).
Clicking it pins/unpins the target at the menu's top level
WITHOUT selecting the row: the pin stops propagation, so the
menu stays open and the icon flips on the next render. */}
{/* The pushpin: a PLAIN span, deliberately not a control. The
Menu renders the row itself as `<button role="menuitem">`, so a
nested button/imitation-button would be invalid markup — and
anything with `tabIndex` inside it is unreachable by keyboard
anyway (the row button swallows focus). It is therefore an
honest MOUSE hot zone: hovering the row reveals it, clicking it
pins/unpins without selecting the row (stopPropagation), and it
keeps a `title` so the affordance is still nameable on hover.
Keyboard/AT users have a real, reachable path to the same list:
the plugin settings panel's pinned-targets editor
(OpenWithSettings). Do not re-add role/tabIndex here without
also making the row itself non-interactive. */}
<span
role="button"
tabIndex={-1}
className={clsx(css.openWithPin, pinnedNow && css.openWithPinActive)}
aria-label={pinnedNow ? t('unpinOpenWith') : t('pinOpenWith')}
title={pinnedNow ? t('unpinOpenWith') : t('pinOpenWith')}
onClick={(event) => {
event.preventDefault()
@@ -1172,9 +1176,15 @@ export function FileTree(props: {
* `.zip`. So the bytes are fetched explicitly: a non-2xx answer is parsed for
* its `{error: {message}}` envelope (the shape `api.ts` reads) and reported
* through `zipFailed`; a 2xx body becomes an object URL handed to the same
* hidden-anchor mechanics. `archiveBusyRef` keeps a double click from
* packaging the same selection twice.
* hidden-anchor mechanics.
*
* The host builds the archive in one pass, which can take a while for a big
* selection: `archiveBusy` drives the progress line under the error strip
* while the request is in flight, and `archiveBusyRef` keeps a repeated pick
* from packaging the same selection twice (the guard is silent by design —
* the line it is already showing IS the feedback).
*/
const [archiveBusy, setArchiveBusy] = useState(false)
const archiveBusyRef = useRef(false)
const downloadArchive = (paths: readonly string[]): void => {
if (archiveBusyRef.current) return
@@ -1187,6 +1197,7 @@ export function FileTree(props: {
return
}
archiveBusyRef.current = true
setArchiveBusy(true)
void fetch(url)
.then(async (response) => {
if (!response.ok) {
@@ -1214,7 +1225,10 @@ export function FileTree(props: {
.catch((error: unknown) => {
setActionError(t('zipFailed', { message: error instanceof Error ? error.message : String(error) }))
})
.finally(() => { archiveBusyRef.current = false })
.finally(() => {
archiveBusyRef.current = false
setArchiveBusy(false)
})
}
const root = cwd
@@ -1450,6 +1464,16 @@ export function FileTree(props: {
/>
</div>
)}
{/* The archive is built host-side in one pass: while it runs, the
row sits right under the error strip (the two CAN coexist — a
stale error stays readable while a new attempt is in flight).
`role="status"` makes it a polite live region, so the busy state
is announced without stealing focus. */}
{archiveBusy && (
<Notice kind="loading" tone="inline" role="status">
{t('loading')}
</Notice>
)}
{selected.size > 0 && (
<SectionHeader
className={css.explorerSelectionBar}
+151 -64
View File
@@ -26,7 +26,7 @@
* failures used to land under the commit box, which read as "your commit
* failed" for an action the user never ran.
*/
import { useCallback, useEffect, useMemo, useRef, useState, type MouseEvent, type ReactNode } from 'react'
import { memo, useCallback, useEffect, useMemo, useRef, useState, type MouseEvent, type ReactNode } from 'react'
import {
Button, IconChevronRightOutlineRegular, IconCodeOutlineRegular, IconCopyOutlineRegular, IconPlusOutlineRegular,
IconTrashOutlineRegular, Input, Menu, writeClipboard,
@@ -125,31 +125,62 @@ const WORKTREE_POLL_MS = 30_000
* just because `snapshot?.entries ?? []` minted a new array. */
const NO_ENTRIES: readonly GitStatusEntry[] = []
/**
* A CONTENT key for one status answer. The shared store mints a brand-new
* entries array for every poll — idle polls with byte-identical content
* included — so array identity cannot decide whether the tree must be rebuilt.
* This string can: the same paths and porcelain codes always produce the same
* key, and a key change is exactly when the tree, its counts and the row
* objects need to be recomputed.
*/
function entriesKey(entries: readonly GitStatusEntry[]): string {
let key = ''
for (const entry of entries) key += `${entry.xy}\u0000${entry.path}\n`
return key
}
/** The preview ref for one changed file (one ref per path+side). */
function worktreeRefOf(
path: string,
untracked: boolean,
staged: boolean,
worktree: string | undefined,
repoRoot: string | undefined,
): SidebarDiffRef {
return { kind: 'worktree', path, staged, untracked, worktree, repoRoot }
}
/** One changed file's row: the host's file artwork + porcelain letter + name
* + the trailing stage action (revealed on hover/focus). */
function FileRow(props: {
node: ChangeFile
icon: ReactNode
* + the trailing stage action (revealed on hover/focus).
*
* Memoized, and every prop keeps its identity across a status poll that
* changed nothing: the tree is content-keyed (so `node` is the same object),
* the callbacks are state-free, and `glyph` is a stable resolver. The icon
* resolution inside the row therefore doubles as the re-render probe the
* specs use — an idle poll must not resolve a single icon. */
const FileRow = memo(function FileRow(props: {
row: ChangeFile
glyph: (path: string) => ReactNode
staged: boolean
busy: boolean
selected: boolean
onPreview(): void
onContextMenu(event: MouseEvent): void
onToggleStage(): void
onPreview: (node: ChangeFile, staged: boolean) => void
onContextMenu: (event: MouseEvent, node: ChangeFile, staged: boolean) => void
onToggleStage: (path: string, staged: boolean) => void
}): ReactNode {
const { node, staged, busy, selected } = props
const { row: node, staged, busy, selected } = props
const action = staged ? t('unstage') : t('stage')
return (
<div className={css.row} data-selected={selected ? 'true' : undefined}>
<div className={css.row} data-row={node.path} data-selected={selected ? 'true' : undefined}>
<button
type="button"
className={css.rowMain}
data-path={node.path}
title={node.path}
onClick={props.onPreview}
onContextMenu={props.onContextMenu}
onClick={() => { props.onPreview(node, staged) }}
onContextMenu={(event) => { props.onContextMenu(event, node, staged) }}
>
<span className={css.rowIcon} aria-hidden="true">{props.icon}</span>
<span className={css.rowIcon} aria-hidden="true">{props.glyph(node.path)}</span>
<StatusBadge tone={BADGE_TONE[node.status.tone]} title={node.path}>{node.status.letter}</StatusBadge>
<span className={css.rowName}>{node.name}</span>
</button>
@@ -159,34 +190,62 @@ function FileRow(props: {
disabled={busy}
label={action}
icon={staged ? <IconTrashOutlineRegular size={14} /> : <IconPlusOutlineRegular size={14} />}
onClick={props.onToggleStage}
onClick={() => { props.onToggleStage(node.path, staged) }}
/>
</div>
)
}
})
/** One directory row: the disclosure chevron + folder artwork + the label
* (a compressed chain reads as one 'a/b/c' line) + how much changed below. */
function DirRow(props: { node: ChangeDir; icon: ReactNode; open: boolean; onToggle(): void }): ReactNode {
const { node, open } = props
* (a compressed chain reads as one 'a/b/c' line) + how much changed below,
* plus the SAME trailing action a file row carries — applied to the whole
* subtree (`git add -A -- <dir>` / `git reset -q -- <dir>`, both of which
* take a directory pathspec). The action is a sibling of the disclosure
* button, so a click can never fold the row; it stops propagation anyway so
* the intent survives a future nesting change. Memoized like {@link FileRow}. */
const DirRow = memo(function DirRow(props: {
row: ChangeDir
folderGlyph: (path: string, open: boolean) => ReactNode
staged: boolean
open: boolean
busy: boolean
onToggle: (key: string) => void
onToggleStage: (path: string, staged: boolean) => void
}): ReactNode {
const { row: node, staged, open, busy } = props
const key = `${staged ? 's' : 'u'}:${node.path}`
const action = staged ? t('unstageAll') : t('stageAll')
return (
<button
type="button"
className={css.dirRow}
data-dir={node.path}
aria-expanded={open}
title={node.path}
onClick={props.onToggle}
>
<span className={css.chevron} data-open={open ? 'true' : undefined} aria-hidden="true">
<IconChevronRightOutlineRegular size={12} />
</span>
<span className={css.rowIcon} aria-hidden="true">{props.icon}</span>
<span className={css.dirName}>{node.name}</span>
<span className={css.countPill}>{node.changes}</span>
</button>
<div className={css.row} data-row={node.path}>
<button
type="button"
className={css.dirRow}
data-dir={node.path}
aria-expanded={open}
title={node.path}
onClick={() => { props.onToggle(key) }}
>
<span className={css.chevron} data-open={open ? 'true' : undefined} aria-hidden="true">
<IconChevronRightOutlineRegular size={12} />
</span>
<span className={css.rowIcon} aria-hidden="true">{props.folderGlyph(node.path, open)}</span>
<span className={css.dirName}>{node.name}</span>
<span className={css.countPill} data-count={node.changes}>{node.changes}</span>
</button>
<IconButton
size="sm"
className={css.rowAction}
disabled={busy}
label={action}
icon={staged ? <IconTrashOutlineRegular size={14} /> : <IconPlusOutlineRegular size={14} />}
onClick={(event) => {
event.stopPropagation()
props.onToggleStage(node.path, staged)
}}
/>
</div>
)
}
})
export interface GitLensProps {
scope: SessionScope
@@ -575,28 +634,12 @@ export function GitLens(props: GitLensProps) {
void writeClipboard(text)
}
const openFileMenu = (event: MouseEvent, path: string, staged: boolean, untracked: boolean): void => {
event.preventDefault()
event.stopPropagation()
setFileMenu({ path, staged, untracked, x: event.clientX, y: event.clientY })
}
const openHistoryMenu = (event: MouseEvent, entry: GitLogEntry): void => {
event.preventDefault()
event.stopPropagation()
setHistoryMenu({ entry, x: event.clientX, y: event.clientY })
}
/** The preview ref for one changed file (one ref per path+side). */
const worktreeRefOf = (path: string, untracked: boolean, staged: boolean): SidebarDiffRef => ({
kind: 'worktree',
path,
staged,
untracked,
worktree: selectedWorktree,
repoRoot,
})
/** The preview ref for one commit. */
const commitRefOf = (entry: GitLogEntry): SidebarDiffRef => ({
kind: 'commit',
@@ -615,8 +658,11 @@ export function GitLens(props: GitLensProps) {
}
const entries = snapshot?.entries ?? NO_ENTRIES
// One fold per group, memoized on the snapshot's own entry array: the tree
// (compression + sorting + counts) is rebuilt only when git answers anew.
// The CONTENT of the answer decides whether the tree must be rebuilt; the
// array identity only decides whether the (cheap) key is recomputed.
const contentKey = useMemo(() => entriesKey(entries), [entries])
// One fold per group, rebuilt only when the change list's CONTENT changes —
// never on a poll that answered the same files, however fresh its arrays are.
const { stagedEntries, unstagedEntries, stagedTree, unstagedTree } = useMemo(() => {
const staged = entries.filter(isStagedEntry)
const unstaged = entries.filter(isUnstagedEntry)
@@ -626,34 +672,67 @@ export function GitLens(props: GitLensProps) {
stagedTree: buildChangeTree(staged),
unstagedTree: buildChangeTree(unstaged),
}
}, [entries])
// The key stands in for `entries`: same content ⇒ same tree objects ⇒ the
// memoized rows below keep their props and never re-render.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [contentKey])
const isRepo = snapshot?.isRepo === true
const branch = snapshot?.branch ?? ''
const branchOptions = branch === '' ? branchNames : [branch, ...branchNames.filter(name => name !== branch)]
const clean = isRepo && stagedEntries.length === 0 && unstagedEntries.length === 0
/** Fold or unfold one directory row (keyed per group, see `collapsed`). */
const toggleDir = (key: string): void => {
/** Fold or unfold one directory row (keyed per group, see `collapsed`).
* Stable: it is a memoized row's prop. */
const toggleDir = useCallback((key: string): void => {
setCollapsed((current) => {
const next = new Set(current)
if (next.has(key)) next.delete(key)
else next.add(key)
return next
})
}
}, [])
/** The latest-value holder for the actions a memoized row calls: a row's
* props must keep their identity across polls, so these callbacks read the
* current closures through the ref instead of closing over them. */
const rowActions = useRef({
preview: (_node: ChangeFile, _staged: boolean): void => { /* replaced below */ },
stage: (_path: string, _staged: boolean): void => { /* replaced below */ },
})
useEffect(() => {
rowActions.current = {
preview: (node, staged) => {
onPreview(worktreeRefOf(node.path, isUntrackedStatus(node.status), staged, chosenPathRef.current, repoRootRef.current))
},
stage: (path, staged) => { stageEntry(path, staged) },
}
})
/** Stable row callbacks (see rowActions). */
const previewFile = useCallback((node: ChangeFile, staged: boolean): void => {
rowActions.current.preview(node, staged)
}, [])
const stagePath = useCallback((path: string, staged: boolean): void => {
rowActions.current.stage(path, staged)
}, [])
const openRowMenu = useCallback((event: MouseEvent, node: ChangeFile, staged: boolean): void => {
event.preventDefault()
event.stopPropagation()
setFileMenu({ path: node.path, staged, untracked: isUntrackedStatus(node.status), x: event.clientX, y: event.clientY })
}, [])
/** One file row of a group's tree (the single file-row renderer). */
const renderFile = (node: ChangeFile, staged: boolean): ReactNode => (
<FileRow
key={`${staged ? 's' : 'u'}:${node.path}`}
node={node}
icon={fileGlyph(node.path)}
key={`file:${staged ? 's' : 'u'}:${node.path}`}
row={node}
glyph={fileGlyph}
staged={staged}
busy={busy}
selected={isPreviewedWorktree(node.path, staged)}
onPreview={() => { onPreview(worktreeRefOf(node.path, isUntrackedStatus(node.status), staged)) }}
onContextMenu={(event) => { openFileMenu(event, node.path, staged, isUntrackedStatus(node.status)) }}
onToggleStage={() => { stageEntry(node.path, staged) }}
onPreview={previewFile}
onContextMenu={openRowMenu}
onToggleStage={stagePath}
/>
)
@@ -662,8 +741,16 @@ export function GitLens(props: GitLensProps) {
const key = `${staged ? 's' : 'u'}:${node.path}`
const open = !collapsed.has(key)
return (
<div key={key}>
<DirRow node={node} icon={folderGlyph(node.path, open)} open={open} onToggle={() => { toggleDir(key) }} />
<div key={`dir:${key}`}>
<DirRow
row={node}
folderGlyph={folderGlyph}
staged={staged}
open={open}
busy={busy}
onToggle={toggleDir}
onToggleStage={stagePath}
/>
{open && (
<div className={css.treeChildren}>
{node.children.map(child => (child.kind === 'dir' ? renderDir(child, staged) : renderFile(child, staged)))}
+18 -4
View File
@@ -47,12 +47,26 @@ interface DirDraft {
files: ChangeFile[]
}
/** Directories before files, then by name — case-insensitively, with the raw
* name breaking ties so the order never depends on the locale's collation. */
/**
* Directories before files, then by name — case-insensitively.
*
* The collation is PINNED (`'en'`, `sensitivity: 'base'`) rather than left to
* the runtime's default locale: `localeCompare(other)` alone follows whatever
* locale the browser/Node happens to run under, so the same change list could
* order differently on two machines. Base sensitivity makes case and accent
* variants compare equal, and the code-point tiebreak below then decides —
* a total order that does not move when the input order changes.
*/
function compareNames(left: string, right: string): number {
const collated = left.localeCompare(right, 'en', { sensitivity: 'base' })
if (collated !== 0) return collated
if (left === right) return 0
return left < right ? -1 : 1
}
function compareNodes(left: ChangeNode, right: ChangeNode): number {
if (left.kind !== right.kind) return left.kind === 'dir' ? -1 : 1
const folded = left.name.toLowerCase().localeCompare(right.name.toLowerCase())
return folded !== 0 ? folded : left.name.localeCompare(right.name)
return compareNames(left.name, right.name)
}
/** The number of changed files under a finished child list. */
+6 -6
View File
@@ -103,23 +103,23 @@
border-left: 1px solid var(--dsw-alias-border-l1);
}
/* One directory row: disclosure chevron + folder artwork + label + count. */
/* A directory row's disclosure button. It shares the `.row` band with a file
row (the band owns the hover/selected fill and the trailing action), so this
button is only the row's main hit area. */
.dirRow {
flex: 1;
min-width: 0;
display: flex;
align-items: center;
gap: 6px;
width: 100%;
min-height: 28px;
padding: 0 6px 0 2px;
border: none;
border-radius: 4px;
background: transparent;
padding: 0;
color: var(--dsw-alias-label-secondary);
font: inherit;
cursor: pointer;
text-align: left;
}
.dirRow:hover { background: var(--dsw-alias-interactive-bg-hover); }
.dirRow:focus-visible { outline: 2px solid var(--dsw-alias-interactive-bg-hover-accent); outline-offset: -1px; }
.chevron { flex: none; display: inline-flex; color: var(--dsw-alias-label-tertiary); transition: transform 140ms var(--ds-ease-in-out); }
+10 -4
View File
@@ -137,9 +137,15 @@ function customIdOf(id: string): string {
/**
* The menu-visible open targets, in order (built-ins then custom editors).
* In SSH mode the local-only targets (the OS file manager, Zed, custom
* editors without the VSCode dialect) are dropped — they cannot reach a
* remote path. Unknown pinned ids are pruned here too.
* In SSH mode the local-only URL openers (Zed, custom editors without the
* VSCode dialect) are dropped — they cannot reach a remote path. The
* `explorer` target is NOT dropped: it reveals a path in the file manager of
* the machine that owns the workspace, which is exactly where the host half
* runs, and it is the only reveal left when the DSH host's own open-in-app
* capability is unavailable (older host, no desktop session). A headless
* remote cannot raise a file manager anyway — that is the same silent
* no-op the other external launches already have. Unknown pinned ids are
* pruned here too.
*/
export function resolveOpenWithTargets(config: OpenWithConfig): OpenWithTarget[] {
const ssh = config.sshHost.trim() !== ''
@@ -156,7 +162,7 @@ export function resolveOpenWithTargets(config: OpenWithConfig): OpenWithTarget[]
localOnly: !editor.isVscodeFamily,
})),
]
return targets.filter(target => !(ssh && target.localOnly))
return targets.filter(target => !(ssh && target.localOnly && target.kind !== 'reveal'))
}
/** The SSH hint appended to a target's label in remote mode. */
+27
View File
@@ -90,6 +90,24 @@ function joinRoot(root: string, rel: string): string {
return `${base}/${rel.replace(/^[\\/]+/, '')}`
}
/**
* Whether two status snapshots carry the same information. Compared field by
* field (never by serializing the whole object): the entries are the payload
* and their order is stable, so a length + pairwise compare is enough.
*/
function sameStatus(a: GitStatusResult, b: GitStatusResult): boolean {
if (a === b) return true
if (a.isRepo !== b.isRepo || a.branch !== b.branch || a.root !== b.root
|| a.truncated !== b.truncated || a.entries.length !== b.entries.length) return false
if (a.repositories?.length !== b.repositories?.length) return false
if (a.repositories !== undefined && b.repositories !== undefined
&& a.repositories.some((root, index) => root !== b.repositories![index])) return false
return a.entries.every((entry, index) => {
const other = b.entries[index]!
return entry.path === other.path && entry.xy === other.xy
})
}
interface Index {
/** Normalized absolute path → status (files only). */
files: Map<string, GitFileStatus>
@@ -177,6 +195,15 @@ function fetchSlot(slot: Slot): void {
api.gitStatus(scope, slot.worktree)
.then((snapshot) => {
if (generation !== slot.generation) return
// Idle polls return the same answer as a NEW object every 2.5s. Keeping
// the previous identity when nothing changed is what lets every
// downstream memo (the changes tree, the file tree's row model, the
// icon index) hold across ticks — without it the whole list re-rendered
// and re-resolved icons on every poll.
if (slot.snapshot !== null && sameStatus(slot.snapshot, snapshot)) {
slot.error = false
return
}
slot.snapshot = snapshot
slot.error = false
})
+14 -7
View File
@@ -38,7 +38,7 @@ import { isTrustedApiRequest } from './trust-fence.ts'
import { registerBundleRoute } from './bundle-route.ts'
import { createDirectoryWatchers, type DirectoryWatchers } from './fs-watch.ts'
import { launchExternal } from './open-external.ts'
import { archiveNameOf, collectZipEntries } from './archive-route.ts'
import { archiveNameOf, collectZipEntries, contentDispositionOf, disambiguateArchiveNames } from './archive-route.ts'
import { buildZip, type ZipEntry } from './zip.ts'
import * as git from './git.ts'
import { SettingsConflictError } from '@deepseek-ai/dsh-settings'
@@ -68,10 +68,10 @@ export type {
FileViewerProps,
FileFetchStrategy,
} from './client/service.ts'
// The archive walk + name sanitizer are exercised directly by tests/zip.spec.ts:
// The archive walk + name helpers are exercised directly by tests/zip.spec.ts:
// their bounds live inside the collector, so the suite needs them observable
// without creating 10 000 rows first.
export { archiveNameOf, collectZipEntries } from './archive-route.ts'
export { archiveNameOf, collectZipEntries, contentDispositionOf, disambiguateArchiveNames } from './archive-route.ts'
/** Plugin identity for cordis.yml rows. */
export const name = 'dsh-better-sidebar'
@@ -945,15 +945,22 @@ export function apply(ctx: Context, config?: SidebarConfig): void {
const name = archiveNameOf(url.searchParams.get('name'))
const cwd = await sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined)
const fenceOn = fenceEnabledOf(() => settingsFace)
const selected: string[] = []
for (const raw of paths) selected.push(await ensureWorkspacePath(cwd, raw, fenceOn))
// Same-basename selections (a/index.ts + b/index.ts) get parent
// segments prepended, so no two archive members collide and an
// extractor cannot overwrite one with the other.
const entries: ZipEntry[] = []
for (const raw of paths) {
const absolute = await ensureWorkspacePath(cwd, raw, fenceOn)
await collectZipEntries(absolute, basename(absolute), entries)
const names = disambiguateArchiveNames(selected)
for (const [index, absolute] of selected.entries()) {
await collectZipEntries(absolute, names[index]!, entries)
}
const body = await buildZip(entries)
res.writeHead(200, {
'content-type': 'application/zip',
'content-disposition': `attachment; filename="${name}"`,
// Never the raw name: a non-latin1 value (中文目录 → 报告.zip) makes
// Node's writeHead throw and turns the download into a 500.
'content-disposition': contentDispositionOf(name),
'content-length': String(body.byteLength),
'cache-control': 'no-cache',
})
+42 -12
View File
@@ -8,11 +8,12 @@
* for the whole archive. No third-party dependency, no filesystem walking —
* the caller supplies the entries, this module only reads their bytes.
*
* The entries are bounded by `maxEntries` / `maxBytes` before anything is
* read, so a runaway selection fails with a clear error instead of filling
* memory with an archive nobody asked for.
* The entries are bounded by `maxEntries` / `maxBytes`: the count is checked
* up front, and every source is stat'ed against the remaining byte budget
* BEFORE it is read, so a runaway selection fails with a clear error instead
* of filling memory with an archive nobody asked for.
*/
import { readFile } from 'node:fs/promises'
import { readFile, stat } from 'node:fs/promises'
import { deflateRaw } from 'node:zlib'
import { promisify } from 'node:util'
import { SidebarError } from './wire.ts'
@@ -95,11 +96,20 @@ function dosDateTime(date: Date): { time: number; date: number } {
}
}
/** Normalize an entry name to a '/'-separated relative archive path. */
/**
* Normalize an entry name to a '/'-separated relative archive path.
*
* A backslash is NOT a separator here: on POSIX (and on the host half, which
* builds the names) `a\b.txt` is one legal file name, so converting it would
* silently split one file into a directory. Windows separators never reach
* this point — the walk composes names from `basename`/`join` results.
* Leading slashes are dropped (names are archive-relative by definition) and
* `..` segments are refused, so a name can never traverse.
*/
function archiveName(raw: string, isDir: boolean): string {
const normalized = raw.replace(/\\/g, '/').replace(/^\/+/, '')
const normalized = raw.replace(/^\/+/, '')
const segments = normalized.split('/').filter(segment => segment !== '' && segment !== '.')
if (segments.length === 0 || segments.includes('..')) {
if (raw === '..' || segments.length === 0 || segments.includes('..')) {
throw new SidebarError('bad-request', `invalid archive entry name "${raw}"`)
}
const name = segments.join('/')
@@ -114,8 +124,26 @@ function requireSourcePath(raw: string): string {
return raw
}
/** Read + compress one member (deflate only when it actually shrinks). */
async function prepareFile(name: string, path: string): Promise<ZipMember> {
/**
* Read + compress one member (deflate only when it actually shrinks).
* @param limit - the archive's total uncompressed ceiling (for the message).
* @param budget - uncompressed bytes still available; the source is stat'ed
* FIRST and refused when it cannot fit, so an oversized archive never pulls
* the payload into memory (let alone deflates it) before failing.
*/
async function prepareFile(name: string, path: string, limit: number, budget: number): Promise<ZipMember> {
let size: number
try {
size = (await stat(path)).size
} catch (error) {
throw new SidebarError('fs-error', `cannot read "${path}": ${error instanceof Error ? error.message : String(error)}`, 400)
}
// `budget <= 0` refuses even a zero-size source. That is not pedantry: a
// FIFO stats as 0 bytes and reading one BLOCKS until a writer shows up, so
// an exhausted budget must never reach open().
if (budget <= 0 || size > budget) {
throw new SidebarError('fs-error', `archive exceeds the ${limit} byte limit`, 400)
}
let data: Buffer
try {
data = await readFile(path)
@@ -183,14 +211,16 @@ export async function buildZip(entries: readonly ZipEntry[], opts: ZipOptions =
continue
}
const path = requireSourcePath(entry.path)
const member = await prepareFile(archiveName(entry.name, false), path)
// The bound is the archive's total UNCOMPRESSED payload: that is what the
// reader materializes, and it makes the limit independent of how well the
// input happens to compress.
totalBytes += member.size
if (totalBytes > maxBytes) {
const member = await prepareFile(archiveName(entry.name, false), path, maxBytes, maxBytes - totalBytes)
// A source can grow AFTER its stat (sparse files, concurrent writers), so
// the ceiling is re-checked against what was actually read.
if (totalBytes + member.size > maxBytes) {
throw new SidebarError('fs-error', `archive exceeds the ${maxBytes} byte limit`, 400)
}
totalBytes += member.size
members.push(member)
}
+25
View File
@@ -250,6 +250,31 @@ describe('change tree (the Git lens reads its groups through it)', () => {
])
})
it('pins the collation, so the order cannot move with the runtime locale', () => {
// Mixed case + accents + non-letters: the comparator runs an explicit
// 'en' base collation and breaks ties by code point. A bare
// `localeCompare(other)` would follow the machine's ICU default and could
// reorder the same change list on another machine.
const names = ['b.ts', 'A.ts', 'ä.ts', 'Z.ts', 'a.ts', 'Ä.ts', '_x.ts', '1.ts']
const order = (list: readonly string[]): string[] =>
buildChangeTree(list.map(path => entry(path))).map(node => node.name)
expect(order(names)).toEqual(['_x.ts', '1.ts', 'A.ts', 'a.ts', 'Ä.ts', 'ä.ts', 'b.ts', 'Z.ts'])
// …and it is a TOTAL order: reversing the input cannot move a row.
expect(order([...names].reverse())).toEqual(order(names))
const dirs = ['Zdir/f.ts', 'adir/f.ts', 'Ädir/f.ts', '_dir/f.ts']
const dirOrder = (list: readonly string[]): string[] =>
buildChangeTree(list.map(path => entry(path))).map(node => node.name)
expect(dirOrder(dirs)).toEqual(['_dir', 'adir', 'Ädir', 'Zdir'])
expect(dirOrder([...dirs].reverse())).toEqual(dirOrder(dirs))
// Why the pin is load-bearing: under Swedish collation 'ä' sorts AFTER
// 'z', so an unpinned comparator would order this very list differently on
// a machine whose runtime locale happens to be Swedish.
expect(['z.ts', 'ä.ts'].sort((left, right) => left.localeCompare(right, 'sv', { sensitivity: 'base' })))
.toEqual(['z.ts', 'ä.ts'])
})
it('returns no nodes for an empty or all-clean list', () => {
expect(buildChangeTree([])).toEqual([])
expect(buildChangeTree([entry('a.ts', ' '), entry('b.ts', '!!')])).toEqual([])
+40
View File
@@ -314,4 +314,44 @@ describe('FileTree zip and download', () => {
expect(fetchMock).toHaveBeenCalledTimes(2)
await settleDownload()
})
it('shows a loading line while the archive is being built, and hides it after', async () => {
let release: (response: Response) => void = () => {}
fetchMock.mockImplementation(async () => await new Promise<Response>(resolve => { release = resolve }))
harness = await mountTree()
const busyLine = (): HTMLElement | null => harness.container.querySelector<HTMLElement>('[data-kind="loading"]')
// Nothing is shown before the request starts…
expect(busyLine()).toBeNull()
rightClick(rowByName(harness.container, 'sub'))
clickMenuitem('Zip and download')
// …the in-flight request keeps one visible, announced line…
const line = busyLine()
expect(line).not.toBeNull()
expect(line?.textContent).toBe('Loading…')
expect(line?.getAttribute('role')).toBe('status')
// …a repeated pick is still guarded (no second fetch) AND still not silent.
rightClick(rowByName(harness.container, 'sub'))
clickMenuitem('Zip and download')
expect(fetchMock).toHaveBeenCalledTimes(1)
expect(busyLine()).not.toBeNull()
await act(async () => { release(okResponse()) })
await settleDownload()
expect(busyLine()).toBeNull()
expect(downloads).toEqual(['blob:mock-1|sub.zip'])
})
it('hides the loading line when the archive fails too', async () => {
let release: (response: Response) => void = () => {}
fetchMock.mockImplementation(async () => await new Promise<Response>(resolve => { release = resolve }))
harness = await mountTree()
rightClick(rowByName(harness.container, 'sub'))
clickMenuitem('Zip and download')
expect(harness.container.querySelector('[data-kind="loading"]')).not.toBeNull()
await act(async () => { release(errorResponse(413, { ok: false, error: { message: 'too big' } })) })
await settleDownload()
expect(harness.container.querySelector('[data-kind="loading"]')).toBeNull()
// The busy line yields to the error strip: both never race for the slot.
expect(harness.container.querySelector('[role="alert"]')?.textContent)
.toContain('Archive failed: too big')
})
})
+30 -6
View File
@@ -162,11 +162,31 @@ describe('FileTree plugin open-with menu', () => {
'File Manager', 'VS Code', 'Cursor', 'Zed', 'Windsurf',
])
expect(submenuRows().every(item => item.querySelector('[class*="openWithPin"]') !== null)).toBe(true)
// Pinned state is announced per row (and swaps the pushpin glyph).
// Pinned state is carried per row by the pin's TITLE (it swaps the glyph
// and the hover hint).
const vscodeRow = submenuRows().find(item => item.textContent?.trim() === 'VS Code')
const cursorRow = submenuRows().find(item => item.textContent?.trim() === 'Cursor')
expect(vscodeRow?.querySelector('[aria-label="Unpin"]')).not.toBeNull()
expect(cursorRow?.querySelector('[aria-label="Pin to menu"]')).not.toBeNull()
expect(vscodeRow?.querySelector('[title="Unpin"]')).not.toBeNull()
expect(cursorRow?.querySelector('[title="Pin to menu"]')).not.toBeNull()
})
it('keeps the pin a mouse hot zone, never a fake nested control', async () => {
// The Menu renders each row as `<button role="menuitem">`, so ANY
// role=button/tabIndex on the pin would be invalid nesting AND
// unreachable by keyboard (the row button owns focus). It is deliberately
// a plain span: the pin's nameable affordance is its `title`, and the
// reachable path to the same pinned list is the settings panel.
harness = await mountTree()
openMenu(harness.container)
act(() => { submenuParent().click() })
const vscodePin = submenuRows().find(item => item.textContent?.trim() === 'VS Code')!
.querySelector<HTMLElement>('[class*="openWithPin"]')!
expect(vscodePin.tagName).toBe('SPAN')
expect(vscodePin.getAttribute('role')).toBeNull()
expect(vscodePin.hasAttribute('tabindex')).toBe(false)
expect(vscodePin.getAttribute('title')).toBe('Unpin')
// The row itself stays the only interactive element of the entry.
expect(vscodePin.closest('[role="menuitem"]')?.tagName).toBe('BUTTON')
})
it('pin click toggles without selecting the row or closing the menu', async () => {
@@ -195,16 +215,20 @@ describe('FileTree plugin open-with menu', () => {
expect(menuItems()).toHaveLength(0)
})
it('appends the SSH hint to VSCode-family labels in remote mode', async () => {
it('appends the SSH hint to VSCode-family labels in remote mode (reveal stays)', async () => {
// The SSH-resolved list keeps `explorer` (see resolveOpenWithTargets):
// reveal runs on the machine that owns the workspace, and it is the only
// reveal left when the DSH host's open-in-app capability is unavailable.
// Dropping it here would test a list the caller never produces.
harness = await mountTree({
openWithSsh: true,
openWithTargets: targets.filter(target => !target.localOnly),
openWithTargets: targets.filter(target => !target.localOnly || target.kind === 'reveal'),
openWithPinned: [],
})
openMenu(harness.container)
act(() => { submenuParent().click() })
expect(submenuRows().map(item => item.textContent?.trim())).toEqual([
'VS Code (SSH)', 'Cursor (SSH)', 'Windsurf (SSH)',
'File Manager', 'VS Code (SSH)', 'Cursor (SSH)', 'Windsurf (SSH)',
])
})
+123 -5
View File
@@ -17,7 +17,9 @@ import { act } from 'react-dom/test-utils'
import { GitLens } from '../src/client/changes/GitLens.tsx'
import { createSidebarStore } from '../src/client/state.ts'
import { api, type GitLogEntry, type GitStatusResult, type GitWorktree } from '../src/client/api.ts'
import type { BetterSidebarService } from '../src/client/service.ts'
import { t } from '../src/client/locales.ts'
import type { Context } from '../src/context-types.ts'
import { setupReactAct } from './test-utils.ts'
setupReactAct()
@@ -70,7 +72,12 @@ async function flushEffects(): Promise<void> {
* — the shared status store only polls/loads while a consumer is on screen. */
function mountGit(
root: Root,
options: { scope?: { sessionId: string; cwd?: string }; refreshTick?: number; visible?: boolean } = {},
options: {
scope?: { sessionId: string; cwd?: string }
refreshTick?: number
visible?: boolean
ctx?: Context
} = {},
): void {
act(() => {
root.render(createElement(GitLens, {
@@ -81,6 +88,7 @@ function mountGit(
selectedRef: null,
visible: options.visible ?? true,
refreshTick: options.refreshTick ?? 0,
...(options.ctx === undefined ? {} : { ctx: options.ctx }),
}))
})
}
@@ -538,10 +546,14 @@ describe('GitLens (changes tab, git lens) change tree', () => {
expect(unstaged.querySelector('[data-count]')?.textContent).toBe('2')
expect(staged.querySelector('[data-count]')?.textContent).toBe('1')
const stageAll = unstaged.querySelector<HTMLButtonElement>(`button[aria-label="${t('stageAll')}"]`)
const unstageAll = staged.querySelector<HTMLButtonElement>(`button[aria-label="${t('unstageAll')}"]`)
expect(stageAll).not.toBeNull()
expect(unstageAll).not.toBeNull()
// The band's own action is the one that is NOT inside a row (directory
// rows carry the same "Stage all" label for their own subtree).
const stageAll = [...unstaged.querySelectorAll<HTMLButtonElement>(`button[aria-label="${t('stageAll')}"]`)]
.find(button => button.closest('[data-row]') === null)
const unstageAll = [...staged.querySelectorAll<HTMLButtonElement>(`button[aria-label="${t('unstageAll')}"]`)]
.find(button => button.closest('[data-row]') === null)
expect(stageAll).not.toBeUndefined()
expect(unstageAll).not.toBeUndefined()
const stage = vi.spyOn(api, 'gitStage').mockResolvedValue({ ok: true })
await act(async () => { stageAll!.click() })
@@ -553,4 +565,110 @@ describe('GitLens (changes tab, git lens) change tree', () => {
container.remove()
}
})
it('stages and unstages a whole directory from its row action', async () => {
const { container, root } = makeRoot()
try {
await mountTree(container, root, [
{ path: 'src/deep/a.ts', xy: ' M' }, // unstaged → the 'src/deep' row
{ path: 'docs/b.md', xy: 'M ' }, // staged → the 'docs' row
])
const stage = vi.spyOn(api, 'gitStage').mockResolvedValue({ ok: true })
const unstage = vi.spyOn(api, 'gitUnstage').mockResolvedValue({ ok: true })
const dirAction = (path: string, label: string): HTMLButtonElement => {
const row = container.querySelector<HTMLElement>(`[data-row="${path}"]`)
expect(row).not.toBeNull()
const button = row!.querySelector<HTMLButtonElement>(`button[aria-label="${label}"]`)
expect(button).not.toBeNull()
return button!
}
const unstagedDir = dirAction('src/deep', t('stageAll'))
await act(async () => { unstagedDir.click() })
await flushEffects()
// The directory path stages the whole subtree (`git add -A -- <dir>`).
expect(stage).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'session' }), 'src/deep', MAIN)
const stagedDir = dirAction('docs', t('unstageAll'))
await act(async () => { stagedDir.click() })
await flushEffects()
expect(unstage).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'session' }), 'docs', MAIN)
// Neither click folded its row: the action is a sibling of the
// disclosure button (and stops propagation anyway).
expect(container.querySelector('button[data-dir="src/deep"]')!.getAttribute('aria-expanded')).toBe('true')
expect(container.querySelector('button[data-path="src/deep/a.ts"]')).not.toBeNull()
expect(container.querySelector('button[data-dir="docs"]')!.getAttribute('aria-expanded')).toBe('true')
} finally {
act(() => { root.unmount() })
container.remove()
}
})
it('resolves no icon for a status poll whose change list did not change', async () => {
vi.useFakeTimers()
const fileIcons: string[] = []
const folderIcons: string[] = []
/** The icon resolvers are the re-render probe: they run inside the rows,
* so one entry per call means one row actually re-rendered. */
const service = {
subscribe: () => () => {},
fileIcon: (path: string) => { fileIcons.push(path); return null },
folderIcon: (path: string, open: boolean) => { folderIcons.push(`${path}:${String(open)}`); return null },
} as unknown as BetterSidebarService
const ctx = { get: (name: string) => (name === 'betterSidebar' ? service : undefined) } as unknown as Context
const { container, root } = makeRoot()
try {
vi.spyOn(api, 'gitWorktrees').mockResolvedValue([{ path: MAIN, branch: 'main', current: true, changes: 2 }])
// Every poll answers with a fresh object and a fresh entries array (the
// store's real shape). The FIRST answer carries no repository list and
// the later ones do — a field OUTSIDE the change list moves, so the store
// cannot preserve the snapshot identity and hands the lens a new array
// whose content is equal. Only the CONTENT key plus the memoized rows
// hold there; identity alone would rebuild the tree and re-render rows.
let poll = 0
vi.spyOn(api, 'gitStatus').mockImplementation(async () => {
poll += 1
return {
isRepo: true,
branch: 'main',
...(poll === 1 ? {} : { repositories: [MAIN] }),
// One compressed directory row (folder icon) + its two file rows.
entries: [{ path: 'src/deep/a.ts', xy: ' M' }, { path: 'src/deep/b.ts', xy: ' M' }],
}
})
vi.spyOn(api, 'gitBranch').mockResolvedValue({ current: 'main', names: ['main'] })
vi.spyOn(api, 'gitLog').mockResolvedValue([])
mountGit(root, { ctx })
await flushEffects()
expect(fileIcons).toEqual(['src/deep/a.ts', 'src/deep/b.ts'])
expect(folderIcons).toEqual(['src/deep:true'])
fileIcons.length = 0
folderIcons.length = 0
// Tick 1: a NEW snapshot object with an equal-content change list.
await act(async () => { await vi.advanceTimersByTimeAsync(2_600) })
await flushEffects()
expect(fileIcons).toEqual([])
expect(folderIcons).toEqual([])
fileIcons.length = 0
folderIcons.length = 0
// Tick 2: a byte-identical answer (the store reuses the snapshot object).
await act(async () => { await vi.advanceTimersByTimeAsync(2_600) })
await flushEffects()
expect(fileIcons).toEqual([])
expect(folderIcons).toEqual([])
// The probe measured stability, not loss: every row is still there.
expect(container.querySelectorAll('[data-path]')).toHaveLength(2)
expect(container.querySelectorAll('button[data-dir="src/deep"]')).toHaveLength(1)
} finally {
act(() => { root.unmount() })
container.remove()
vi.useRealTimers()
}
})
})
+8 -2
View File
@@ -63,10 +63,16 @@ describe('resolveOpenWithTargets', () => {
expect(targets.map(target => target.id)).toEqual(['explorer', 'vscode', 'cursor', 'zed', 'custom:e1'])
})
it('drops local-only targets in SSH mode (file manager, Zed, non-VSCode custom)', () => {
it('drops the local-only URL openers in SSH mode but KEEPS reveal (file manager)', () => {
const config = parseOpenWithConfig({ sshHost: 'dev', customEditors: [localCustom, familyCustom] })
const targets = resolveOpenWithTargets(config)
expect(targets.map(target => target.id)).toEqual(['vscode', 'cursor', 'custom:e2'])
// Zed and the non-VSCode custom editor cannot reach a remote path. The
// `explorer` target stays: reveal runs on the host machine (the one that
// owns the workspace) and is the only reveal left when the DSH host's own
// open-in-app capability is unavailable.
expect(targets.map(target => target.id)).toEqual(['explorer', 'vscode', 'cursor', 'custom:e2'])
expect(targets[0]?.kind).toBe('reveal')
expect(targets.filter(target => target.kind === 'reveal')).toHaveLength(1)
})
it('prunes unknown pinned ids only when resolving (the parse keeps them)', () => {
+39
View File
@@ -163,6 +163,45 @@ describe('useGitStatus', () => {
await unmount()
})
it('keeps the snapshot identity when an idle poll returns the same answer', async () => {
// Without this, every 2.5s tick minted a new object and every downstream
// memo (the changes tree, the file tree rows) re-ran on an unchanged view.
// A FRESH object per call: the host really does re-serialize the answer
// every poll, so `mockResolvedValue` (same reference) would not prove this.
gitStatus.mockImplementation(async () => snapshot())
// Collect the snapshots the hook PUBLISHES (the view object itself is
// re-created per render, so identity must be read off the snapshot).
const published: (GitStatusResult | null)[] = []
let view: GitStatusView | undefined
const { unmount } = await mount(createElement(Probe, {
scope: { sessionId: 's-idle' },
onView: (next) => { view = next; published.push(next.snapshot) },
}))
expect(gitStatus).toHaveBeenCalledTimes(1)
const before = published[published.length - 1]
expect(before).not.toBeNull()
await act(async () => { view?.refresh(); await Promise.resolve(); await Promise.resolve() })
expect(gitStatus).toHaveBeenCalledTimes(2)
expect(published[published.length - 1]).toBe(before)
await unmount()
})
it('replaces the snapshot when the poll reports a real change', async () => {
gitStatus.mockResolvedValueOnce(snapshot())
const published: (GitStatusResult | null)[] = []
let view: GitStatusView | undefined
const { unmount } = await mount(createElement(Probe, {
scope: { sessionId: 's-change' },
onView: (next) => { view = next; published.push(next.snapshot) },
}))
const before = published[published.length - 1]
gitStatus.mockResolvedValue(snapshot({ entries: [{ path: 'src/a.ts', xy: ' M' }, { path: 'src/b.ts', xy: '??' }] }))
await act(async () => { view?.refresh(); await Promise.resolve(); await Promise.resolve() })
expect(published[published.length - 1]).not.toBe(before)
expect(view?.statusOf('/ws/src/b.ts')?.tone).toBe('untracked')
await unmount()
})
it('invalidateGitStatus re-reads every live key of one session', async () => {
gitStatus.mockResolvedValue(snapshot())
const { unmount } = await mount(createElement(Probe, { scope: { sessionId: 's6' }, onView: () => {} }))
+174 -5
View File
@@ -14,12 +14,13 @@
import { afterEach, describe, expect, it } from 'vitest'
import { spawnSync } from 'node:child_process'
import { mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'
import { validateHeaderValue } from 'node:http'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { inflateRawSync } from 'node:zlib'
import { apply } from '../src/index.ts'
import { archiveUrl } from '../src/client/api.ts'
import { archiveNameOf, collectZipEntries } from '../src/archive-route.ts'
import { archiveNameOf, collectZipEntries, contentDispositionOf, disambiguateArchiveNames } from '../src/archive-route.ts'
import { crc32, buildZip, type ZipEntry } from '../src/zip.ts'
import { SidebarError } from '../src/wire.ts'
import type { SidebarWebRoute, SidebarWebUpgradeRoute } from '../src/context-types.ts'
@@ -224,6 +225,52 @@ describe('buildZip', () => {
.rejects.toMatchObject({ code: 'fs-error' })
})
it('enforces the byte bound BEFORE reading the payload (stat first)', async () => {
const root = tempRoot()
const huge = join(root, 'huge.bin')
writeFileSync(huge, Buffer.alloc(4 * 1024 * 1024, 7))
// A source far past the budget is refused on its stat, with the archive's
// own ceiling in the message (never an internal budget number)…
await expect(buildZip([{ path: huge, name: 'huge.bin' }], { maxBytes: 1024 }))
.rejects.toThrow(/exceeds the 1024 byte limit/)
await expect(buildZip([{ path: huge, name: 'huge.bin' }], { maxBytes: 1024 }))
.rejects.toMatchObject({ code: 'fs-error' })
// maxBytes: 0 refuses even a one-byte source.
const one = join(root, 'one.txt')
writeFileSync(one, 'x')
await expect(buildZip([{ path: one, name: 'one.txt' }], { maxBytes: 0 }))
.rejects.toThrow(/exceeds the 0 byte limit/)
// …and a source that exactly fills the budget is accepted.
const two = join(root, 'two.txt')
writeFileSync(two, 'xy')
expect(readZip(await buildZip([{ path: two, name: 't.txt' }], { maxBytes: 2 }))[0]!.size).toBe(2)
// The budget is the REMAINING allowance: the second entry no longer fits
// (an exhausted budget refuses even a zero-byte source, so a FIFO can
// never be opened — see the probe below).
const empty = join(root, 'empty.txt')
writeFileSync(empty, '')
await expect(buildZip([{ path: two, name: 'a.txt' }, { path: empty, name: 'b.txt' }], { maxBytes: 2 }))
.rejects.toMatchObject({ code: 'fs-error' })
})
it('never opens a source the size probe already refused (FIFO)', async () => {
if (process.platform === 'win32') return // no FIFOs there
const root = tempRoot()
const fifo = join(root, 'pipe')
const made = spawnSync('mkfifo', [fifo], { encoding: 'utf8' })
if (made.error !== undefined || made.status !== 0) return // binary absent: skip
// A 2-byte file consumes the whole 2-byte budget, so the FIFO's remaining
// allowance is 0. Reading an unwritten FIFO BLOCKS FOREVER, so this test
// can only complete when the size probe refuses it before open() — with the
// read-then-check order it hangs and vitest fails it on timeout instead.
// (This is the discriminating guard for F3: a "rejects quickly" timing
// probe does NOT discriminate, because the reverted code also rejects.)
const two = join(root, 'two.txt')
writeFileSync(two, 'xy')
await expect(buildZip([{ path: two, name: 'a.txt' }, { path: fifo, name: 'p' }], { maxBytes: 2 }))
.rejects.toThrow(/exceeds the 2 byte limit/)
}, 5_000)
it('reports an unreadable source as fs-error and refuses traversal names', async () => {
const root = tempRoot()
await expect(buildZip([{ path: join(root, 'missing.txt'), name: 'missing.txt' }]))
@@ -232,6 +279,22 @@ describe('buildZip', () => {
.rejects.toMatchObject({ code: 'bad-request' })
})
it('keeps a backslash as part of the file name (POSIX names are literal)', async () => {
const root = tempRoot()
// `a\b.txt` is ONE legal POSIX file name; splitting it into a directory
// would silently rename the member (F8).
const path = join(root, 'a\\b.txt')
writeFileSync(path, 'literal\n')
const archive = await buildZip([{ path, name: 'a\\b.txt' }])
const entries = readZip(archive)
expect(entries).toHaveLength(1)
expect(entries[0]!.name).toBe('a\\b.txt')
expect(entries[0]!.data.toString('utf8')).toBe('literal\n')
// A traversal name is still refused, and so is a bare '..'.
await expect(buildZip([{ path, name: 'dir/../../x' }])).rejects.toMatchObject({ code: 'bad-request' })
await expect(buildZip([{ path, name: '..' }])).rejects.toMatchObject({ code: 'bad-request' })
})
it('passes `unzip -t` when the binary exists (skipped otherwise)', async () => {
const probe = spawnSync('unzip', ['-v'], { encoding: 'utf8' })
if (probe.error !== undefined || probe.status !== 0) return
@@ -307,6 +370,52 @@ describe('archiveNameOf', () => {
})
})
describe('contentDispositionOf', () => {
it('never puts a non-latin1 character in the header value', () => {
const header = contentDispositionOf('报告.zip')
// Node's writeHead rejects any header value above U+00FF — the whole
// reason the raw name cannot be used (it 500s the route). Control: the
// exact shape this fix replaced must FAIL the same check.
expect(() => validateHeaderValue('content-disposition', 'attachment; filename="报告.zip"'))
.toThrow(/Invalid character in header content/)
expect([...header].every(char => char.codePointAt(0)! <= 0xFF)).toBe(true)
expect(validateHeaderValue('content-disposition', header)).toBeUndefined()
expect(header).toBe(`attachment; filename="download.zip"; filename*=UTF-8''${encodeURIComponent('报告.zip')}`)
})
it('keeps a plain ASCII name in both halves (suffix stripped for the fallback stem only)', () => {
const header = contentDispositionOf('docs.zip')
expect(header).toBe(`attachment; filename="docs.zip"; filename*=UTF-8''docs.zip`)
expect(validateHeaderValue('content-disposition', header)).toBeUndefined()
// A mixed name keeps whatever ASCII survives in the fallback.
expect(contentDispositionOf('報告 Q3.zip')).toContain('filename=" Q3.zip"')
})
})
describe('disambiguateArchiveNames', () => {
it('keeps the basename when nothing collides', () => {
expect(disambiguateArchiveNames(['/ws/src', '/ws/notes.md'])).toEqual(['src', 'notes.md'])
})
it('prepends parent directories only for the colliding selections', () => {
expect(disambiguateArchiveNames(['/ws/a/index.ts', '/ws/b/index.ts']))
.toEqual(['a/index.ts', 'b/index.ts'])
// The unaffected selection keeps its short name.
expect(disambiguateArchiveNames(['/ws/a/index.ts', '/ws/notes.md']))
.toEqual(['index.ts', 'notes.md'])
})
it('walks further up while ancestors still collide', () => {
// Both are `src/index.ts` relative to their own parent: one more level.
expect(disambiguateArchiveNames(['/ws/a/src/index.ts', '/ws/b/src/index.ts']))
.toEqual(['a/src/index.ts', 'b/src/index.ts'])
})
it('falls back to the full path when no ancestor can separate the names', () => {
expect(disambiguateArchiveNames(['/ws/a.txt', '/ws/a.txt'])).toEqual(['ws/a.txt', 'ws/a.txt'])
})
})
describe('/sidebar/archive route', () => {
it('archiveUrl builds the GET URL the route parses (repeated path, encoded)', () => {
const url = archiveUrl({ sessionId: 's-1', cwd: '/work tree' }, ['/work tree/a b.txt', '/work tree/子目录'], 'my zip.zip')
@@ -335,7 +444,10 @@ describe('/sidebar/archive route', () => {
const response = await get(route, url)
expect(response.status).toBe(200)
expect(response.headers['content-type']).toBe('application/zip')
expect(response.headers['content-disposition']).toBe('attachment; filename="我的打包.zip"')
// F1: the name is never header-raw. See the real-HTTP case below for the
// node:http proof that a non-latin1 name no longer 500s.
expect(response.headers['content-disposition'])
.toBe(`attachment; filename="download.zip"; filename*=UTF-8''${encodeURIComponent('我的打包.zip')}`)
expect(Number(response.headers['content-length'])).toBe(response.body.byteLength)
const entries = readZip(response.body)
expect(entries.map(entry => entry.name)).toEqual([
@@ -350,6 +462,63 @@ describe('/sidebar/archive route', () => {
expect(byName.get('notes.md')!.data.toString('utf8')).toBe('# notes\n')
})
it('serves a non-latin1 download name through REAL node:http (no 500, RFC 5987)', async () => {
const root = tempRoot()
const workspace = join(root, 'workspace')
mkdirSync(join(workspace, '报告'), { recursive: true })
writeFileSync(join(workspace, '报告', '数据.txt'), 'x\n')
const route = mountArchive()
const name = '报告.zip'
const url = `/sidebar/archive?sessionId=s-zip&cwd=${encodeURIComponent(workspace)}`
+ `&name=${encodeURIComponent(name)}`
+ `&path=${encodeURIComponent(join(workspace, '报告'))}`
// The route handler answers a REAL ServerResponse here: node:http's
// writeHead validates header values, which the plain-object stub in the
// other cases cannot do — that is exactly how the Chinese-name 500 hid.
const { createServer } = await import('node:http')
const server = createServer((req, res) => { void route.handler(req as never, res as never) })
await new Promise<void>(resolve => server.listen(0, '127.0.0.1', resolve))
try {
const address = server.address()
if (address === null || typeof address === 'string') throw new Error('test setup: no server port')
const response = await fetch(`http://127.0.0.1:${address.port}${url}`)
expect(response.status).toBe(200)
const disposition = response.headers.get('content-disposition') ?? ''
// The ASCII half is what old clients read; the RFC 5987 half carries the
// real name and is what every current browser picks.
expect(disposition).toContain(`filename*=UTF-8''${encodeURIComponent(name)}`)
expect(disposition).toContain('filename="download.zip"')
// The header VALUE must be latin1-clean (Node's own rule).
expect([...disposition].every(char => char.codePointAt(0)! <= 0xFF)).toBe(true)
const entries = readZip(Buffer.from(await response.arrayBuffer()))
expect(entries.map(entry => entry.name)).toEqual(['报告/', '报告/数据.txt'])
} finally {
await new Promise<void>(resolve => server.close(() => resolve()))
}
})
it('disambiguates same-name selections with their parent directories', async () => {
const root = tempRoot()
const workspace = join(root, 'workspace')
mkdirSync(join(workspace, 'a'), { recursive: true })
mkdirSync(join(workspace, 'b'), { recursive: true })
writeFileSync(join(workspace, 'a', 'index.ts'), 'from a\n')
writeFileSync(join(workspace, 'b', 'index.ts'), 'from b\n')
const route = mountArchive()
const url = `/sidebar/archive?sessionId=s-zip&cwd=${encodeURIComponent(workspace)}`
+ `&path=${encodeURIComponent(join(workspace, 'a', 'index.ts'))}`
+ `&path=${encodeURIComponent(join(workspace, 'b', 'index.ts'))}`
const response = await get(route, url)
expect(response.status).toBe(200)
const entries = readZip(response.body)
const names = entries.map(entry => entry.name)
expect(new Set(names).size).toBe(names.length)
expect(names).toEqual(['a/index.ts', 'b/index.ts'])
const byName = new Map(entries.map(entry => [entry.name, entry]))
expect(byName.get('a/index.ts')!.data.toString('utf8')).toBe('from a\n')
expect(byName.get('b/index.ts')!.data.toString('utf8')).toBe('from b\n')
})
it('never follows a symlink out of the workspace', async () => {
const root = tempRoot()
const workspace = join(root, 'workspace')
@@ -415,11 +584,11 @@ describe('/sidebar/archive route', () => {
const path = encodeURIComponent(join(workspace, 'a.txt'))
const scope = `sessionId=s-zip&cwd=${encodeURIComponent(workspace)}&path=${path}`
const traversal = await get(route, `/sidebar/archive?${scope}&name=${encodeURIComponent('../evil")b.zip')}`)
expect(traversal.headers['content-disposition']).toBe('attachment; filename="evil)b.zip"')
expect(traversal.headers['content-disposition']).toContain('filename="evil)b.zip"')
const bare = await get(route, `/sidebar/archive?${scope}&name=docs`)
expect(bare.headers['content-disposition']).toBe('attachment; filename="docs.zip"')
expect(bare.headers['content-disposition']).toContain('filename="docs.zip"')
const fallback = await get(route, `/sidebar/archive?${scope}`)
expect(fallback.headers['content-disposition']).toBe('attachment; filename="archive.zip"')
expect(fallback.headers['content-disposition']).toContain('filename="archive.zip"')
})
it('rejects a non-GET method', async () => {