mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix(desktop): anchor output chips and previews on real changed files
Fix four root causes in the desktop preview pipeline, surfaced when the model writes the files the user pointed it at: - Output chips guessed paths from prose and could point at a missing file. They are now reconciled against the turn's real changed files: a bare `index.html` resolves to the `todo-app/index.html` actually written, and mentions the turn never changed are dropped. - A standalone single-page index.html got no browser preview (mistaken for a Vite template). It is now only routed to the source view when a package.json/vite.config ships in the same change-set. - Files written outside the session workdir (another folder, or another drive on Windows) failed to preview with 'Path is outside workspace'. The turn's changed-file directories are registered as filesystem access roots; html serves via /local-file and other files via a workdir-relaxed read. - The visual-selection prompt leaked as a raw bubble on Windows because the server-appended '[Image source: ...]' line broke replay dedupe. Replay text is now metadata-normalized before comparison (affects any image message). Adds unit tests for each: htmlPreviewPolicy, assistantOutputTargets reconciliation, replay dedupe + stripGeneratedImageMetadataLines, filesystem access roots, and workspace outside-workdir reads.
This commit is contained in:
parent
6818db34fb
commit
fc27397205
@ -19,11 +19,14 @@ type Props = {
|
||||
branchAction?: MessageBranchAction
|
||||
sessionId?: string
|
||||
timestamp?: number
|
||||
/** This turn's real changed files (absolute), used to anchor output chips onto
|
||||
* files that were actually written instead of guessing from the prose. */
|
||||
turnChangedFiles?: string[]
|
||||
}
|
||||
|
||||
const MAX_CARDS = 3
|
||||
|
||||
export const AssistantMessage = memo(function AssistantMessage({ content, isStreaming, branchAction, sessionId, timestamp }: Props) {
|
||||
export const AssistantMessage = memo(function AssistantMessage({ content, isStreaming, branchAction, sessionId, timestamp, turnChangedFiles }: Props) {
|
||||
const t = useTranslation()
|
||||
const workDir = useWorkspacePanelStore((s) => (sessionId ? s.statusBySession[sessionId]?.workDir : undefined))
|
||||
|
||||
@ -53,10 +56,10 @@ export const AssistantMessage = memo(function AssistantMessage({ content, isStre
|
||||
isStreaming || !sessionId
|
||||
? []
|
||||
: // Image/video targets render inline (InlineImageGallery/InlineVideoGallery); never also as a card.
|
||||
extractAssistantOutputTargets(content, { workDir }).filter(
|
||||
extractAssistantOutputTargets(content, { workDir, changedFiles: turnChangedFiles }).filter(
|
||||
(target) => target.kind !== 'image' && target.kind !== 'video',
|
||||
),
|
||||
[content, isStreaming, sessionId, workDir],
|
||||
[content, isStreaming, sessionId, workDir, turnChangedFiles],
|
||||
)
|
||||
|
||||
if (!content.trim()) return null
|
||||
|
||||
@ -84,6 +84,7 @@ vi.mock('../../i18n', () => ({
|
||||
// Import after mocks
|
||||
// ──────────────────────────────────────────────────────────────────────────────
|
||||
import { CurrentTurnChangeCard } from './CurrentTurnChangeCard'
|
||||
import { localFileUrl } from '../../lib/handlePreviewLink'
|
||||
import type { SessionTurnCheckpoint } from '../../api/sessions'
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────────────
|
||||
@ -201,6 +202,24 @@ describe('CurrentTurnChangeCard – row opens the workspace diff', () => {
|
||||
expect(openPreviewSpy).toHaveBeenCalledWith('s1', 'README.md', 'diff')
|
||||
})
|
||||
|
||||
it('clicking an outside-workspace html changed file opens the in-app browser via local-file', () => {
|
||||
// The file lives outside the workdir (absolute displayPath) — no diff baseline,
|
||||
// so html renders directly in the in-app browser via the /local-file route.
|
||||
renderCard(['/other/place/todo.html'])
|
||||
const row = screen.getByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })
|
||||
fireEvent.click(row)
|
||||
expect(browserOpenSpy).toHaveBeenCalledWith('s1', localFileUrl('http://127.0.0.1:4321', '/other/place/todo.html'))
|
||||
expect(openPreviewSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('clicking an outside-workspace non-html changed file opens a file preview (not a diff)', () => {
|
||||
renderCard(['/other/place/notes.txt'])
|
||||
const row = screen.getByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })
|
||||
fireEvent.click(row)
|
||||
expect(openPreviewSpy).toHaveBeenCalledWith('s1', '/other/place/notes.txt', 'file')
|
||||
expect(browserOpenSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does NOT render an inline diff surface after clicking a row', () => {
|
||||
renderCard(['/w/proj/src/main.ts'])
|
||||
const row = screen.getByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })
|
||||
@ -280,7 +299,9 @@ describe('CurrentTurnChangeCard – open-with buttons', () => {
|
||||
expect(openPreviewSpy).toHaveBeenCalledWith('s1', 'README.md', 'file')
|
||||
})
|
||||
|
||||
it('clicking project index.html open-with opens menu with workspace preview item', async () => {
|
||||
it('clicking a standalone index.html (no manifest in change-set) offers both workspace preview and in-app browser', async () => {
|
||||
// A hand-authored single-page index.html is statically previewable, so the
|
||||
// menu offers the in-app browser alongside the workspace source view.
|
||||
renderCard(['/w/proj/index.html'])
|
||||
const [openWithBtn] = screen.getAllByRole('button', { name: 'openWith.title' })
|
||||
|
||||
@ -288,6 +309,21 @@ describe('CurrentTurnChangeCard – open-with buttons', () => {
|
||||
fireEvent.click(openWithBtn!)
|
||||
})
|
||||
|
||||
expect(await screen.findByText('openWith.workspacePreview')).toBeInTheDocument()
|
||||
expect(screen.queryByText('openWith.inAppBrowser')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('clicking a framework-template index.html (manifest in same change-set) hides the in-app browser', async () => {
|
||||
// With a package.json in the same turn, the root index.html is a build
|
||||
// template that needs a dev server — static preview would render blank — so
|
||||
// only the workspace source view is offered.
|
||||
renderCard(['/w/proj/index.html', '/w/proj/package.json', '/w/proj/vite.config.ts'])
|
||||
const [openWithBtn] = screen.getAllByRole('button', { name: 'openWith.title' })
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(openWithBtn!)
|
||||
})
|
||||
|
||||
expect(await screen.findByText('openWith.workspacePreview')).toBeInTheDocument()
|
||||
expect(screen.queryByText('openWith.inAppBrowser')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
@ -6,6 +6,8 @@ import { useTranslation, type TranslationKey } from '../../i18n'
|
||||
import { OpenWithMenu } from '../common/OpenWithMenu'
|
||||
import { buildOpenWithItems, describeFileType, isPreviewableChangedFile, type OpenWithItem } from '../../lib/openWithItems'
|
||||
import { openWithContextForWorkspaceFile } from '../../lib/openWithContextForHref'
|
||||
import { isAbsoluteLocalPath, localFileUrl } from '../../lib/handlePreviewLink'
|
||||
import { shouldOfferStaticHtmlPreview } from '../../lib/htmlPreviewPolicy'
|
||||
import { getServerBaseUrl } from '../../lib/desktopRuntime'
|
||||
import { getDesktopHost } from '../../lib/desktopHost'
|
||||
import { useOpenTargetStore } from '../../stores/openTargetStore'
|
||||
@ -57,12 +59,24 @@ export function CurrentTurnChangeCard({
|
||||
? files.slice(0, COLLAPSED_COUNT)
|
||||
: files
|
||||
|
||||
const openDiffInWorkspace = useCallback((fileEntry: ChangedFileEntry) => {
|
||||
const openChangedFile = useCallback((fileEntry: ChangedFileEntry) => {
|
||||
// A changed file outside the workdir (absolute displayPath — e.g. another
|
||||
// drive) has no checkpoint baseline, so a diff is meaningless. Render html in
|
||||
// the in-app browser and everything else as a file preview (served by its
|
||||
// absolute path). In-workdir files keep the diff view.
|
||||
if (isAbsoluteLocalPath(fileEntry.displayPath)) {
|
||||
if (shouldOfferStaticHtmlPreview(fileEntry.displayPath, { siblingFiles: files.map((entry) => entry.displayPath) })) {
|
||||
useBrowserPanelStore.getState().open(sessionId, localFileUrl(getServerBaseUrl(), fileEntry.apiPath))
|
||||
return
|
||||
}
|
||||
void useWorkspacePanelStore.getState().openPreview(sessionId, fileEntry.displayPath, 'file')
|
||||
return
|
||||
}
|
||||
// Jump to the right-side workspace and open a diff tab. We pass the workDir-relative
|
||||
// path (same format the workspace file tree passes to openPreview), so the diff tab
|
||||
// is keyed/fetched identically to the tree-driven one.
|
||||
void useWorkspacePanelStore.getState().openPreview(sessionId, fileEntry.displayPath, 'diff')
|
||||
}, [sessionId])
|
||||
}, [sessionId, files])
|
||||
|
||||
const handleOpenWith = useCallback((event: ReactMouseEvent<HTMLButtonElement>, fileEntry: ChangedFileEntry) => {
|
||||
event.stopPropagation()
|
||||
@ -81,6 +95,7 @@ export function CurrentTurnChangeCard({
|
||||
const ctx = openWithContextForWorkspaceFile(fileEntry.displayPath, fileEntry.apiPath, {
|
||||
sessionId,
|
||||
serverBaseUrl: getServerBaseUrl(),
|
||||
siblingFiles: files.map((entry) => entry.displayPath),
|
||||
})
|
||||
const items = buildOpenWithItems(ctx, targets, {
|
||||
openInAppBrowser: (url) => useBrowserPanelStore.getState().open(sessionId, url),
|
||||
@ -91,7 +106,7 @@ export function CurrentTurnChangeCard({
|
||||
})
|
||||
setOpenWith({ items, anchor: rect, triggerEl })
|
||||
})()
|
||||
}, [openWith, sessionId, t])
|
||||
}, [openWith, sessionId, t, files])
|
||||
|
||||
const cardLabel = isLatest
|
||||
? t('chat.turnChangesLatestCardLabel')
|
||||
@ -150,7 +165,7 @@ export function CurrentTurnChangeCard({
|
||||
<div key={fileEntry.apiPath} className="flex items-center gap-2">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => openDiffInWorkspace(fileEntry)}
|
||||
onClick={() => openChangedFile(fileEntry)}
|
||||
aria-label={t('chat.turnChangesOpenInWorkspaceAria', { path: fileEntry.displayPath })}
|
||||
title={fileEntry.displayPath}
|
||||
className="flex min-h-[52px] min-w-0 flex-1 items-center gap-3 rounded-[var(--radius-md)] px-4 text-left transition-colors hover:bg-[var(--color-surface-hover)] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-[var(--color-brand)]/35"
|
||||
|
||||
@ -770,6 +770,40 @@ function buildTurnCardInsertionMap(
|
||||
return cardsByRenderIndex
|
||||
}
|
||||
|
||||
/**
|
||||
* Map each render item to the REAL changed files of the turn it belongs to, so an
|
||||
* assistant message can anchor its output chips on files that were actually
|
||||
* written this turn instead of guessing paths from the prose. Items are attributed
|
||||
* to the most recent preceding non-pending user message (the turn boundary).
|
||||
*/
|
||||
function buildChangedFilesByRenderIndex(
|
||||
renderItems: RenderItem[],
|
||||
turnChangeCards: TurnChangeCardModel[],
|
||||
): Map<number, string[]> {
|
||||
const filesByTurnId = new Map<string, string[]>()
|
||||
for (const card of turnChangeCards) {
|
||||
if (card.checkpoint.code.filesChanged.length > 0) {
|
||||
filesByTurnId.set(card.target.messageId, card.checkpoint.code.filesChanged)
|
||||
}
|
||||
}
|
||||
if (filesByTurnId.size === 0) return new Map()
|
||||
|
||||
const filesByRenderIndex = new Map<number, string[]>()
|
||||
let activeTurnId: string | null = null
|
||||
renderItems.forEach((item, index) => {
|
||||
if (item.kind === 'message' && item.message.type === 'user_text' && !item.message.pending) {
|
||||
activeTurnId = item.message.id
|
||||
return
|
||||
}
|
||||
if (activeTurnId) {
|
||||
const files = filesByTurnId.get(activeTurnId)
|
||||
if (files) filesByRenderIndex.set(index, files)
|
||||
}
|
||||
})
|
||||
|
||||
return filesByRenderIndex
|
||||
}
|
||||
|
||||
function getApiErrorMessage(error: unknown) {
|
||||
return error instanceof ApiError
|
||||
? typeof error.body === 'object' && error.body && 'message' in error.body
|
||||
@ -1645,6 +1679,10 @@ export function MessageList({ sessionId, compact = false }: MessageListProps = {
|
||||
() => buildTurnCardInsertionMap(renderItems, turnChangeCards),
|
||||
[renderItems, turnChangeCards],
|
||||
)
|
||||
const changedFilesByRenderIndex = useMemo(
|
||||
() => buildChangedFilesByRenderIndex(renderItems, turnChangeCards),
|
||||
[renderItems, turnChangeCards],
|
||||
)
|
||||
const renderItemKeys = useMemo(
|
||||
() => renderItems.map(getRenderItemKey),
|
||||
[renderItems],
|
||||
@ -1903,6 +1941,7 @@ export function MessageList({ sessionId, compact = false }: MessageListProps = {
|
||||
: null
|
||||
}
|
||||
branchAction={branchActionByMessageId.get(item.message.id)}
|
||||
turnChangedFiles={changedFilesByRenderIndex.get(index)}
|
||||
/>
|
||||
)}
|
||||
|
||||
@ -2033,6 +2072,7 @@ export const MessageBlock = memo(function MessageBlock({
|
||||
agentTaskNotifications,
|
||||
toolResult,
|
||||
branchAction,
|
||||
turnChangedFiles,
|
||||
}: {
|
||||
sessionId?: string | null
|
||||
message: UIMessage
|
||||
@ -2044,6 +2084,7 @@ export const MessageBlock = memo(function MessageBlock({
|
||||
loading?: boolean
|
||||
onBranch: () => void
|
||||
}
|
||||
turnChangedFiles?: string[]
|
||||
}) {
|
||||
const t = useTranslation()
|
||||
|
||||
@ -2077,6 +2118,7 @@ export const MessageBlock = memo(function MessageBlock({
|
||||
branchAction={branchAction}
|
||||
sessionId={sessionId ?? undefined}
|
||||
timestamp={message.timestamp}
|
||||
turnChangedFiles={turnChangedFiles}
|
||||
/>
|
||||
</SelectableChatMessage>
|
||||
)
|
||||
|
||||
@ -251,3 +251,81 @@ describe('extractAssistantOutputTargets', () => {
|
||||
expect(targets.map((target) => target.href)).toEqual(['one.html', 'two.html'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('extractAssistantOutputTargets with changedFiles reconciliation', () => {
|
||||
it('corrects a bare mention to the real changed path in a subfolder', () => {
|
||||
// The reported bug: the model writes /private/tmp/todo-app/index.html but the
|
||||
// prose only says `index.html`, so the chip used to point at the (missing)
|
||||
// workdir-root index.html. With the turn's real changed files it is corrected.
|
||||
const targets = extractAssistantOutputTargets('已创建 `index.html`,直接用浏览器打开。', {
|
||||
workDir: '/private/tmp',
|
||||
changedFiles: [
|
||||
'/private/tmp/todo-app/index.html',
|
||||
'/private/tmp/todo-app/style.css',
|
||||
'/private/tmp/todo-app/app.js',
|
||||
],
|
||||
})
|
||||
|
||||
expect(targets).toHaveLength(1)
|
||||
expect(targets[0]).toMatchObject({
|
||||
kind: 'local-html',
|
||||
href: 'todo-app/index.html',
|
||||
normalizedPath: 'todo-app/index.html',
|
||||
subtitle: 'todo-app/index.html',
|
||||
})
|
||||
})
|
||||
|
||||
it('drops a mentioned file that the turn never changed', () => {
|
||||
const targets = extractAssistantOutputTargets('参考旧文件 old-report.html 和新结果 result.html', {
|
||||
workDir: '/work',
|
||||
changedFiles: ['/work/out/result.html'],
|
||||
})
|
||||
|
||||
expect(targets.map((target) => target.normalizedPath)).toEqual(['out/result.html'])
|
||||
})
|
||||
|
||||
it('keeps localhost url chips untouched while reconciling files', () => {
|
||||
const targets = extractAssistantOutputTargets('启动后访问 http://localhost:5173/ ,源码见 index.html', {
|
||||
workDir: '/work',
|
||||
changedFiles: ['/work/app/index.html'],
|
||||
})
|
||||
|
||||
const byKind = new Map(targets.map((target) => [target.kind, target]))
|
||||
expect(byKind.get('localhost-url')?.href).toBe('http://localhost:5173/')
|
||||
expect(byKind.get('local-html')?.normalizedPath).toBe('app/index.html')
|
||||
})
|
||||
|
||||
it('rewrites a changed file outside the workdir to its absolute posix path', () => {
|
||||
const targets = extractAssistantOutputTargets('已创建 todo.html', {
|
||||
workDir: 'C:/Users/me/tmp/session',
|
||||
changedFiles: ['D:\\workspace\\demo\\todo.html'],
|
||||
})
|
||||
|
||||
expect(targets).toHaveLength(1)
|
||||
expect(targets[0]).toMatchObject({
|
||||
kind: 'local-html',
|
||||
href: 'D:/workspace/demo/todo.html',
|
||||
normalizedPath: 'D:/workspace/demo/todo.html',
|
||||
})
|
||||
})
|
||||
|
||||
it('falls back to text-only behavior when changedFiles is empty', () => {
|
||||
const targets = extractAssistantOutputTargets('已创建 `index.html`', {
|
||||
workDir: '/private/tmp',
|
||||
changedFiles: [],
|
||||
})
|
||||
|
||||
// No reconciliation → original bare-path behavior (mention kept as-is).
|
||||
expect(targets).toMatchObject([{ kind: 'local-html', normalizedPath: 'index.html' }])
|
||||
})
|
||||
|
||||
it('does not correct when the basename is ambiguous across changed files', () => {
|
||||
const targets = extractAssistantOutputTargets('见 index.html', {
|
||||
workDir: '/work',
|
||||
changedFiles: ['/work/a/index.html', '/work/b/index.html'],
|
||||
})
|
||||
|
||||
// Ambiguous basename match → no unique target, mention dropped rather than guessed.
|
||||
expect(targets).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
@ -24,6 +24,15 @@ export type AssistantOutputTarget = {
|
||||
export type ExtractAssistantOutputTargetOptions = {
|
||||
workDir?: string | null
|
||||
limit?: number
|
||||
/**
|
||||
* The turn's REAL changed files (absolute paths from the turn checkpoint). When
|
||||
* provided, file chips are reconciled against this ground truth: a mentioned
|
||||
* file is corrected to the actual changed path (so `index.html` resolves to the
|
||||
* `todo-app/index.html` that was really written), and a mentioned file that the
|
||||
* turn never changed is dropped instead of pointing at a non-existent path.
|
||||
* Localhost URLs are unaffected. Omitted/empty → fall back to text-only behavior.
|
||||
*/
|
||||
changedFiles?: string[]
|
||||
}
|
||||
|
||||
type FileTargetMatch = {
|
||||
@ -217,9 +226,96 @@ export function extractAssistantOutputTargets(
|
||||
results.push(candidate.target)
|
||||
}
|
||||
|
||||
if (options.changedFiles && options.changedFiles.length > 0) {
|
||||
return reconcileTargetsWithChangedFiles(results, options.changedFiles, workDir)
|
||||
}
|
||||
|
||||
return results
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-anchor file chips onto the turn's real changed files. A mentioned file that
|
||||
* matches a changed file (by exact relative-path suffix, else by basename) is
|
||||
* rewritten to that real path; a mentioned file with no match is dropped so we
|
||||
* never render a chip that opens "file does not exist". Localhost URLs pass
|
||||
* through untouched.
|
||||
*/
|
||||
function reconcileTargetsWithChangedFiles(
|
||||
targets: AssistantOutputTarget[],
|
||||
changedFiles: string[],
|
||||
workDir: string | null,
|
||||
): AssistantOutputTarget[] {
|
||||
const out: AssistantOutputTarget[] = []
|
||||
const seen = new Set<string>()
|
||||
|
||||
for (const target of targets) {
|
||||
if (target.kind === 'localhost-url') {
|
||||
out.push(target)
|
||||
continue
|
||||
}
|
||||
|
||||
const mentioned = target.normalizedPath ?? target.href
|
||||
const match = matchChangedFile(mentioned, changedFiles)
|
||||
if (!match) {
|
||||
continue
|
||||
}
|
||||
|
||||
const resolvedMatch = resolveFilePath(match)
|
||||
const corrected = workDir && isWithinWorkDir(resolvedMatch, workDir)
|
||||
? relativeFilePath(workDir, resolvedMatch)
|
||||
: toPosixPath(match)
|
||||
|
||||
const key = `${target.kind}:${corrected}`
|
||||
if (seen.has(key)) {
|
||||
continue
|
||||
}
|
||||
seen.add(key)
|
||||
|
||||
out.push({
|
||||
...target,
|
||||
href: corrected,
|
||||
normalizedPath: corrected,
|
||||
subtitle: corrected,
|
||||
})
|
||||
}
|
||||
|
||||
return out
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a mentioned (often relative or bare) path against the turn's absolute
|
||||
* changed-file list. Prefer an unambiguous relative-suffix match, then fall back
|
||||
* to an unambiguous basename match. Ambiguous matches return null (we'd rather
|
||||
* keep the original text path than guess wrong).
|
||||
*/
|
||||
function matchChangedFile(mentioned: string, changedFiles: string[]): string | null {
|
||||
const normalized = toPosixPath(mentioned).replace(/^\.?\//, '').toLowerCase()
|
||||
if (!normalized) {
|
||||
return null
|
||||
}
|
||||
const basename = normalized.split('/').pop() ?? normalized
|
||||
|
||||
const suffixMatches = changedFiles.filter((file) => {
|
||||
const normalizedFile = toPosixPath(file).toLowerCase()
|
||||
return normalizedFile === normalized || normalizedFile.endsWith(`/${normalized}`)
|
||||
})
|
||||
if (suffixMatches.length === 1) {
|
||||
return suffixMatches[0]!
|
||||
}
|
||||
if (suffixMatches.length > 1) {
|
||||
return null
|
||||
}
|
||||
|
||||
const basenameMatches = changedFiles.filter(
|
||||
(file) => getBasename(file).toLowerCase() === basename,
|
||||
)
|
||||
if (basenameMatches.length === 1) {
|
||||
return basenameMatches[0]!
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
function toWorkspaceFileTarget(candidate: string, workDir: string | null): FileTargetMatch | null {
|
||||
if (!candidate || candidate.startsWith('file://') || isExternalUrl(candidate)) {
|
||||
return null
|
||||
|
||||
@ -75,12 +75,18 @@ describe('handlePreviewLink', () => {
|
||||
expect(deps.openFilePreview).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('routes a frontend project index.html to workspace preview instead of static browser preview', () => {
|
||||
it('routes a hand-authored single-page index.html to the static browser preview', () => {
|
||||
// Without change-set context a bare index.html is assumed to be a standalone
|
||||
// static page (the common "make me a todo page" output), so it renders in the
|
||||
// in-app browser rather than dropping the user into the source view.
|
||||
const deps = makeDeps()
|
||||
const handled = handlePreviewLink('todo-app/index.html', deps)
|
||||
expect(handled).toBe(true)
|
||||
expect(deps.openFilePreview).toHaveBeenCalledWith('s1', 'todo-app/index.html')
|
||||
expect(deps.openBrowser).not.toHaveBeenCalled()
|
||||
expect(deps.openBrowser).toHaveBeenCalledWith(
|
||||
's1',
|
||||
'http://127.0.0.1:8787/preview-fs/s1/todo-app/index.html',
|
||||
)
|
||||
expect(deps.openFilePreview).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('routes relative previewable docs to openFilePreview with the relative path', () => {
|
||||
|
||||
82
desktop/src/lib/htmlPreviewPolicy.test.ts
Normal file
82
desktop/src/lib/htmlPreviewPolicy.test.ts
Normal file
@ -0,0 +1,82 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { isHtmlFilePath, shouldOfferStaticHtmlPreview } from './htmlPreviewPolicy'
|
||||
|
||||
describe('isHtmlFilePath', () => {
|
||||
it('matches html/htm/xhtml regardless of separators and suffixes', () => {
|
||||
expect(isHtmlFilePath('index.html')).toBe(true)
|
||||
expect(isHtmlFilePath('page.htm')).toBe(true)
|
||||
expect(isHtmlFilePath('a/b.xhtml')).toBe(true)
|
||||
expect(isHtmlFilePath('C:\\proj\\page.html')).toBe(true)
|
||||
expect(isHtmlFilePath('app.html?v=2#top')).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects non-html paths', () => {
|
||||
expect(isHtmlFilePath('style.css')).toBe(false)
|
||||
expect(isHtmlFilePath('readme.md')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('shouldOfferStaticHtmlPreview', () => {
|
||||
it('is false for non-html files', () => {
|
||||
expect(shouldOfferStaticHtmlPreview('app.js')).toBe(false)
|
||||
expect(shouldOfferStaticHtmlPreview('styles/site.css')).toBe(false)
|
||||
})
|
||||
|
||||
it('offers static preview for non-index html pages', () => {
|
||||
expect(shouldOfferStaticHtmlPreview('todo.html')).toBe(true)
|
||||
expect(shouldOfferStaticHtmlPreview('pages/about.html')).toBe(true)
|
||||
})
|
||||
|
||||
it('offers static preview for a hand-authored single-page index.html (no project manifest)', () => {
|
||||
// The real-world regression: a "make me a todo page" output written into a
|
||||
// subfolder must still get a browser preview — it is NOT a build template.
|
||||
expect(shouldOfferStaticHtmlPreview('todo-app/index.html')).toBe(true)
|
||||
expect(
|
||||
shouldOfferStaticHtmlPreview('todo-app/index.html', {
|
||||
siblingFiles: ['todo-app/index.html', 'todo-app/style.css', 'todo-app/app.js'],
|
||||
}),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('defaults a bare index.html to static preview when no change-set context is given', () => {
|
||||
expect(shouldOfferStaticHtmlPreview('index.html')).toBe(true)
|
||||
})
|
||||
|
||||
it('routes a framework-template index.html (manifest in the same change-set) to source view', () => {
|
||||
expect(
|
||||
shouldOfferStaticHtmlPreview('index.html', {
|
||||
siblingFiles: ['index.html', 'package.json', 'vite.config.ts', 'src/main.tsx'],
|
||||
}),
|
||||
).toBe(false)
|
||||
expect(
|
||||
shouldOfferStaticHtmlPreview('my-app/index.html', {
|
||||
siblingFiles: ['my-app/package.json', 'my-app/index.html', 'my-app/src/main.tsx'],
|
||||
}),
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('only treats a manifest as a signal when it sits at or above the index.html dir', () => {
|
||||
// A package.json in an unrelated sibling folder must not suppress the preview.
|
||||
expect(
|
||||
shouldOfferStaticHtmlPreview('site/index.html', {
|
||||
siblingFiles: ['api/package.json', 'site/index.html', 'site/app.js'],
|
||||
}),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('always offers static preview for build-output and generated exports', () => {
|
||||
expect(shouldOfferStaticHtmlPreview('dist/index.html')).toBe(true)
|
||||
expect(shouldOfferStaticHtmlPreview('out/index.html')).toBe(true)
|
||||
expect(shouldOfferStaticHtmlPreview('build/index.html')).toBe(true)
|
||||
expect(shouldOfferStaticHtmlPreview('report_files/index.html')).toBe(true)
|
||||
// Even with a manifest present, a built artifact dir stays statically previewable.
|
||||
expect(
|
||||
shouldOfferStaticHtmlPreview('dist/index.html', { siblingFiles: ['package.json', 'dist/index.html'] }),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('normalizes windows separators and url suffixes', () => {
|
||||
expect(shouldOfferStaticHtmlPreview('todo-app\\index.html')).toBe(true)
|
||||
expect(shouldOfferStaticHtmlPreview('page.html?v=1#frag')).toBe(true)
|
||||
})
|
||||
})
|
||||
@ -14,6 +14,21 @@ const STATIC_OUTPUT_DIRS = new Set([
|
||||
'storybook-static',
|
||||
])
|
||||
|
||||
/**
|
||||
* Files whose presence in the SAME change-set marks a buildable source project
|
||||
* (a framework scaffold). For such a project a root `index.html` is almost
|
||||
* always a build-tool template (`<script type="module" src="/src/main.tsx">`)
|
||||
* that renders blank when served statically — it needs a dev server — so we
|
||||
* route it to the workspace source view instead of a static browser preview.
|
||||
*
|
||||
* A hand-authored single-page `index.html` (the common "make me a todo page"
|
||||
* output) ships with no manifest alongside it, so it stays statically
|
||||
* previewable. This is the signal that lets us tell the two apart without
|
||||
* reading file contents.
|
||||
*/
|
||||
const PROJECT_MANIFEST_RE =
|
||||
/^(?:package\.json|(?:vite|next|nuxt|svelte|astro|remix|rollup|webpack|rsbuild|rspack|parcel|gatsby)\.config\.[cm]?[jt]s|angular\.json|deno\.jsonc?)$/i
|
||||
|
||||
function normalizePathForPolicy(filePath: string): string {
|
||||
return filePath
|
||||
.split(/[?#]/, 1)[0]!
|
||||
@ -25,14 +40,61 @@ export function isHtmlFilePath(filePath: string): boolean {
|
||||
return HTML_EXT.test(normalizePathForPolicy(filePath))
|
||||
}
|
||||
|
||||
export function shouldOfferStaticHtmlPreview(filePath: string): boolean {
|
||||
function dirOfNormalized(normalizedPath: string): string {
|
||||
const slash = normalizedPath.lastIndexOf('/')
|
||||
return slash < 0 ? '' : normalizedPath.slice(0, slash)
|
||||
}
|
||||
|
||||
/** True when `ancestorDir` is the same as, or a parent of, `dir`. */
|
||||
function isAtOrAbove(ancestorDir: string, dir: string): boolean {
|
||||
if (ancestorDir === dir) return true
|
||||
return ancestorDir === '' || dir.startsWith(`${ancestorDir}/`)
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the change-set contains a project manifest sitting at or above the
|
||||
* `index.html`'s own directory — i.e. the html is the entry template of a real
|
||||
* buildable project rather than a standalone static page.
|
||||
*/
|
||||
function hasProjectManifestSibling(indexNormalizedPath: string, siblingFiles: string[]): boolean {
|
||||
const indexDir = dirOfNormalized(indexNormalizedPath)
|
||||
return siblingFiles.some((raw) => {
|
||||
const sibling = normalizePathForPolicy(raw)
|
||||
const name = sibling.slice(sibling.lastIndexOf('/') + 1)
|
||||
if (!PROJECT_MANIFEST_RE.test(name)) return false
|
||||
return isAtOrAbove(dirOfNormalized(sibling), indexDir)
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide whether an html file should be offered as a static browser preview
|
||||
* (served as-is via `/preview-fs` or `/local-file`) versus the workspace source
|
||||
* view.
|
||||
*
|
||||
* Everything that is clearly a finished static artifact previews statically:
|
||||
* non-`index` pages (`todo.html`), generated `*_files/index.html` exports, and
|
||||
* anything under a build-output dir (`dist/`, `build/`, `out/`, …).
|
||||
*
|
||||
* A bare `index.html` is ambiguous from its path alone. When the caller can
|
||||
* supply the rest of the change-set via `opts.siblingFiles`, we treat the html
|
||||
* as a framework template (→ source view, not static preview) ONLY when a
|
||||
* project manifest (`package.json`, `vite.config.*`, …) accompanies it. Without
|
||||
* that signal we default to offering the static preview, so a hand-authored
|
||||
* single-page `index.html` is no longer mis-classified as a build template.
|
||||
*/
|
||||
export function shouldOfferStaticHtmlPreview(
|
||||
filePath: string,
|
||||
opts?: { siblingFiles?: string[] },
|
||||
): boolean {
|
||||
const normalized = normalizePathForPolicy(filePath)
|
||||
if (!HTML_EXT.test(normalized)) return false
|
||||
if (!INDEX_HTML_RE.test(normalized)) return true
|
||||
if (GENERATED_STATIC_FILES_DIR_RE.test(normalized)) return true
|
||||
|
||||
return normalized
|
||||
.split('/')
|
||||
.filter(Boolean)
|
||||
.some((segment) => STATIC_OUTPUT_DIRS.has(segment.toLowerCase()))
|
||||
if (normalized.split('/').filter(Boolean).some((segment) => STATIC_OUTPUT_DIRS.has(segment.toLowerCase()))) {
|
||||
return true
|
||||
}
|
||||
if (!INDEX_HTML_RE.test(normalized)) return true
|
||||
if (opts?.siblingFiles && hasProjectManifestSibling(normalized, opts.siblingFiles)) {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
@ -77,13 +77,32 @@ describe('openWithContextForWorkspaceFile', () => {
|
||||
expect(result).toEqual({ kind: 'file', absolutePath: '/w/proj/README.md', relPath: 'README.md', previewable: true })
|
||||
})
|
||||
|
||||
it('frontend project index.html rel path → no inAppBrowserUrl because it needs a dev server', () => {
|
||||
const result = openWithContextForWorkspaceFile('todo-app/index.html', '/w/proj/todo-app/index.html', { sessionId: SESSION, serverBaseUrl: BASE })
|
||||
it('hand-authored single-page index.html (no manifest in change-set) → static preview offered', () => {
|
||||
const result = openWithContextForWorkspaceFile('todo-app/index.html', '/w/proj/todo-app/index.html', {
|
||||
sessionId: SESSION,
|
||||
serverBaseUrl: BASE,
|
||||
siblingFiles: ['todo-app/index.html', 'todo-app/style.css', 'todo-app/app.js'],
|
||||
})
|
||||
expect(result).toEqual({
|
||||
kind: 'file',
|
||||
absolutePath: '/w/proj/todo-app/index.html',
|
||||
relPath: 'todo-app/index.html',
|
||||
previewable: true,
|
||||
inAppBrowserUrl: previewFsUrl(BASE, SESSION, 'todo-app/index.html'),
|
||||
})
|
||||
})
|
||||
|
||||
it('framework-template index.html (manifest in change-set) → no inAppBrowserUrl (needs dev server)', () => {
|
||||
const result = openWithContextForWorkspaceFile('index.html', '/w/proj/index.html', {
|
||||
sessionId: SESSION,
|
||||
serverBaseUrl: BASE,
|
||||
siblingFiles: ['index.html', 'package.json', 'vite.config.ts', 'src/main.tsx'],
|
||||
})
|
||||
expect(result).toEqual({
|
||||
kind: 'file',
|
||||
absolutePath: '/w/proj/index.html',
|
||||
relPath: 'index.html',
|
||||
previewable: true,
|
||||
})
|
||||
})
|
||||
|
||||
@ -126,4 +145,31 @@ describe('openWithContextForWorkspaceFile', () => {
|
||||
expect(result.previewable).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('outside-workspace html (absolute displayPath) → inAppBrowserUrl via localFileUrl, not preview-fs', () => {
|
||||
// A changed file that could not be relativized arrives with an absolute relPath;
|
||||
// it lives outside the workdir, so it must be served by the /local-file route.
|
||||
const result = openWithContextForWorkspaceFile('D:/workspace/demo/todo.html', 'D:\\workspace\\demo\\todo.html', {
|
||||
sessionId: SESSION,
|
||||
serverBaseUrl: BASE,
|
||||
})
|
||||
expect(result).toEqual({
|
||||
kind: 'file',
|
||||
absolutePath: 'D:\\workspace\\demo\\todo.html',
|
||||
relPath: 'D:/workspace/demo/todo.html',
|
||||
previewable: true,
|
||||
inAppBrowserUrl: localFileUrl(BASE, 'D:\\workspace\\demo\\todo.html'),
|
||||
})
|
||||
})
|
||||
|
||||
it('outside-workspace POSIX html absolute path → inAppBrowserUrl via localFileUrl', () => {
|
||||
const result = openWithContextForWorkspaceFile('/elsewhere/site/todo.html', '/elsewhere/site/todo.html', {
|
||||
sessionId: SESSION,
|
||||
serverBaseUrl: BASE,
|
||||
})
|
||||
expect(result.kind).toBe('file')
|
||||
if (result.kind === 'file') {
|
||||
expect(result.inAppBrowserUrl).toBe(localFileUrl(BASE, '/elsewhere/site/todo.html'))
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@ -3,18 +3,36 @@ import { shouldOfferStaticHtmlPreview } from './htmlPreviewPolicy'
|
||||
import { isAbsoluteLocalPath, localFileUrl, previewFsUrl } from './handlePreviewLink'
|
||||
import type { OpenWithContext } from './openWithItems'
|
||||
|
||||
/** Build an open-with context for a workspace file (we have both its relative + absolute path). */
|
||||
/**
|
||||
* Build an open-with context for a workspace file (we have both its relative +
|
||||
* absolute path).
|
||||
*
|
||||
* `siblingFiles` is the rest of the same change-set (the turn's changed files).
|
||||
* It lets {@link shouldOfferStaticHtmlPreview} tell a hand-authored single-page
|
||||
* `index.html` (→ offer a static browser preview) from a framework template
|
||||
* that ships with a `package.json` / `vite.config.*` (→ source view only).
|
||||
*/
|
||||
export function openWithContextForWorkspaceFile(
|
||||
relPath: string,
|
||||
absolutePath: string,
|
||||
opts: { sessionId: string; serverBaseUrl: string },
|
||||
opts: { sessionId: string; serverBaseUrl: string; siblingFiles?: string[] },
|
||||
): OpenWithContext {
|
||||
// A changed file that could not be relativized against the workdir arrives with
|
||||
// an absolute `relPath` — it lives outside the session workspace (e.g. another
|
||||
// drive). Such a file is served by the $HOME/registered-root /local-file route,
|
||||
// not the workdir-sandboxed /preview-fs route.
|
||||
const outsideWorkspace = isAbsoluteLocalPath(relPath)
|
||||
const inAppBrowserUrl = shouldOfferStaticHtmlPreview(relPath, { siblingFiles: opts.siblingFiles })
|
||||
? outsideWorkspace
|
||||
? localFileUrl(opts.serverBaseUrl, absolutePath)
|
||||
: previewFsUrl(opts.serverBaseUrl, opts.sessionId, relPath)
|
||||
: undefined
|
||||
return {
|
||||
kind: 'file',
|
||||
absolutePath,
|
||||
relPath,
|
||||
previewable: true,
|
||||
inAppBrowserUrl: shouldOfferStaticHtmlPreview(relPath) ? previewFsUrl(opts.serverBaseUrl, opts.sessionId, relPath) : undefined,
|
||||
inAppBrowserUrl,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -129,6 +129,7 @@ import { sessionsApi } from '../api/sessions'
|
||||
import {
|
||||
mapHistoryMessagesToUiMessages,
|
||||
reconstructAgentNotifications,
|
||||
stripGeneratedImageMetadataLines,
|
||||
type PerSessionState,
|
||||
useChatStore,
|
||||
} from './chatStore'
|
||||
@ -163,6 +164,28 @@ function makeSession(overrides: Partial<PerSessionState> = {}): PerSessionState
|
||||
}
|
||||
}
|
||||
|
||||
describe('stripGeneratedImageMetadataLines', () => {
|
||||
it('removes simple, detailed, and resize metadata lines but keeps the prompt body', () => {
|
||||
const text = [
|
||||
'first line of the prompt',
|
||||
'second line',
|
||||
'[Image source: C:\\Users\\Relakkes\\.claude\\uploads\\sid\\a.png]',
|
||||
'[Image: source: /Users/me/.claude/uploads/sid/b.png, original 1024x768, displayed at 512x384. Multiply coordinates by 2 to map to original image.]',
|
||||
'[Image: original 800x600, displayed at 400x300. Multiply coordinates by 2 to map to original image.]',
|
||||
].join('\n')
|
||||
expect(stripGeneratedImageMetadataLines(text)).toBe('first line of the prompt\nsecond line')
|
||||
})
|
||||
|
||||
it('normalizes CRLF and leaves metadata-free text untouched', () => {
|
||||
expect(stripGeneratedImageMetadataLines('a\r\nb\r\n')).toBe('a\nb')
|
||||
expect(stripGeneratedImageMetadataLines('just a normal prompt')).toBe('just a normal prompt')
|
||||
})
|
||||
|
||||
it('returns empty string when the text is only metadata', () => {
|
||||
expect(stripGeneratedImageMetadataLines('[Image source: /tmp/x.png]')).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
describe('chatStore history mapping', () => {
|
||||
beforeEach(() => {
|
||||
sendMock.mockReset()
|
||||
@ -3522,6 +3545,64 @@ describe('chatStore history mapping', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('does not leak an image-bearing prompt when the replay appends [Image source] metadata (Windows path)', () => {
|
||||
// The optimistic message (e.g. a visual-selection annotation card) stores the
|
||||
// prompt body in modelContent with a hidden display. The CLI replay carries
|
||||
// the server-appended `[Image source: …]` line on the same text. Dedupe must
|
||||
// still match — otherwise the raw prompt + absolute upload path leak in as a
|
||||
// second grey bubble (the reported Windows regression).
|
||||
const prompt = '请根据截图中编号 1 的蓝色标注修改本地前端。\n目标元素:<button>'
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
[TEST_SESSION_ID]: makeSession({
|
||||
messages: [
|
||||
{
|
||||
id: 'live-user',
|
||||
type: 'user_text',
|
||||
content: '',
|
||||
modelContent: prompt,
|
||||
timestamp: 1,
|
||||
},
|
||||
],
|
||||
chatState: 'thinking',
|
||||
}),
|
||||
},
|
||||
})
|
||||
|
||||
useChatStore.getState().handleServerMessage(TEST_SESSION_ID, {
|
||||
type: 'user_message_replay',
|
||||
content: `${prompt}\n[Image source: C:\\Users\\Relakkes\\.claude\\uploads\\sid\\82017405-_button_.png]`,
|
||||
})
|
||||
|
||||
const userMessages = useChatStore.getState().sessions[TEST_SESSION_ID]?.messages
|
||||
.filter((message) => message.type === 'user_text')
|
||||
expect(userMessages).toHaveLength(1)
|
||||
expect(userMessages?.[0]).toMatchObject({ content: '', modelContent: prompt })
|
||||
})
|
||||
|
||||
it('dedupes an image-bearing prompt when the replay appends detailed (macOS) image metadata', () => {
|
||||
const prompt = 'describe this screenshot for me'
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
[TEST_SESSION_ID]: makeSession({
|
||||
messages: [
|
||||
{ id: 'live-user', type: 'user_text', content: prompt, timestamp: 1 },
|
||||
],
|
||||
chatState: 'thinking',
|
||||
}),
|
||||
},
|
||||
})
|
||||
|
||||
useChatStore.getState().handleServerMessage(TEST_SESSION_ID, {
|
||||
type: 'user_message_replay',
|
||||
content: `${prompt}\n[Image: source: /Users/me/.claude/uploads/sid/a.png, original 1024x768, displayed at 512x384. Multiply coordinates by 2 to map to original image.]`,
|
||||
})
|
||||
|
||||
const userMessages = useChatStore.getState().sessions[TEST_SESSION_ID]?.messages
|
||||
.filter((message) => message.type === 'user_text')
|
||||
expect(userMessages).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('flushes pending text before appending an error message', () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
|
||||
@ -2188,6 +2188,24 @@ function isGeneratedImageMetadataText(text: string): boolean {
|
||||
return Boolean(extractImageMetadataSourcePath(text)) || IMAGE_RESIZE_METADATA_RE.test(text.trim())
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip the generated image-metadata lines (`[Image source: …]`, resize notes)
|
||||
* that the server appends to a user turn's text. The optimistic message never
|
||||
* carried them, so live-replay dedupe must normalize them away first — otherwise
|
||||
* `findCurrentTurnUserMessageIndex` never matches and the raw prompt leaks in as
|
||||
* a duplicate bubble. This was most visible on Windows, where the appended
|
||||
* absolute upload path (`[Image source: C:\Users\…\uploads\…png]`) made the
|
||||
* mismatch obvious, but it affects any message that carries an image.
|
||||
*/
|
||||
export function stripGeneratedImageMetadataLines(text: string): string {
|
||||
return text
|
||||
.replace(/\r\n?/g, '\n')
|
||||
.split('\n')
|
||||
.filter((line) => !isGeneratedImageMetadataText(line))
|
||||
.join('\n')
|
||||
.trim()
|
||||
}
|
||||
|
||||
function parseVisualSelectionHistoryPrompt(text: string): VisualSelectionHistoryDisplay | null {
|
||||
const lines = text.replace(/\r\n?/g, '\n').split('\n')
|
||||
if (lines[0]?.trim() !== VISUAL_SELECTION_PROMPT_HEADER) return null
|
||||
@ -2745,16 +2763,21 @@ function extractLeadingFileReferences(text: string): {
|
||||
}
|
||||
}
|
||||
|
||||
function appendReplayedUserMessage(
|
||||
export function appendReplayedUserMessage(
|
||||
messages: UIMessage[],
|
||||
content: string,
|
||||
timestamp: number,
|
||||
): UIMessage[] {
|
||||
const parsed = extractLeadingFileReferences(content)
|
||||
const displayContent = parsed.content.trim() || content.trim()
|
||||
// The replayed text carries server-appended image-metadata lines that the
|
||||
// optimistic message never had. Normalize them away (same as the history
|
||||
// mapping) so the dedupe below can match the already-rendered message instead
|
||||
// of appending the raw prompt — paths and all — as a duplicate bubble.
|
||||
const sanitized = stripGeneratedImageMetadataLines(content) || content.trim()
|
||||
const parsed = extractLeadingFileReferences(sanitized)
|
||||
const displayContent = parsed.content.trim() || sanitized
|
||||
if (!displayContent) return messages
|
||||
|
||||
const modelContent = parsed.modelContent ?? content.trim()
|
||||
const modelContent = parsed.modelContent ?? sanitized
|
||||
const currentTurnUserIndex = findCurrentTurnUserMessageIndex(messages, modelContent)
|
||||
if (currentTurnUserIndex >= 0) {
|
||||
const optimisticMessage = messages[currentTurnUserIndex]
|
||||
|
||||
@ -4,6 +4,10 @@ import { execFileSync } from 'node:child_process'
|
||||
import * as os from 'node:os'
|
||||
import * as path from 'node:path'
|
||||
import { WorkspaceService } from '../services/workspaceService.js'
|
||||
import {
|
||||
clearFilesystemAccessRootsForTests,
|
||||
registerFilesystemAccessRoot,
|
||||
} from '../services/filesystemAccessRoots.js'
|
||||
|
||||
const cleanupDirs = new Set<string>()
|
||||
const ONE_MIB = 1024 * 1024
|
||||
@ -76,6 +80,43 @@ afterEach(async () => {
|
||||
cleanupDirs.clear()
|
||||
})
|
||||
|
||||
describe('WorkspaceService outside-workspace preview', () => {
|
||||
afterEach(() => {
|
||||
clearFilesystemAccessRootsForTests()
|
||||
})
|
||||
|
||||
it('rejects a file outside the workdir until its dir is a registered access root, then reads it', async () => {
|
||||
const workDir = await makeTempDir('workspace-service-work-')
|
||||
const outsideDir = await makeTempDir('workspace-service-outside-')
|
||||
const outsideFile = path.join(outsideDir, 'todo.html')
|
||||
await fs.writeFile(outsideFile, '<h1>hi</h1>\n')
|
||||
|
||||
const service = new WorkspaceService(async (sessionId) => sessionId === 'session-1' ? workDir : null)
|
||||
|
||||
// Not registered yet → treated as a sandbox escape.
|
||||
await expect(service.readFile('session-1', outsideFile)).rejects.toThrow(/outside workspace/)
|
||||
|
||||
// Registered (as the turn-checkpoint flow does for changed files) → readable.
|
||||
registerFilesystemAccessRoot(outsideDir)
|
||||
const allowed = await service.readFile('session-1', outsideFile)
|
||||
expect(allowed.state).toBe('ok')
|
||||
expect(allowed.content).toContain('<h1>hi</h1>')
|
||||
})
|
||||
|
||||
it('still rejects an unrelated path even when another outside dir is registered', async () => {
|
||||
const workDir = await makeTempDir('workspace-service-work-')
|
||||
const registeredDir = await makeTempDir('workspace-service-reg-')
|
||||
const unrelatedDir = await makeTempDir('workspace-service-unrelated-')
|
||||
const unrelatedFile = path.join(unrelatedDir, 'secret.txt')
|
||||
await fs.writeFile(unrelatedFile, 'nope\n')
|
||||
registerFilesystemAccessRoot(registeredDir)
|
||||
|
||||
const service = new WorkspaceService(async (sessionId) => sessionId === 'session-1' ? workDir : null)
|
||||
|
||||
await expect(service.readFile('session-1', unrelatedFile)).rejects.toThrow(/outside workspace/)
|
||||
})
|
||||
})
|
||||
|
||||
describe('WorkspaceService', () => {
|
||||
it('returns git status for modified, added, deleted, and untracked files', async () => {
|
||||
const repoDir = await createGitWorkspace()
|
||||
|
||||
@ -40,7 +40,7 @@ import {
|
||||
createSessionBranch,
|
||||
SessionBranchingError,
|
||||
} from '../../utils/sessionBranching.js'
|
||||
import { registerFilesystemAccessRoot } from '../services/filesystemAccessRoots.js'
|
||||
import { registerChangedFileAccessRoot, registerFilesystemAccessRoot } from '../services/filesystemAccessRoots.js'
|
||||
import { traceCaptureService, trimTraceCallPreviews } from '../services/traceCaptureService.js'
|
||||
|
||||
const workspaceService = new WorkspaceService(
|
||||
@ -851,6 +851,14 @@ async function branchSession(req: Request, sessionId: string): Promise<Response>
|
||||
|
||||
async function getTurnCheckpoints(sessionId: string): Promise<Response> {
|
||||
const checkpoints = await listSessionTurnCheckpoints(sessionId)
|
||||
// Make this turn's real changed files previewable even when they live outside
|
||||
// the session workdir (e.g. the user told the model to write to an absolute
|
||||
// path on another drive). Writing them was authorized, so previewing is too.
|
||||
for (const checkpoint of checkpoints) {
|
||||
for (const filePath of checkpoint.code.filesChanged) {
|
||||
registerChangedFileAccessRoot(filePath, checkpoint.workDir)
|
||||
}
|
||||
}
|
||||
return Response.json({ checkpoints })
|
||||
}
|
||||
|
||||
|
||||
37
src/server/services/filesystemAccessRoots.test.ts
Normal file
37
src/server/services/filesystemAccessRoots.test.ts
Normal file
@ -0,0 +1,37 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'bun:test'
|
||||
import {
|
||||
clearFilesystemAccessRootsForTests,
|
||||
isWithinRegisteredFilesystemRoot,
|
||||
registerChangedFileAccessRoot,
|
||||
} from './filesystemAccessRoots.js'
|
||||
|
||||
describe('registerChangedFileAccessRoot', () => {
|
||||
beforeEach(() => clearFilesystemAccessRootsForTests())
|
||||
afterEach(() => clearFilesystemAccessRootsForTests())
|
||||
|
||||
it('registers the containing dir of a changed file outside the workdir', () => {
|
||||
registerChangedFileAccessRoot('/elsewhere/proj/todo.html', '/work/dir')
|
||||
expect(isWithinRegisteredFilesystemRoot('/elsewhere/proj/todo.html')).toBe(true)
|
||||
// sibling assets in the same dir become previewable too (html needs its css/js)
|
||||
expect(isWithinRegisteredFilesystemRoot('/elsewhere/proj/style.css')).toBe(true)
|
||||
// an unrelated dir stays denied
|
||||
expect(isWithinRegisteredFilesystemRoot('/elsewhere/other/secret.txt')).toBe(false)
|
||||
})
|
||||
|
||||
it('skips files already inside the workdir (already previewable)', () => {
|
||||
registerChangedFileAccessRoot('/work/dir/sub/page.html', '/work/dir')
|
||||
expect(isWithinRegisteredFilesystemRoot('/work/dir/sub/page.html')).toBe(false)
|
||||
})
|
||||
|
||||
it('registers when no workdir context is given', () => {
|
||||
registerChangedFileAccessRoot('/scratch/a/b.html', null)
|
||||
expect(isWithinRegisteredFilesystemRoot('/scratch/a/b.html')).toBe(true)
|
||||
})
|
||||
|
||||
it('ignores empty / nullish input', () => {
|
||||
registerChangedFileAccessRoot('', '/work/dir')
|
||||
registerChangedFileAccessRoot(null, '/work/dir')
|
||||
registerChangedFileAccessRoot(undefined, '/work/dir')
|
||||
expect(isWithinRegisteredFilesystemRoot('/anything')).toBe(false)
|
||||
})
|
||||
})
|
||||
@ -15,6 +15,28 @@ export function registerFilesystemAccessRoot(rootPath: string | null | undefined
|
||||
registeredRoots.add(path.resolve(normalizeDriveRootPathForPlatform(rootPath)))
|
||||
}
|
||||
|
||||
/**
|
||||
* Register the directory of a file this session actually changed so it becomes
|
||||
* previewable, even when the user pointed the model at an absolute path outside
|
||||
* the session workdir (a different folder, or a different drive on Windows).
|
||||
*
|
||||
* Writing the file was already authorized via the permission system, so reading
|
||||
* it back for a preview is consistent. Files inside the workdir need nothing —
|
||||
* they are previewable already — so those are skipped to keep the root set tight.
|
||||
*/
|
||||
export function registerChangedFileAccessRoot(
|
||||
absoluteFilePath: string | null | undefined,
|
||||
workDir: string | null | undefined,
|
||||
): void {
|
||||
if (!absoluteFilePath) return
|
||||
const resolved = path.resolve(normalizeDriveRootPathForPlatform(absoluteFilePath))
|
||||
if (workDir) {
|
||||
const root = path.resolve(normalizeDriveRootPathForPlatform(workDir))
|
||||
if (isWithinRoot(resolved, root)) return
|
||||
}
|
||||
registeredRoots.add(path.dirname(resolved))
|
||||
}
|
||||
|
||||
export function isWithinRegisteredFilesystemRoot(targetPath: string): boolean {
|
||||
for (const rootPath of registeredRoots) {
|
||||
if (isWithinRoot(targetPath, rootPath)) return true
|
||||
|
||||
@ -6,6 +6,7 @@ import { diffLines } from 'diff'
|
||||
import type { MessageEntry } from './sessionService.js'
|
||||
import type { FileHistorySnapshot } from '../../utils/fileHistory.js'
|
||||
import { getClaudeConfigHomeDir } from '../../utils/envUtils.js'
|
||||
import { isWithinRegisteredFilesystemRoot } from './filesystemAccessRoots.js'
|
||||
import {
|
||||
isSameOrInsidePathForPlatform,
|
||||
normalizeDriveRootPathForPlatform,
|
||||
@ -1059,6 +1060,14 @@ export class WorkspaceService {
|
||||
|
||||
const absolutePath = path.resolve(workDir, requestedPath || '.')
|
||||
if (!this.isWithinRoot(absolutePath, workDir)) {
|
||||
// Files this session changed outside its workdir (the user pointed the
|
||||
// model at an absolute path elsewhere, possibly another drive) are
|
||||
// registered as access roots when the turn checkpoint is built. Preview
|
||||
// those by absolute path — they have no workspace-relative form — instead
|
||||
// of rejecting them as out-of-sandbox.
|
||||
if (isWithinRegisteredFilesystemRoot(absolutePath)) {
|
||||
return this.resolveOutsideWorkspacePath(absolutePath, requestedPath)
|
||||
}
|
||||
throw new Error(`Path is outside workspace: ${requestedPath}`)
|
||||
}
|
||||
|
||||
@ -1080,6 +1089,33 @@ export class WorkspaceService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a path that sits OUTSIDE the session workdir but inside a registered
|
||||
* access root (a file this turn actually changed elsewhere). Such a file has no
|
||||
* meaningful workspace-relative form, so it is keyed by its absolute request
|
||||
* path and rooted at its own containing directory — enough for `readFile`
|
||||
* (and a best-effort git diff if that directory happens to be a repo).
|
||||
*/
|
||||
private async resolveOutsideWorkspacePath(
|
||||
absolutePath: string,
|
||||
requestedPath: string,
|
||||
): Promise<WorkspacePathResolution> {
|
||||
let canonicalTargetPath = absolutePath
|
||||
try {
|
||||
canonicalTargetPath = await fs.realpath(absolutePath)
|
||||
} catch {
|
||||
// File may not exist yet, or realpath is unavailable — keep the raw path.
|
||||
}
|
||||
return {
|
||||
absolutePath,
|
||||
requestedPath,
|
||||
workspaceRoot: path.dirname(absolutePath),
|
||||
canonicalWorkspaceRoot: path.dirname(canonicalTargetPath),
|
||||
canonicalTargetPath,
|
||||
relativePath: this.normalizeRequestedPath(requestedPath),
|
||||
}
|
||||
}
|
||||
|
||||
private async validateCanonicalWorkspacePath(
|
||||
canonicalWorkspaceRoot: string,
|
||||
absolutePath: string,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user