From 036d0faea7957b6e34bdbab321805036cfbaca53 Mon Sep 17 00:00:00 2001 From: usrnk1 <7547651+usrnk1@users.noreply.github.com> Date: Wed, 21 Oct 2026 12:53:01 +0200 Subject: [PATCH] fix(app): prevent inline file links flashing on tab switches (#57725) --- .../app/e2e/regression/timeline-cache.spec.ts | 102 +++++++++++++++++- packages/app/e2e/utils/mock-server.ts | 13 ++- packages/gui-extensions/src/file/renderer.tsx | 67 ++++++++++-- .../component-tests/markdown.fixture.tsx | 11 +- .../component-tests/markdown.spec.ts | 31 +++++- .../session-ui/src/components/markdown.tsx | 25 ++++- 6 files changed, 229 insertions(+), 20 deletions(-) diff --git a/packages/app/e2e/regression/timeline-cache.spec.ts b/packages/app/e2e/regression/timeline-cache.spec.ts index 337f9831036..83f96e7b524 100644 --- a/packages/app/e2e/regression/timeline-cache.spec.ts +++ b/packages/app/e2e/regression/timeline-cache.spec.ts @@ -2,9 +2,10 @@ import { expect, test, type Page } from "@playwright/test" import type { SessionMessageInfo } from "@opencode/client/promise" import { timelinePresets } from "@opencode/session-ui/timeline/detail" import { expected, messages } from "../utils/markdown-sessions" -import { expectPath, seed, sessionHref, type SeedInput } from "../utils/app" +import { expectPath, holdRoute, seed, sessionHref, type SeedInput } from "../utils/app" import type { MockServerConfig } from "../utils/mock-server" import { fixture, installTimelineSettings, mockStressTimeline } from "../utils/session-fixture" +import { mockWorkspace } from "../utils/workspace" test.use({ viewport: { width: 1440, height: 900 }, serviceWorkers: "block" }) @@ -98,6 +99,105 @@ test("paints cold and warm session tabs at the latest message", async ({ page }) expect(await warm.stop()).toBe(0) }) +test("restores verified inline paths before painting after a workspace tab switch", async ({ page }) => { + const directory = "C:/OpenCode/InlinePaths" + const other = "C:/OpenCode/OtherInlinePaths" + const sourceID = "ses_inline_source" + const targetID = "ses_inline_target" + const queries: { query: string; directory: string }[] = [] + const existing = new Set([directory]) + await page.clock.setFixedTime(new Date("2026-10-21T00:00:00Z")) + await mockWorkspace(page, { + name: "InlinePaths", + directory, + project: { sandboxes: [other] }, + sessions: [ + { id: sourceID, title: "Known path" }, + { id: targetID, title: "Missing path", directory: other }, + ], + pageMessages: (id) => ({ + items: [ + { id: `user_${id}`, type: "user", text: "Check the file", time: { created: 1 } }, + { + id: `answer_${id}`, + type: "assistant", + agent: "build", + model: { id: "test", providerID: "opencode" }, + time: { created: 2, completed: 3 }, + content: [{ type: "text", text: "Inspect `src/file.ts`." }], + }, + ], + }), + findFiles: (input) => { + queries.push({ query: input.query, directory: input.directory }) + + return existing.has(input.directory) ? ["src/file.ts"] : [] + }, + }) + await page.goto(sessionHref(sourceID)) + const source = page.locator(`[data-timeline-part-id="answer_${sourceID}:text:0"] code`) + await expect(source).toHaveAttribute("data-inline-code-kind", "path") + const original = await source.elementHandle() + await tab(page, targetID).click() + const target = page.locator(`[data-timeline-part-id="answer_${targetID}:text:0"] code`) + await expect(target).toHaveText("src/file.ts") + await expect.poll(() => queries.some((query) => query.directory === other)).toBe(true) + await expect(target).not.toHaveAttribute("data-inline-code-kind") + expect(await original!.evaluate((node) => node.isConnected)).toBe(false) + + const insertion = await page.evaluateHandle((id) => { + type Insertion = { kind?: string | null } + + const state: Insertion = {} + + const observer = new MutationObserver(() => { + const code = document.querySelector(`[data-timeline-part-id="answer_${id}:text:0"] code`) + + if (!code) return + state.kind = code.getAttribute("data-inline-code-kind") + observer.disconnect() + }) + + observer.observe(document, { childList: true, subtree: true }) + + return state + }, sourceID) + + const count = queries.length + const held = await holdRoute(page, /\/api\/fs\/find\?/) + + try { + await tab(page, sourceID).click() + await expect(source).toBeVisible() + // The other workspace disposes the old timeline, so this must exercise a real Markdown remount. + expect(await source.evaluate((node, original) => node === original, original)).toBe(false) + expect(await insertion.evaluate((state) => state.kind)).toBe("path") + expect(held.requests).toHaveLength(0) + expect(queries).toHaveLength(count) + } finally { + held.release() + await insertion.dispose() + } + + // Expired positives keep their styling while revalidation is pending; negatives must discover new files. + await tab(page, targetID).click() + await expect(target).toBeVisible() + await page.clock.setFixedTime(new Date("2026-10-21T00:01:01Z")) + const refresh = await holdRoute(page, /\/api\/fs\/find\?/) + + try { + await tab(page, sourceID).click() + await refresh.arrived + await expect(source).toHaveAttribute("data-inline-code-kind", "path") + } finally { + refresh.release() + } + + existing.add(other) + await tab(page, targetID).click() + await expect(target).toHaveAttribute("data-inline-code-kind", "path") +}) + test("scrolls within a long answer without mounting unrelated history", async ({ page }) => { await openTimeline(page, messages[fixture.sourceID]) const answer = page.locator(`[data-timeline-part-id="${expected[fixture.sourceID].answerID}"]`) diff --git a/packages/app/e2e/utils/mock-server.ts b/packages/app/e2e/utils/mock-server.ts index 22fcf004ffe..9ea526ba93e 100644 --- a/packages/app/e2e/utils/mock-server.ts +++ b/packages/app/e2e/utils/mock-server.ts @@ -136,7 +136,7 @@ export interface MockServerConfig { fileList?: (path: string) => unknown[] | Promise fileContent?: (path: string) => MockFileContent | Promise // Paths become directory entries. - findFiles?: (input: { query: string; dirs?: string; limit?: number }) => unknown[] + findFiles?: (input: { query: string; dirs?: string; limit?: number; directory: string }) => unknown[] sessionStatus?: Resolvable> inbox?: unknown[] | (() => unknown[]) onPrompt?: (input: { sessionID: string; body: Schema.JsonObject }) => void @@ -1083,18 +1083,23 @@ function mockHandlers( fsFind: (ctx) => Effect.promise(() => Promise.resolve( - config.findFiles?.({ query: ctx.query.query ?? "", dirs: ctx.query.type, limit: ctx.query.limit }), + config.findFiles?.({ + query: ctx.query.query ?? "", + dirs: ctx.query.type, + limit: ctx.query.limit, + directory: requestDirectory(config, ctx.request), + }), ), ).pipe( Effect.map((entries) => ({ - location: location(config), + location: location(config, requestDirectory(config, ctx.request)), data: Array.isArray(entries) ? entries.map((entry) => Predicate.isString(entry) ? { name: entry.split(/[\\/]/).at(-1) ?? entry, path: entry, - absolute: `${config.directory}/${entry}`, + absolute: `${requestDirectory(config, ctx.request)}/${entry}`, type: "directory", ignored: false, } diff --git a/packages/gui-extensions/src/file/renderer.tsx b/packages/gui-extensions/src/file/renderer.tsx index 3b3d5302753..6f3427feac4 100644 --- a/packages/gui-extensions/src/file/renderer.tsx +++ b/packages/gui-extensions/src/file/renderer.tsx @@ -30,6 +30,7 @@ import { type PanelTab, type MountedSession, type SessionScreen, + type ServerRef, type Setup, onIdle, } from "../sdk" @@ -61,6 +62,8 @@ type Handoff = { sessions: Record> } type StyleLoad = { loaded?: Promise } +type LinkCheck = { result: boolean | Promise; expires: number; known?: true } + const setup: Setup = (ctx) => { const sessions = ctx.sessions const layout = ctx.layout @@ -516,24 +519,74 @@ const setup: Setup = (ctx) => { return files.resolve(resolveArtifactPath(dir, value) ?? value) } + // Screens survive tab switches, unlike Markdown and MountedSession objects. Weak client keys also drop results + // from an old server connection without retaining its controller. + const linkChecks = new WeakMap>>() + // Messages style a path as a link only when this says the file exists. const linkExists = (link: Link): boolean | Promise => { const session = sessions.current() - const files = ctx.screen.current()?.file + const screen = ctx.screen.current() + const files = screen?.file - if (!session || !files || (link.session && link.session.key !== session.key)) return false + if (!session || !screen || !files || (link.session && link.session.key !== session.key)) return false - return import("./resolve-link") - .then(({ checkFileLinkExists, parseFileLink }) => { - if (link.base === undefined) return checkFileLinkExists({ files, href: link.href, signal: ctx.signal }) + const clients = linkChecks.get(screen) ?? new WeakMap>() + linkChecks.set(screen, clients) + const client = session.server.client + const checks = clients.get(client) ?? new Map() + clients.set(client, checks) + const root = files.root + const key = JSON.stringify([session.key, root, link.base ?? null, link.href]) + const cached = checks.get(key) + + if (cached && cached.expires > Date.now()) return cached.known ?? cached.result + + const current = () => + !ctx.signal.aborted && + sessions.current()?.key === session.key && + files.root === root && + session.server.client === client + + const pending = import("./resolve-link") + .then(async ({ checkFileLinkExists, parseFileLink }) => { + if (!current()) return false // A link in a previewed document opens relative to it, so only that path counts. const path = parseFileLink(link.href).path - const direct = path.startsWith("//") || path.startsWith("~") ? undefined : resolve(files, path, link.base) - return direct ? files.exists(direct) : false + const direct = + link.base === undefined || path.startsWith("//") || path.startsWith("~") + ? undefined + : resolve(files, path, link.base) + + const exists = await (link.base === undefined + ? checkFileLinkExists({ files, href: link.href, signal: ctx.signal }) + : direct + ? files.exists(direct) + : false) + + if (!current()) return false + + if (checks.get(key)?.result === pending) + checks.set(key, { result: exists, expires: Date.now() + (exists ? 60_000 : 5_000) }) + + return exists }) .catch(() => false) + .finally(() => { + // Aborted, failed or no-longer-current checks are retryable, not cached negatives. + if (checks.get(key)?.result === pending) checks.delete(key) + }) + + // Revalidate known paths without bringing back their plain-code flash. Expired negatives await a fresh check. + const known = cached?.result === true ? true : undefined + checks.delete(key) + checks.set(key, { result: pending, expires: Infinity, known }) + + while (checks.size > 512) checks.delete(checks.keys().next().value!) + + return known ?? pending } // Opens files the agent references as side panel tabs, inside or outside the workspace, or as diff --git a/packages/session-ui/component-tests/markdown.fixture.tsx b/packages/session-ui/component-tests/markdown.fixture.tsx index 24f1adebba9..80e6cddbf19 100644 --- a/packages/session-ui/component-tests/markdown.fixture.tsx +++ b/packages/session-ui/component-tests/markdown.fixture.tsx @@ -28,6 +28,7 @@ export async function mountMarkdown(options: { streaming?: boolean cached?: boolean images?: boolean + synchronousFiles?: boolean }) { if (options.cached) await preloadMarkdown(options.text, "markdown-test") const host = document.createElement("div") @@ -57,7 +58,9 @@ export async function mountMarkdown(options: { readImage={(path, signal) => options.images ? readLocalImage(api, "C:/project", path, signal) : Promise.resolve(undefined) } - localFileExists={(path) => Promise.resolve(existingFiles.has(path))} + localFileExists={(path) => + options.synchronousFiles ? existingFiles.has(path) : Promise.resolve(existingFiles.has(path)) + } > ) }, host) + + // Capture before Promise callbacks run: eventual assertions cannot catch a flash on insertion. + return Array.from(host.querySelectorAll(":not(pre) > code"), (node) => ({ + text: node.textContent, + kind: node.dataset.inlineCodeKind ?? null, + })) } diff --git a/packages/session-ui/component-tests/markdown.spec.ts b/packages/session-ui/component-tests/markdown.spec.ts index 8067f48afe7..6ed9f4c2374 100644 --- a/packages/session-ui/component-tests/markdown.spec.ts +++ b/packages/session-ui/component-tests/markdown.spec.ts @@ -127,6 +127,30 @@ story("keeps Markdown sanitization and link protections after Mermaid renders", expect(result.afterInvalid).toBe(result.before) }) +for (const cached of [false, true]) { + story(`decorates known inline paths before insertion with cached Markdown ${cached}`, async ({ page }) => { + const initial = await page.evaluate( + async ({ fixture, cached }) => { + const { mountMarkdown } = await import(fixture) + + return mountMarkdown({ + text: "`src/file.ts` and `src/file.ts` and `missing.ts` and [`src/file.ts`](https://example.com)", + cached, + synchronousFiles: true, + }) + }, + { fixture, cached }, + ) + + expect(initial).toEqual([ + { text: "src/file.ts", kind: "path" }, + { text: "src/file.ts", kind: "path" }, + { text: "missing.ts", kind: null }, + { text: "src/file.ts", kind: null }, + ]) + }) +} + story("mounts cached completed Markdown with sanitized HTML and decorations", async ({ page }) => { await page.evaluate( async ({ fixture, text }) => { @@ -546,7 +570,7 @@ story("renders the trailing chunk while the stream is paused", async ({ page }) story("keeps live elements and selection when a stream completes and later changes", async ({ page }) => { await page.evaluate(async (fixture) => { const { mountMarkdown } = await import(fixture) - await mountMarkdown({ text: "Hello **world**", streaming: true }) + await mountMarkdown({ text: "Hello **world**", streaming: true, synchronousFiles: true }) }, fixture) const harness = page.getByTestId("markdown-fixture") const markdown = harness.locator('[data-component="markdown"]') @@ -571,8 +595,9 @@ story("keeps live elements and selection when a stream completes and later chang await expect(markdown).toHaveAttribute("data-markdown-ready", "") await expect(paragraph).toHaveAttribute("data-retained", "true") expect(await page.evaluate(() => window.getSelection()?.toString())).toBe("world") - await harness.getByLabel("Markdown text").fill("Changed **content**") - await expect(paragraph).toHaveText("Changed content") + await harness.getByLabel("Markdown text").fill("Changed **content** in `new/file.ts` and `new/file.ts`") + await expect(paragraph).toHaveText("Changed content in new/file.ts and new/file.ts") + await expect(markdown.locator('code[data-inline-code-kind="path"]')).toHaveText(["new/file.ts", "new/file.ts"]) await expect(paragraph).toHaveAttribute("data-retained", "true") await expect(markdown.locator("[data-markdown-word]")).toHaveCount(0) await harness.getByRole("button", { name: "Toggle Markdown" }).click() diff --git a/packages/session-ui/src/components/markdown.tsx b/packages/session-ui/src/components/markdown.tsx index fd930d280ef..709a238c4ad 100644 --- a/packages/session-ui/src/components/markdown.tsx +++ b/packages/session-ui/src/components/markdown.tsx @@ -397,7 +397,7 @@ function markInlineCode( live: boolean, localFileExists?: MarkdownLocalFileExists, ) { - const checked = new Set() + const checked = new Map>() for (const code of source.querySelectorAll(":not(pre) > code")) { delete code.dataset.inlineCodeKind @@ -407,10 +407,27 @@ function markInlineCode( if (kind === "url") code.dataset.inlineCodeKind = kind // Code inside a link already goes where the link does. - if (kind !== "path" || live || !localFileExists || code.closest("a") || checked.has(text)) continue + if (kind !== "path" || live || !localFileExists || code.closest("a")) continue - checked.add(text) - void Promise.resolve(localFileExists(text)) + const previous = checked.get(text) + + if (previous !== undefined) { + if (previous === true) code.dataset.inlineCodeKind = "path" + + continue + } + + const exists = localFileExists(text) + checked.set(text, exists) + + // Decorate the source before it is inserted or copied into a live Markdown renderer. + if (exists === true || exists === false) { + if (exists) code.dataset.inlineCodeKind = "path" + + continue + } + + void exists .then((exists) => { if (!exists) return target.querySelectorAll(":not(pre) > code").forEach((node) => {