diff --git a/packages/app/e2e/browser/file-editing.spec.ts b/packages/app/e2e/browser/file-editing.spec.ts index da729381d2..0d63336d59 100644 --- a/packages/app/e2e/browser/file-editing.spec.ts +++ b/packages/app/e2e/browser/file-editing.spec.ts @@ -105,6 +105,37 @@ async function seedAgentWithFileLink(input: LinkedFile) { } test.describe("CodeMirror workspace file editing", () => { + test("opens explicit screenshot links and explains missing files", async ({ page }) => { + const session = await seedMockAgentWorkspace({ + repoPrefix: "screenshot-markdown-link-", + title: "Screenshot file links", + initialPrompt: "Show screenshots", + featureValues: { + mockAssistantResponse: + "[Capture](.dev/tablet-review/capture.png) [Missing capture](.dev/tablet-review/missing.png)", + }, + }); + try { + await mkdir(path.join(session.cwd, ".dev/tablet-review"), { recursive: true }); + await writeFile(path.join(session.cwd, ".dev/tablet-review/capture.png"), RED_PIXEL); + await openAgentRoute(page, session); + await test.step("Open the linked screenshot", async () => { + await openAssistantMarkdownLink(page, "Capture"); + await expectFileTabOpen(page, ".dev/tablet-review/capture.png"); + await expect(page.getByTestId("image-file-preview-canvas")).toBeVisible(); + }); + await test.step("Explain why the missing screenshot cannot open", async () => { + await returnToAssistant(page, session.agentId); + await openAssistantMarkdownLink(page, "Missing capture"); + await expectFileTabOpen(page, ".dev/tablet-review/missing.png"); + await expect(page.getByText(/ENOENT/)).toBeVisible(); + await expect(page.getByRole("button", { name: "Retry", exact: true })).toBeVisible(); + }); + } finally { + await session.cleanup(); + } + }); + test("opens the non-ASCII file named by an assistant markdown link", async ({ page }) => { const session = await seedMockAgentWorkspace({ repoPrefix: "encoded-markdown-link-", diff --git a/packages/app/src/assistant-file-links/parse.ts b/packages/app/src/assistant-file-links/parse.ts index c195a277cf..4fb41a72ca 100644 --- a/packages/app/src/assistant-file-links/parse.ts +++ b/packages/app/src/assistant-file-links/parse.ts @@ -23,6 +23,8 @@ const INLINE_PAREN_LINE_SUFFIX = /^(.+?)\(([0-9]+)(?:,[0-9]+)?(?:-([0-9]+)(?:,[0 const INLINE_WORD_LINE_SUFFIX = /^(.+?)\s+lines?\s+([0-9]+)(?:-([0-9]+))?$/i; export interface AssistantHrefParseOptions { workspaceRoot?: string; + /** Authored links identify a file; only inferred text needs filename heuristics. */ + explicitLink?: boolean; /** * True only when `value` is a markdown href — the renderer percent-encodes * non-ASCII and space characters, so the path must be decoded. Raw path @@ -30,8 +32,6 @@ export interface AssistantHrefParseOptions { * send a real file named `my%20file.md` to `my file.md`. */ decodeHref?: boolean; - /** Explicit Markdown links can name nested folders without a filename extension. */ - allowExtensionlessNestedPaths?: boolean; } export type AssistantFileLinkClassification = @@ -236,7 +236,7 @@ export function classifyAssistantFileLink( return null; } - if (isAmbiguousWorkspaceCandidate(trimmed, target, options)) { + if (!options.explicitLink && isAmbiguousWorkspaceCandidate(trimmed, target, options)) { return { kind: "ambiguousFileCandidate", target, @@ -280,7 +280,7 @@ export function parseAssistantFileLink( const relativeTarget = parseWorkspaceRelativeFileLink(trimmed, { workspaceRoot: options.workspaceRoot, decodeHref: options.decodeHref, - allowExtensionlessNestedPaths: options.allowExtensionlessNestedPaths, + explicitLink: options.explicitLink, }); if (relativeTarget) { return relativeTarget; @@ -414,7 +414,7 @@ function parseWorkspaceRelativeFileLink( function parseLocalPathParts( value: string, - options: Pick = {}, + options: Pick = {}, ): { path: string; lines: Pick; @@ -440,7 +440,7 @@ function parseLocalPathParts( if (options.decodeHref) { pathPart = safeDecodeURIComponent(pathPart); } - if (!isPlausibleAssistantLocalPath(pathPart, options)) { + if (!options.explicitLink && !isPlausibleAssistantLocalPath(pathPart)) { return null; } return { @@ -461,7 +461,7 @@ function parseLocalPathParts( decodedBeforeHash = safeDecodeURIComponent(beforeHash); } const plausiblePath = parseInlinePathToken(decodedBeforeHash)?.path ?? decodedBeforeHash; - if (!isPlausibleAssistantLocalPath(plausiblePath, options)) { + if (!options.explicitLink && !isPlausibleAssistantLocalPath(plausiblePath)) { return null; } @@ -586,10 +586,7 @@ function getHeuristicLocalPath(value: string): string | null { return beforeHash; } -function isPlausibleAssistantLocalPath( - pathValue: string, - options: Pick = {}, -): boolean { +function isPlausibleAssistantLocalPath(pathValue: string): boolean { const normalized = normalizePathToken(pathValue); if (!normalized) { return false; @@ -613,10 +610,7 @@ function isPlausibleAssistantLocalPath( if (segments.length > 1) { const lastSegment = segments[segments.length - 1]; - return ( - !isDomainLikePathSegment(firstSegment) && - (options.allowExtensionlessNestedPaths === true || isPlausibleAssistantFileName(lastSegment)) - ); + return !isDomainLikePathSegment(firstSegment) && isPlausibleAssistantFileName(lastSegment); } return isPlausibleAssistantFileName(firstSegment); diff --git a/packages/app/src/assistant-file-links/resolver.test.ts b/packages/app/src/assistant-file-links/resolver.test.ts index 854c358627..96884c21c7 100644 --- a/packages/app/src/assistant-file-links/resolver.test.ts +++ b/packages/app/src/assistant-file-links/resolver.test.ts @@ -50,7 +50,7 @@ it.each([{ markup: "linkify" }, { sourceInfo: "auto" }])( }, ); -it.each(["example.com/guides", "docs/guides?view=all"])( +it.each(["docs/guides?view=all"])( "does not turn URL-like hrefs into workspace paths: %s", (href) => { expect(classifyForResolution({ href }, CONTEXT)).toEqual({ @@ -60,6 +60,21 @@ it.each(["example.com/guides", "docs/guides?view=all"])( }, ); +it.each([ + ".dev/tablet-review/tablet-agent-draft.png", + "images/capture.jpg", + "downloads/archive.custom", + "assets/LICENSE", + "example.com/guides", + "capture.png", + "missing.png", +])("opens explicit file link %s without filename guessing", (href) => { + expect(classifyForResolution({ href }, CONTEXT)).toMatchObject({ + kind: "resolved", + value: { kind: "file", target: { path: `${CONTEXT.workspaceRoot}/${href}` } }, + }); +}); + it("opens the existing non-ASCII file named by a rendered markdown link", async () => { const workspaceRoot = await mkdtemp(path.join(tmpdir(), "assistant-file-link-")); try { diff --git a/packages/app/src/assistant-file-links/resolver.ts b/packages/app/src/assistant-file-links/resolver.ts index 59057ba0ca..be012077a3 100644 --- a/packages/app/src/assistant-file-links/resolver.ts +++ b/packages/app/src/assistant-file-links/resolver.ts @@ -117,7 +117,7 @@ export function classifyForResolution( const classification = classifyAssistantFileLink(token, { workspaceRoot: context.workspaceRoot, decodeHref: tokenInfo.fromHref, - allowExtensionlessNestedPaths: tokenInfo.fromHref && !isLinkifiedSource(source), + explicitLink: source.sourceType !== "inline-code" && !isLinkifiedSource(source), }); if (!classification) { return { kind: "resolved", value: { kind: "ignored" } };