mirror of
https://github.com/anomalyco/opencode.git
synced 2026-11-13 11:39:41 +03:00
fix(app): prevent inline file links flashing on tab switches (#57725)
This commit is contained in:
committed by
GitHub
Unverified
parent
3449855dab
commit
036d0faea7
@@ -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}"]`)
|
||||
|
||||
@@ -136,7 +136,7 @@ export interface MockServerConfig {
|
||||
fileList?: (path: string) => unknown[] | Promise<unknown[]>
|
||||
fileContent?: (path: string) => MockFileContent | Promise<MockFileContent>
|
||||
// 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<Record<string, { type: string }>>
|
||||
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,
|
||||
}
|
||||
|
||||
@@ -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<string, Record<string, LineRange | null>> }
|
||||
|
||||
type StyleLoad = { loaded?: Promise<void> }
|
||||
|
||||
type LinkCheck = { result: boolean | Promise<boolean>; expires: number; known?: true }
|
||||
|
||||
const setup: Setup<typeof File> = (ctx) => {
|
||||
const sessions = ctx.sessions
|
||||
const layout = ctx.layout
|
||||
@@ -516,24 +519,74 @@ const setup: Setup<typeof File> = (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<SessionScreen, WeakMap<ServerRef["client"], Map<string, LinkCheck>>>()
|
||||
|
||||
// Messages style a path as a link only when this says the file exists.
|
||||
const linkExists = (link: Link): boolean | Promise<boolean> => {
|
||||
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<ServerRef["client"], Map<string, LinkCheck>>()
|
||||
linkChecks.set(screen, clients)
|
||||
const client = session.server.client
|
||||
const checks = clients.get(client) ?? new Map<string, LinkCheck>()
|
||||
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
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
>
|
||||
<Show when={visible()}>
|
||||
<Markdown
|
||||
@@ -71,4 +74,10 @@ export async function mountMarkdown(options: {
|
||||
</DialogProvider>
|
||||
)
|
||||
}, host)
|
||||
|
||||
// Capture before Promise callbacks run: eventual assertions cannot catch a flash on insertion.
|
||||
return Array.from(host.querySelectorAll<HTMLElement>(":not(pre) > code"), (node) => ({
|
||||
text: node.textContent,
|
||||
kind: node.dataset.inlineCodeKind ?? null,
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -397,7 +397,7 @@ function markInlineCode(
|
||||
live: boolean,
|
||||
localFileExists?: MarkdownLocalFileExists,
|
||||
) {
|
||||
const checked = new Set<string>()
|
||||
const checked = new Map<string, boolean | Promise<boolean>>()
|
||||
|
||||
for (const code of source.querySelectorAll<HTMLElement>(":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<HTMLElement>(":not(pre) > code").forEach((node) => {
|
||||
|
||||
Reference in New Issue
Block a user