From 60e105161c6fd307ff4a5baabaeb6b259544e97f Mon Sep 17 00:00:00 2001 From: usrnk1 <7547651+usrnk1@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:43:31 +0200 Subject: [PATCH] fix(app): resolve inline file links before cold transcript reveal (#57801) --- .../app/e2e/regression/timeline-cache.spec.ts | 70 ++++++++++++++ .../component-tests/markdown.fixture.tsx | 6 +- .../component-tests/markdown.spec.ts | 96 +++++++++++++++++++ .../session-ui/src/components/markdown.tsx | 77 +++++++++++---- 4 files changed, 230 insertions(+), 19 deletions(-) diff --git a/packages/app/e2e/regression/timeline-cache.spec.ts b/packages/app/e2e/regression/timeline-cache.spec.ts index 83f96e7b524..6d378f5ba65 100644 --- a/packages/app/e2e/regression/timeline-cache.spec.ts +++ b/packages/app/e2e/regression/timeline-cache.spec.ts @@ -99,6 +99,76 @@ test("paints cold and warm session tabs at the latest message", async ({ page }) expect(await warm.stop()).toBe(0) }) +test("reloads a transcript only after inline file links have their final styling", async ({ page }) => { + const sessionID = "ses_cold_inline_paths" + await mockWorkspace(page, { + name: "ColdInlinePaths", + sessions: [{ id: sessionID, title: "Cold inline paths" }], + findFiles: ({ query }) => (query === "src/file.ts" ? ["src/file.ts"] : []), + pageMessages: () => ({ + items: [ + { id: "user_cold_paths", type: "user", text: "Review these files", time: { created: 1 } }, + { + id: "answer_cold_paths", + type: "assistant", + agent: "build", + model: { id: "test", providerID: "opencode" }, + time: { created: 2, completed: 3 }, + content: [ + { + type: "text", + text: "| Area | Files |\n| --- | --- |\n| App | `src/file.ts`, `src/file.ts` |\n| Missing | `missing/file.ts` |", + }, + ], + }, + ], + }), + }) + await page.goto(sessionHref(sessionID)) + const answer = page.locator('[data-timeline-part-id="answer_cold_paths:text:0"]') + const content = page.locator("[data-timeline-virtual-content]") + await expect(answer.locator('code[data-inline-code-kind="path"]')).toHaveCount(2) + await expect(content).toHaveCSS("visibility", "visible") + const held = await holdRoute(page, /\/api\/fs\/find\?/) + + try { + await page.reload({ waitUntil: "domcontentloaded" }) + await held.arrived + await expect(answer.getByRole("table", { includeHidden: true })).toBeAttached() + await expect(content).toHaveCSS("visibility", "hidden") + const editor = page.locator('[data-component="composer-editor"]') + await expect(editor).toBeEditable() + await editor.fill("The composer still works") + await expect(editor).toHaveText("The composer still works") + + const reveal = await content.evaluateHandle((root) => { + type PathReveal = { kinds?: (string | null)[] } + + const state: PathReveal = {} + + const observer = new MutationObserver(() => { + if (!root.checkVisibility({ checkVisibilityCSS: true })) return + state.kinds = [...root.querySelectorAll("code")].map((code) => code.getAttribute("data-inline-code-kind")) + observer.disconnect() + }) + + observer.observe(root, { attributes: true, attributeFilter: ["style"] }) + + return state + }) + + held.release() + await expect(content).toHaveCSS("visibility", "visible") + expect(await reveal.evaluate((state) => state.kinds)).toEqual(["path", "path", null]) + await reveal.dispose() + await expect(answer.locator("code")).toHaveText(["src/file.ts", "src/file.ts", "missing/file.ts"]) + await expect(answer.locator('code[data-inline-code-kind="path"]')).toHaveText(["src/file.ts", "src/file.ts"]) + await expect(answer.getByText("missing/file.ts", { exact: true })).not.toHaveAttribute("data-inline-code-kind") + } finally { + held.release() + } +}) + test("restores verified inline paths before painting after a workspace tab switch", async ({ page }) => { const directory = "C:/OpenCode/InlinePaths" const other = "C:/OpenCode/OtherInlinePaths" diff --git a/packages/session-ui/component-tests/markdown.fixture.tsx b/packages/session-ui/component-tests/markdown.fixture.tsx index 80e6cddbf19..0b6a2c13e8e 100644 --- a/packages/session-ui/component-tests/markdown.fixture.tsx +++ b/packages/session-ui/component-tests/markdown.fixture.tsx @@ -3,7 +3,7 @@ import { render } from "solid-js/web" import { DialogProvider } from "@opencode/ui/context/dialog" import { Markdown } from "../src/components/markdown" import { preloadMarkdown } from "../src/components/markdown-cache" -import { MarkdownProvider } from "../src/context/markdown" +import { MarkdownProvider, type MarkdownLocalFileExists } from "../src/context/markdown" import { OpenCode } from "@opencode/client/promise" import { readLocalImage } from "../../app/src/runtime/server/image" @@ -29,6 +29,7 @@ export async function mountMarkdown(options: { cached?: boolean images?: boolean synchronousFiles?: boolean + localFileExists?: MarkdownLocalFileExists }) { if (options.cached) await preloadMarkdown(options.text, "markdown-test") const host = document.createElement("div") @@ -59,7 +60,8 @@ export async function mountMarkdown(options: { options.images ? readLocalImage(api, "C:/project", path, signal) : Promise.resolve(undefined) } localFileExists={(path) => - options.synchronousFiles ? existingFiles.has(path) : Promise.resolve(existingFiles.has(path)) + options.localFileExists?.(path) ?? + (options.synchronousFiles ? existingFiles.has(path) : Promise.resolve(existingFiles.has(path))) } > diff --git a/packages/session-ui/component-tests/markdown.spec.ts b/packages/session-ui/component-tests/markdown.spec.ts index 6ed9f4c2374..e3ae3fa2c0a 100644 --- a/packages/session-ui/component-tests/markdown.spec.ts +++ b/packages/session-ui/component-tests/markdown.spec.ts @@ -151,6 +151,102 @@ for (const cached of [false, true]) { }) } +story("does not report completed Markdown ready until all inline path checks settle", async ({ page }) => { + const checks = await page.evaluateHandle(async (fixture) => { + const { mountMarkdown } = await import(fixture) + const existing = Promise.withResolvers() + const missing = Promise.withResolvers() + const requested: string[] = [] + await mountMarkdown({ + text: "`src/file.ts` and `src/file.ts` and `missing/file.ts`", + localFileExists: (path: string) => { + requested.push(path) + + return path === "src/file.ts" ? existing.promise : missing.promise + }, + }) + + return { requested, existing: () => existing.resolve(true), missing: () => missing.reject(new Error("Offline")) } + }, fixture) + + const markdown = page.getByTestId("markdown-fixture").locator('[data-component="markdown"]') + await expect(markdown.locator("code")).toHaveCount(3) + expect(await checks.evaluate((state) => state.requested)).toEqual(["src/file.ts", "missing/file.ts"]) + await expect(markdown).not.toHaveAttribute("data-markdown-ready") + await checks.evaluate((state) => state.existing()) + await expect(markdown.locator('code[data-inline-code-kind="path"]')).toHaveCount(2) + await expect(markdown).not.toHaveAttribute("data-markdown-ready") + await checks.evaluate((state) => state.missing()) + await expect(markdown).toHaveAttribute("data-markdown-ready", "") + await expect(markdown.getByText("missing/file.ts", { exact: true })).not.toHaveAttribute("data-inline-code-kind") + await checks.dispose() +}) + +story("ignores stale inline path results when completed Markdown is replaced", async ({ page }) => { + const checks = await page.evaluateHandle(async (fixture) => { + const { mountMarkdown } = await import(fixture) + const first = Promise.withResolvers() + const next = Promise.withResolvers() + const requested: string[] = [] + await mountMarkdown({ + text: "Before `src/file.ts`", + cached: true, + localFileExists: (path: string) => { + requested.push(path) + + return requested.length === 1 ? first.promise : next.promise + }, + }) + + return { requested, first: () => first.resolve(true), next: () => next.resolve(false) } + }, fixture) + + const harness = page.getByTestId("markdown-fixture") + const markdown = harness.locator('[data-component="markdown"]') + await expect(markdown.locator("p")).toHaveText("Before src/file.ts") + await expect(markdown).not.toHaveAttribute("data-markdown-ready") + await harness.getByLabel("Markdown text").fill("Replacement `src/file.ts`") + await expect(markdown.locator("p")).toHaveText("Replacement src/file.ts") + expect(await checks.evaluate((state) => state.requested)).toEqual(["src/file.ts", "src/file.ts"]) + await checks.evaluate((state) => state.first()) + await expect(markdown).not.toHaveAttribute("data-markdown-ready") + await checks.evaluate((state) => state.next()) + await expect(markdown).toHaveAttribute("data-markdown-ready", "") + await expect(markdown.locator('code[data-inline-code-kind="path"]')).toHaveCount(0) + await checks.dispose() +}) + +story("keeps streaming Markdown ready while a completed prefix waits for a path check", async ({ page }) => { + const checks = await page.evaluateHandle(async (fixture) => { + const { mountMarkdown } = await import(fixture) + const existing = Promise.withResolvers() + const requested: string[] = [] + await mountMarkdown({ + text: "Completed `src/file.ts`.\n\nStreaming **answer**", + streaming: true, + localFileExists: (path: string) => { + requested.push(path) + + return existing.promise + }, + }) + + return { requested, resolve: () => existing.resolve(true) } + }, fixture) + + const harness = page.getByTestId("markdown-fixture") + const markdown = harness.locator('[data-component="markdown"]') + await expect(markdown.locator("strong")).toHaveText("answer") + await expect(markdown).toHaveAttribute("data-markdown-ready", "") + expect(await checks.evaluate((state) => state.requested)).toEqual(["src/file.ts"]) + await harness.getByLabel("Streaming").uncheck() + await expect(markdown).not.toHaveAttribute("data-markdown-ready") + await checks.evaluate((state) => state.resolve()) + await expect(markdown).toHaveAttribute("data-markdown-ready", "") + await expect(markdown.locator('code[data-inline-code-kind="path"]')).toHaveText("src/file.ts") + await checks.dispose() +}) + story("mounts cached completed Markdown with sanitized HTML and decorations", async ({ page }) => { await page.evaluate( async ({ fixture, text }) => { diff --git a/packages/session-ui/src/components/markdown.tsx b/packages/session-ui/src/components/markdown.tsx index 709a238c4ad..3b66c1292b7 100644 --- a/packages/session-ui/src/components/markdown.tsx +++ b/packages/session-ui/src/components/markdown.tsx @@ -74,6 +74,10 @@ const renderedMarkdown = new WeakMap< { renderer: ReturnType; raw: string } >() +type InlineCodeCheck = { pending?: Promise } + +const inlineCodeChecks = new WeakMap() + function escape(text: string) { return text .replace(/&/g, "&") @@ -222,6 +226,7 @@ function disposeRenderedMarkdown(root: Element) { blocks.forEach((block) => { renderedMarkdown.get(block)?.renderer.dispose() renderedMarkdown.delete(block) + inlineCodeChecks.delete(block) }) } @@ -395,9 +400,13 @@ function markInlineCode( source: HTMLDivElement, target: HTMLDivElement, live: boolean, + signal: AbortSignal, localFileExists?: MarkdownLocalFileExists, ) { + const check: InlineCodeCheck = {} + inlineCodeChecks.set(target, check) const checked = new Map>() + const pending: Promise[] = [] for (const code of source.querySelectorAll(":not(pre) > code")) { delete code.dataset.inlineCodeKind @@ -427,15 +436,25 @@ function markInlineCode( continue } - void exists - .then((exists) => { - if (!exists) return - target.querySelectorAll(":not(pre) > code").forEach((node) => { - if (!node.closest("a") && (node.textContent ?? "").trim() === text) node.dataset.inlineCodeKind = "path" + pending.push( + exists + .then((exists) => { + if (!exists || signal.aborted || inlineCodeChecks.get(target) !== check) return + target.querySelectorAll(":not(pre) > code").forEach((node) => { + if (!node.closest("a") && (node.textContent ?? "").trim() === text) node.dataset.inlineCodeKind = "path" + }) }) - }) - .catch(() => undefined) + .catch(() => undefined), + ) } + + if (pending.length === 0) return + + check.pending = Promise.all(pending).then(() => { + if (inlineCodeChecks.get(target) === check) inlineCodeChecks.delete(target) + }) + + return check.pending } function localLinkTarget(target: EventTarget | null) { @@ -815,10 +834,17 @@ export function Markdown( const sessionLinks = markdown?.sessionTitle const localFileExists = markdown?.localFileExists - content.forEach((block, index) => { - if (renderedBlocks[index] === block && container.children[index] instanceof HTMLDivElement) return - updateBlock(container, index, block, labels, sessionLinks, localFileExists) + const pending = content.flatMap((block, index) => { + const current = container.children[index] + + const check = + renderedBlocks[index] === block && current instanceof HTMLDivElement + ? inlineCodeChecks.get(current)?.pending + : updateBlock(container, index, block, labels, sessionLinks, lifetime.signal, localFileExists) + + return check ? [check] : [] }) + renderedBlocks = content if (sessionLinks) markSessionLinks(container, sessionLinks) @@ -854,7 +880,20 @@ export function Markdown( if (!faviconCleanup) faviconCleanup = setupExternalLinkFavicons(container) container.toggleAttribute("data-local-links", !!markdown?.openLocalFile) - if (result?.ready && result.text === local.text) container.dataset.markdownReady = "" + if (!result?.ready || result.text !== local.text) return + + if (local.streaming || pending.length === 0) { + container.dataset.markdownReady = "" + + return + } + + // Cold timelines already wait for Markdown readiness before measuring and revealing their content. Include + // path classification so the first visible frame uses the final blue-link styling and padding. + void Promise.all(pending).then(() => { + if (lifetime.signal.aborted || renderedBlocks !== content || result.text !== local.text) return + container.dataset.markdownReady = "" + }) }) onCleanup(() => { @@ -943,6 +982,7 @@ function updateBlock( block: RenderedBlock, labels: CopyLabels, sessionLinks: ((id: string) => string | undefined) | undefined, + signal: AbortSignal, localFileExists?: MarkdownLocalFileExists, ) { const current = container.children[index] @@ -962,10 +1002,11 @@ function updateBlock( // A block that finishes streaming keeps its DOM and the user's selection; only its file paths get checked now. if (existing.dataset.markdownMode === "live" && block.mode !== "live") { existing.dataset.markdownMode = block.mode - markInlineCode(existing, existing, false, localFileExists) + + return markInlineCode(existing, existing, false, signal, localFileExists) } - return + return inlineCodeChecks.get(existing)?.pending } const next = existing ?? document.createElement("div") @@ -980,7 +1021,7 @@ function updateBlock( if (source === next) disposeCopyButtons(next) source.innerHTML = block.html - markInlineCode(source, next, block.mode === "live", localFileExists) + const pending = markInlineCode(source, next, block.mode === "live", signal, localFileExists) markCodeLinks(source) wrapTables(source) @@ -991,7 +1032,7 @@ function updateBlock( rendered.renderer.update(source.innerHTML, block.mode === "live", rendered.raw !== block.raw) rendered.raw = block.raw - return + return pending } if (block.mode === "live") { @@ -1020,17 +1061,19 @@ function updateBlock( }) } - if (existing) return + if (existing) return pending if (!current) { container.appendChild(next) - return + return pending } disposeCopyButtons(current) disposeRenderedMarkdown(current) current.replaceWith(next) + + return pending } function updateCodeBlock(