fix: address Copilot review (9 comments)
#1 GitView: hide/disable 'open in editor' for external linked worktrees (path outside session workspace is rejected by the fence); add isWithinWorkspace client helper for the gate. #2 DiffTab: untracked diff read resolves against diff.repoRoot (child-repo paths are repo-root-relative, not cwd-relative); fs.read route threads selectedRepoOf into resolveGitPath so the host resolves the right base. #3 GitView: chooseRepo handler mirrors chooseWorktree — clears status/ history/log and bumps generation before async refresh, preventing stale rows from acting on the newly selected repository. #4 git.ts: repoRoots/repoRoot compare roots via pathIdentity() (platform- aware) instead of raw includes(), fixing win32 forward/backslash mismatch that silently fell back to roots[0]. #5 markdown-images: mask fenced code blocks and inline code spans before rewriting; only rewrite reference definitions whose label is actually referenced by image syntax (not plain links). +3 tests. #6 package.json: downgrade unrun ^0.3.1 → ^0.2.39 (0.3.x requires Node ^22.13, incompatible with declared engines >=20). #7 market-manifest.spec: shell:true on win32 so execFileSync resolves the pnpm .cmd shim. #8 smoke.spec: build html-route URL via encodeHtmlUrl() (production encoder) so the Windows drive separator is correct. #9 index.ts git.worktrees route: thread selectedRepoOf so a workspace container's selected child repo's linked worktrees appear.
This commit is contained in:
@@ -61,7 +61,10 @@ export function DiffTab(props: { sessionId: string; cwd: string | undefined; dif
|
||||
// Empty diff: an untracked file (git diff never lists it) falls back
|
||||
// to a full-file addition; anything else is a genuine no-text-change.
|
||||
if (diff.untracked === true && !diff.staged) {
|
||||
const text = await api.fsRead(scope, resolveSidebarPath(diff.worktree ?? cwd, diff.path))
|
||||
// A child-repo path is relative to diff.repoRoot, not the session
|
||||
// cwd or the linked-worktree root; resolve against whichever the
|
||||
// diff ref carries so the untracked fallback reads the right file.
|
||||
const text = await api.fsRead(scope, resolveSidebarPath(diff.repoRoot ?? diff.worktree ?? cwd, diff.path))
|
||||
if (!cancelled) {
|
||||
setData(text.kind === 'text' ? { diff: '', untracked: text.content } : { diff: '' })
|
||||
}
|
||||
|
||||
+33
-4
@@ -16,7 +16,7 @@ import {
|
||||
} from '@deepseek-ai/dsh-client-ui-primitives'
|
||||
import type { GitLogEntry, GitStatusEntry, GitStatusResult, GitWorktree, SessionScope } from './api.ts'
|
||||
import { api } from './api.ts'
|
||||
import { relativeTo } from './paths.ts'
|
||||
import { isWithinWorkspace, relativeTo } from './paths.ts'
|
||||
import { resolveSidebarPath } from './produced-files.ts'
|
||||
import { relativeTime, t } from './locales.ts'
|
||||
import type { SidebarTab } from './state.ts'
|
||||
@@ -231,6 +231,23 @@ export function GitView(props: {
|
||||
const generation = refreshGeneration.current += 1
|
||||
void refreshTarget(target, { loading: true, generation })
|
||||
}
|
||||
/** Switching the selected child repository must invalidate every
|
||||
* target-derived surface (status/history/log) before the asynchronous
|
||||
* refresh resolves; otherwise stale rows remain actionable while their
|
||||
* handlers already address the new repository. Mirrors chooseWorktree. */
|
||||
const chooseRepo = (target: string): void => {
|
||||
setRepoRoot(target)
|
||||
setStatus(null)
|
||||
setBranchNames([])
|
||||
setLogEntries([])
|
||||
setLogEnded(false)
|
||||
setLogLoadingMore(false)
|
||||
// Re-list worktrees for the selected child (a workspace container's
|
||||
// own worktree list is empty); keep the current linked-checkout choice
|
||||
// unless it does not belong to the new repository.
|
||||
const generation = refreshGeneration.current += 1
|
||||
void refreshTarget(selectedRef.current ?? '', { loading: true, generation })
|
||||
}
|
||||
useEffect(() => {
|
||||
if (!visible) return
|
||||
const timer = window.setInterval(() => { void refresh(true) }, 2_000)
|
||||
@@ -420,7 +437,7 @@ export function GitView(props: {
|
||||
className={css.gitBranchSelect}
|
||||
value={repoRoot ?? ''}
|
||||
title={repoRoot}
|
||||
onChange={(event) => { setRepoRoot(event.target.value) }}
|
||||
onChange={(event) => { chooseRepo(event.target.value) }}
|
||||
disabled={busy}
|
||||
>
|
||||
{status!.repositories!.map(root => <option key={root} value={root}>{baseName(root)}</option>)}
|
||||
@@ -551,7 +568,13 @@ export function GitView(props: {
|
||||
open={fileMenu !== null}
|
||||
onClose={() => { setFileMenu(null) }}
|
||||
items={[
|
||||
{ id: 'open', label: t('openEditor'), icon: <IconCodeOutline16 size={14} /> },
|
||||
// A linked worktree outside the session workspace cannot be
|
||||
// opened in the editor: the host's workspace fence rejects
|
||||
// every path under it. Hide the action for that checkout so
|
||||
// the menu does not offer a no-op that confuses the user.
|
||||
...(fileMenu !== null && isWithinWorkspace(scope.cwd ?? '', resolveSidebarPath(repoRoot ?? selectedWorktree ?? scope.cwd, fileMenu.entry.path))
|
||||
? [{ id: 'open', label: t('openEditor'), icon: <IconCodeOutline16 size={14} /> }]
|
||||
: []),
|
||||
fileMenu?.staged === true
|
||||
? { id: 'stage', label: t('unstage'), icon: <IconTrashOutline16 size={14} /> }
|
||||
: { id: 'stage', label: t('stage'), icon: <IconBranchOutline16 size={14} /> },
|
||||
@@ -567,7 +590,13 @@ export function GitView(props: {
|
||||
if (target === null) return
|
||||
setFileMenu(null)
|
||||
if (id === 'open') {
|
||||
onOpenFile(resolveSidebarPath(repoRoot ?? selectedWorktree ?? scope.cwd, target.entry.path))
|
||||
const resolved = resolveSidebarPath(repoRoot ?? selectedWorktree ?? scope.cwd, target.entry.path)
|
||||
// Defense-in-depth: the menu hides this action when the
|
||||
// resolved path escapes the session workspace, but a
|
||||
// racing repo switch could still reach here with a path
|
||||
// the host would reject. No-op in that case.
|
||||
if (!isWithinWorkspace(scope.cwd ?? '', resolved)) return
|
||||
onOpenFile(resolved)
|
||||
return
|
||||
}
|
||||
if (id === 'stage') {
|
||||
|
||||
@@ -50,6 +50,14 @@ function normalizeLocalPath(path: string): string {
|
||||
* local paths pass through. Remote (http/https/data/mailto) and `#`-anchor
|
||||
* destinations are left untouched for `MarkdownText`. Reference-style images
|
||||
* (`![x][id]` + `[id]: url`) are covered by rewriting their definition lines.
|
||||
*
|
||||
* Code spans (`` `...` ``) and fenced code blocks (``` ```...``` ```) are
|
||||
* masked before rewriting so documentation that demonstrates ``
|
||||
* is not mutated into a `/sidebar/file` URL. Reference definitions are only
|
||||
* rewritten when their label is actually referenced by an image (collapsed
|
||||
* `[![][id]]`, full `![alt][id]`, or shortcut `![]` referencing the next
|
||||
* definition) — a plain link `[text][id]` must not have its destination
|
||||
* redirected to the media route.
|
||||
* @param text - The raw markdown source (inline + reference images).
|
||||
* @param scope - The session scope (sessionId + cwd) for the media route.
|
||||
* @param filePath - The absolute path of the opened `.md` file.
|
||||
@@ -79,11 +87,38 @@ export function rewriteLocalImageUrls(
|
||||
const candidate = isAbsolutePath(trimmed) ? trimmed : directory + trimmed
|
||||
return mediaUrl(normalizeLocalPath(candidate))
|
||||
}
|
||||
const inline = text.replace(/!\[([^\]]*)\]\(([^)\s]+)(?:\s+"[^"]*")?\)/g, (_match, alt, dest) => {
|
||||
|
||||
// Mask fenced code blocks and inline code spans so image-looking text
|
||||
// inside documentation examples is never rewritten. The sentinel uses a
|
||||
// character unlikely to appear in prose; the original spans are restored
|
||||
// after the image rewrite.
|
||||
const masks: string[] = []
|
||||
const masked = text
|
||||
.replace(/```[\s\S]*?```/g, (block) => { masks.push(block); return `\u0000${masks.length - 1}\u0000` })
|
||||
.replace(/`[^`\n]*`/g, (span) => { masks.push(span); return `\u0000${masks.length - 1}\u0000` })
|
||||
|
||||
const inline = masked.replace(/!\[([^\]]*)\]\(([^)\s]+)(?:\s+"[^"]*")?\)/g, (_match, alt, dest) => {
|
||||
return `})`
|
||||
})
|
||||
// Reference-style images: rewrite their `[id]: <url>` definitions.
|
||||
return inline.replace(/^(\s*\[[^\]]+\]:\s*)(<[^>]+>|[^\s]+)/gm, (_match, head, dest) => {
|
||||
|
||||
// Collect labels referenced by image syntax (full `![alt][id]` and
|
||||
// collapsed `![][id]`) so only those reference definitions are rewritten.
|
||||
// A shortcut reference (`![alt]` with no `[id]`) resolves to the label
|
||||
// text `alt` itself; include it too. Plain links `[text][id]` never match
|
||||
// the leading `!` and are left untouched.
|
||||
const imageLabels = new Set<string>()
|
||||
const labelRe = /!\[([^\]]*)\](?:\[((?:[^\][]|\[[^\]]*\])*)\])?/g
|
||||
let labelMatch: RegExpExecArray | null
|
||||
while ((labelMatch = labelRe.exec(inline)) !== null) {
|
||||
const alt = labelMatch[1] ?? ''
|
||||
const ref = labelMatch[2]
|
||||
imageLabels.add(ref !== undefined && ref !== '' ? ref.toLowerCase() : alt.toLowerCase())
|
||||
}
|
||||
|
||||
const refsRewritten = inline.replace(/^(\s*\[([^\]]+)\]:\s*)(<[^>]+>|[^\s]+)/gm, (match, head: string, label: string, dest: string) => {
|
||||
if (!imageLabels.has(label.toLowerCase())) return match
|
||||
return `${head}${resolve(dest.replace(/^<|>$/g, ''))}`
|
||||
})
|
||||
|
||||
return refsRewritten.replace(/\u0000(\d+)\u0000/g, (_m, index: string) => masks[Number(index)] ?? '')
|
||||
}
|
||||
|
||||
@@ -45,3 +45,19 @@ export function relativeTo(cwd: string, path: string): string {
|
||||
if (nPath.toLowerCase().startsWith(`${nBase.toLowerCase()}/`)) return nPath.slice(nBase.length + 1)
|
||||
return path
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `target` lies under `base` (or equals it), tolerant of separator
|
||||
* style and — on Windows-style drive paths — of letter case. A client-side
|
||||
* mirror of the host's `isWithin` (fs-tree.ts) used to decide whether a
|
||||
* git-derived path can be opened in the editor (a linked worktree outside
|
||||
* the session workspace cannot: the host's workspace fence would reject it).
|
||||
*/
|
||||
export function isWithinWorkspace(base: string, target: string): boolean {
|
||||
const norm = (value: string): string => value.replace(/[\\/]+/g, '/').replace(/\/$/, '')
|
||||
const b = norm(base)
|
||||
const t = norm(target)
|
||||
const lb = b.toLowerCase()
|
||||
const lt = t.toLowerCase()
|
||||
return lt === lb || lt.startsWith(`${lb}/`)
|
||||
}
|
||||
|
||||
+8
-2
@@ -212,7 +212,7 @@ export async function repoRoots(cwd: string): Promise<string[]> {
|
||||
.sort((left, right) => left.name.localeCompare(right.name))) {
|
||||
try {
|
||||
const root = await directRepoRoot(join(cwd, entry.name))
|
||||
if (!roots.includes(root)) roots.push(root)
|
||||
if (!roots.some(existing => pathIdentity(existing) === pathIdentity(root))) roots.push(root)
|
||||
} catch {
|
||||
// Ordinary child directory; keep discovering sibling repositories.
|
||||
}
|
||||
@@ -225,7 +225,13 @@ export async function repoRoots(cwd: string): Promise<string[]> {
|
||||
export async function repoRoot(cwd: string, selected?: string): Promise<string> {
|
||||
const roots = await repoRoots(cwd)
|
||||
if (roots.length === 0) throw new GitCommandError('not a git repository', 'not-repo', 'rev-parse')
|
||||
if (selected !== undefined && roots.includes(selected)) return selected
|
||||
// Git for Windows may return forward-slash roots while callers pass
|
||||
// backslashes (or vice-versa); compare via the platform-aware identity.
|
||||
if (selected !== undefined) {
|
||||
const identity = pathIdentity(selected)
|
||||
const match = roots.find(root => pathIdentity(root) === identity)
|
||||
if (match !== undefined) return match
|
||||
}
|
||||
return roots[0]!
|
||||
}
|
||||
|
||||
|
||||
+12
-4
@@ -286,8 +286,11 @@ function buildApi(
|
||||
'fs.read': async (payload) => {
|
||||
const { cwd } = cwdOf(payload)
|
||||
// Relative paths are git-derived (status/diff report repo-root-relative
|
||||
// names; the untracked diff view reads the file through this route).
|
||||
const path = await ensureWorkspacePath(cwd, await resolveGitPath(cwd, requireString(payload, 'path')))
|
||||
// names; the untracked diff view reads the file through this route). A
|
||||
// child-repo path is relative to the selected repoRoot, not the session
|
||||
// cwd; thread it so the path resolves inside the authorized workspace.
|
||||
const selected = selectedRepoOf(payload)
|
||||
const path = await ensureWorkspacePath(cwd, await resolveGitPath(cwd, requireString(payload, 'path'), selected))
|
||||
const { content, truncated, binary, size, head } = await readText(path, resolved.readLimit)
|
||||
if (binary) return { kind: 'binary', size, truncated, head }
|
||||
return { kind: 'text', content, truncated }
|
||||
@@ -308,8 +311,13 @@ function buildApi(
|
||||
return { ok: true }
|
||||
},
|
||||
'git.worktrees': async (payload) => {
|
||||
const { cwd } = cwdOf(payload)
|
||||
return git.worktrees(cwd)
|
||||
const { cwd } = await gitCwdOf(payload)
|
||||
const selected = selectedRepoOf(payload)
|
||||
// A workspace container (no repo at cwd) has child repos; the worktree
|
||||
// list belongs to the SELECTED child, not the container. Thread the
|
||||
// validated repoRoot so linked checkouts of a chosen child appear.
|
||||
const base = selected !== undefined ? await git.repoRoot(cwd, selected).catch(() => cwd) : cwd
|
||||
return git.worktrees(base)
|
||||
},
|
||||
'git.status': async (payload) => {
|
||||
const { cwd } = await gitCwdOf(payload)
|
||||
|
||||
Reference in New Issue
Block a user