From d78355cf59862f4bc0002c1306b467f935be4f33 Mon Sep 17 00:00:00 2001 From: brooklyn! Date: Thu, 24 Sep 2026 00:20:00 -0500 Subject: [PATCH] fix(desktop): tag and preview Windows absolute paths (C:\, UNC) --- .../assistant-ui/tool/fallback-model.test.ts | 25 ++++++++++++++++++- .../tool/fallback-model/targets.ts | 6 +++-- apps/desktop/src/lib/local-preview.test.ts | 13 ++++++++++ apps/desktop/src/lib/local-preview.ts | 3 ++- apps/desktop/src/lib/path-compare.test.ts | 12 ++++++++- apps/desktop/src/lib/path-compare.ts | 4 +++ apps/desktop/src/lib/preview-targets.test.ts | 20 ++++++++++++++- apps/desktop/src/lib/preview-targets.ts | 7 ++++++ 8 files changed, 84 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts index 3c04f0ed32..a7508cc7b0 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts @@ -7,9 +7,11 @@ import { clampForDisplay, countDiffLineStats, inlineDiffFromResult, + isPreviewableTarget, MAX_TOOL_RENDER_CHARS, prettyJson, - type ToolPart + type ToolPart, + toolPreviewOutcome } from './fallback-model' const part = (overrides: Partial): ToolPart => ({ @@ -464,6 +466,27 @@ describe('buildToolView title actions', () => { }) }) +// #85132: Windows agents write `C:\\...` / UNC paths; those must get the same +// artifact preview tag a POSIX `/Users/...` path gets. +describe('Windows absolute preview targets', () => { + it.each(['C:\\Users\\me\\report.html', 'D:/work/report.htm', '\\\\server\\share\\report.html'])( + 'tags a written %s as a previewable artifact', + path => { + const outcome = toolPreviewOutcome( + part({ args: { content: '

hi

', path }, result: { bytes_written: 11 }, toolName: 'write_file' }) + ) + + expect(outcome.previewTarget).toBe(path) + expect(isPreviewableTarget(outcome.previewTarget)).toBe(true) + } + ) + + it('keeps non-HTML Windows files out of the preview tag, like POSIX ones', () => { + expect(isPreviewableTarget('C:\\Users\\me\\notes.txt')).toBe(isPreviewableTarget('/Users/me/notes.txt')) + expect(isPreviewableTarget('C:\\Users\\me\\notes.txt')).toBe(false) + }) +}) + describe('clampForDisplay', () => { it('passes short payloads through untouched', () => { expect(clampForDisplay('hello')).toBe('hello') diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts index 01605f80b4..a6cf124a56 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts @@ -1,3 +1,5 @@ +import { isWindowsAbsolutePath } from '@/lib/path-compare' + import type { ToolPart } from './types' export function looksLikeUrl(value: string): boolean { @@ -5,7 +7,7 @@ export function looksLikeUrl(value: string): boolean { } export function looksLikePath(value: string): boolean { - return /^file:\/\//i.test(value) || /^(?:\/|\.{1,2}\/|~\/).+/.test(value) + return /^file:\/\//i.test(value) || /^(?:\/|\.{1,2}\/|~\/).+/.test(value) || isWindowsAbsolutePath(value) } export function isPreviewableTarget(target: string): boolean { @@ -17,7 +19,7 @@ export function isPreviewableTarget(target: string): boolean { return Boolean( target && (/^file:\/\//i.test(target) || - /^(?:\/|\.{1,2}\/|~\/).+\.html?$/i.test(target) || + (looksLikePath(target) && /\.html?$/i.test(target)) || /^https?:\/\/(?:localhost|127\.0\.0\.1|0\.0\.0\.0|\[::1\])/i.test(target)) ) } diff --git a/apps/desktop/src/lib/local-preview.test.ts b/apps/desktop/src/lib/local-preview.test.ts index 9274ce9faa..1dcdc18096 100644 --- a/apps/desktop/src/lib/local-preview.test.ts +++ b/apps/desktop/src/lib/local-preview.test.ts @@ -148,6 +148,19 @@ describe('remote HTML previews', () => { expect(localPreviewTarget('/tmp/report\\draft.html')?.url).toBe('file:///tmp/report%5Cdraft.html') }) + // #85132: a Windows absolute path is absolute; joining it onto cwd made + // `/repo/C:\\Users\\...` which no filesystem has. + it.each(['C:\\Users\\me\\report.html', 'C:/Users/me/report.html', '\\\\server\\share\\report.html'])( + 'treats Windows absolute path %s as absolute instead of joining it onto cwd', + raw => { + expect(localPreviewTarget(raw, '/repo')).toMatchObject({ label: 'report.html', path: raw, previewKind: 'html' }) + } + ) + + it('still joins relative paths onto cwd', () => { + expect(localPreviewTarget('out/report.html', '/repo')?.path).toBe('/repo/out/report.html') + }) + it('preserves POSIX double-slash file paths', () => { expect(localPreviewTarget('//srv/share/report #1?.html')?.url).toBe('file:////srv/share/report%20%231%3F.html') }) diff --git a/apps/desktop/src/lib/local-preview.ts b/apps/desktop/src/lib/local-preview.ts index 15f0044216..9eb7ff8a26 100644 --- a/apps/desktop/src/lib/local-preview.ts +++ b/apps/desktop/src/lib/local-preview.ts @@ -1,6 +1,7 @@ import DOMPurify from 'dompurify' import { isDesktopFsRemoteMode, readDesktopFileDataUrl, readDesktopFileText } from '@/lib/desktop-fs' +import { isWindowsAbsolutePath } from '@/lib/path-compare' import type { PreviewTarget } from '@/store/preview' const HTML_EXTENSIONS = new Set(['.htm', '.html']) @@ -221,7 +222,7 @@ export function localPreviewTarget(rawTarget: string, cwd?: string | null): Prev } catch { path = raw.replace(/^file:\/\//i, '') } - } else if (!raw.startsWith('/') && cwd) { + } else if (!raw.startsWith('/') && !isWindowsAbsolutePath(raw) && cwd) { path = joinPath(cwd, raw) } diff --git a/apps/desktop/src/lib/path-compare.test.ts b/apps/desktop/src/lib/path-compare.test.ts index 30e3184f88..4ef6f1180d 100644 --- a/apps/desktop/src/lib/path-compare.test.ts +++ b/apps/desktop/src/lib/path-compare.test.ts @@ -1,6 +1,16 @@ import { describe, expect, it } from 'vitest' -import { cleanPath, comparisonPath, isUnderPath } from './path-compare' +import { cleanPath, comparisonPath, isUnderPath, isWindowsAbsolutePath } from './path-compare' + +describe('isWindowsAbsolutePath', () => { + it.each(['C:\\Users\\me', 'd:/work', '\\\\server\\share'])('accepts %s', path => { + expect(isWindowsAbsolutePath(path)).toBe(true) + }) + + it.each(['/home/me', './out', 'out\\report.html', 'C:relative', 'https://x.com'])('rejects %s', path => { + expect(isWindowsAbsolutePath(path)).toBe(false) + }) +}) describe('cleanPath', () => { it('unifies separators and drops trailing slashes', () => { diff --git a/apps/desktop/src/lib/path-compare.ts b/apps/desktop/src/lib/path-compare.ts index e477a578cf..533893bd7a 100644 --- a/apps/desktop/src/lib/path-compare.ts +++ b/apps/desktop/src/lib/path-compare.ts @@ -5,6 +5,10 @@ * case. Compare through these rather than `===` / `startsWith`. */ +/** Windows drive (`C:\\`, `C:/`) or UNC (`\\\\server\\share`) absolute path. + * Such a path never gets joined onto a cwd. */ +export const isWindowsAbsolutePath = (path: string): boolean => /^(?:[A-Za-z]:[\\/]|\\\\)/.test(path) + /** POSIX-style spelling: one separator, no trailing slash. */ export const cleanPath = (path: string): string => path.trim().replace(/\\/g, '/').replace(/\/+$/, '') || '/' diff --git a/apps/desktop/src/lib/preview-targets.test.ts b/apps/desktop/src/lib/preview-targets.test.ts index 3a51c29ab0..ac738f254d 100644 --- a/apps/desktop/src/lib/preview-targets.test.ts +++ b/apps/desktop/src/lib/preview-targets.test.ts @@ -1,6 +1,24 @@ import { describe, expect, it } from 'vitest' -import { extractPreviewTargets, previewTargetFromMarkdownHref, stripPreviewTargets } from './preview-targets' +import { + extractPreviewTargets, + previewName, + previewTargetFromMarkdownHref, + stripPreviewTargets +} from './preview-targets' + +describe('previewName', () => { + // #85132: `new URL('C:\\...')` parses the drive letter as a scheme, which + // labelled the tag with the whole backslash path. + it.each([ + 'C:\\Users\\me\\report.html', + 'C:/Users/me/report.html', + '\\\\server\\share\\report.html', + '/Users/me/report.html' + ])('labels %s by its file name', target => { + expect(previewName(target)).toBe('report.html') + }) +}) describe('preview target detection', () => { it('does not infer preview targets from raw paths or URLs', () => { diff --git a/apps/desktop/src/lib/preview-targets.ts b/apps/desktop/src/lib/preview-targets.ts index 8e16b3cc24..8f472241ab 100644 --- a/apps/desktop/src/lib/preview-targets.ts +++ b/apps/desktop/src/lib/preview-targets.ts @@ -1,3 +1,5 @@ +import { isWindowsAbsolutePath } from '@/lib/path-compare' + const PREVIEW_MARKDOWN_RE = /\[Preview:[^\]]+\]\((?#preview[:/][^)]+)\)/gi export function stripPreviewTargets(text: string): string { @@ -37,6 +39,11 @@ export function previewTargetFromMarkdownHref(href?: string): string | null { } export function previewName(target: string): string { + // `new URL('C:\\...')` would read the drive letter as a URL scheme. + if (isWindowsAbsolutePath(target)) { + return target.split(/[\\/]/).filter(Boolean).pop() || target + } + try { const url = new URL(target)