From d8bbf79ba547e76721c7dc2c3b4df007eb68907f Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Tue, 20 Oct 2026 21:15:02 -0700 Subject: [PATCH] fix(app): stream previewed videos so they play right away (#57587) --- packages/app/e2e/regression/review.spec.ts | 38 +++++++++++++---- packages/app/src/runtime/server/api.test.ts | 10 ++++- packages/app/src/runtime/server/api.ts | 17 ++++++++ packages/app/src/runtime/server/client.tsx | 18 +++++++- packages/app/src/workspaces/files/artifact.ts | 12 ++++++ packages/app/src/workspaces/files/model.tsx | 28 ++++++++++--- .../gui-extensions/src/file/artifact-view.tsx | 41 +++++++++++++++---- packages/gui-extensions/src/file/i18n/en.ts | 1 + packages/gui-extensions/src/sdk/host-apis.ts | 2 + 9 files changed, 144 insertions(+), 23 deletions(-) diff --git a/packages/app/e2e/regression/review.spec.ts b/packages/app/e2e/regression/review.spec.ts index 761cc254013..e0d4e968438 100644 --- a/packages/app/e2e/regression/review.spec.ts +++ b/packages/app/e2e/regression/review.spec.ts @@ -850,7 +850,7 @@ test("image files load into the review panel and keep it painted while loading", ).toEqual([]) }) -test("video files over 25 MB play in the viewer, expand into the theatre overlay, and collapse on Escape without remounting", async ({ +test("video files stream into the viewer behind a loading state, expand into the theatre overlay, and collapse on Escape without remounting", async ({ page, }) => { const directory = "C:/OpenCode/ReviewVideo" @@ -874,13 +874,29 @@ test("video files over 25 MB play in the viewer, expand into the theatre overlay panes: { ses_reviewvideo: { review: true } }, }, }) - await page.route(`**/api/fs/read/${file}*`, (route) => - route.fulfill({ - status: 200, - headers: { "access-control-allow-origin": "*", "content-type": "application/octet-stream" }, - body: mp4, - }), - ) + // The app only checks the file with a HEAD; the video element streams it with range requests, held until released. + const ranged = Promise.withResolvers() + const release = Promise.withResolvers() + await page.route(`**/api/fs/read/${file}*`, async (route) => { + const headers = { "access-control-allow-origin": "*", "accept-ranges": "bytes", "content-type": "video/mp4" } + + if (route.request().method() === "HEAD") + return route.fulfill({ status: 200, headers: { ...headers, "content-length": String(mp4.length) } }) + + const range = /^bytes=(\d+)-(\d*)$/.exec((await route.request().headerValue("range")) ?? "") + + if (!range) return route.fulfill({ status: 200, headers, body: mp4 }) + ranged.resolve() + await release.promise + const start = Number(range[1]) + const end = Math.min(range[2] ? Number(range[2]) : mp4.length - 1, start + 4_000_000, mp4.length - 1) + + return route.fulfill({ + status: 206, + headers: { ...headers, "content-range": `bytes ${start}-${end}/${mp4.length}` }, + body: mp4.subarray(start, end + 1), + }) + }) const panel = page.locator("#review-panel") await openFileLauncher(panel) @@ -892,7 +908,13 @@ test("video files over 25 MB play in the viewer, expand into the theatre overlay const video = stage.locator('video[data-slot="artifact-media"]') const fullscreen = stage.getByRole("button", { name: "Enter full screen", exact: true }) const exitFullscreen = stage.getByRole("button", { name: "Exit full screen", exact: true }) + const loading = stage.getByRole("status", { name: "Loading", exact: true }) + await ranged.promise + await expect(loading).toBeVisible() + expect(await video.getAttribute("src")).toMatch(new RegExp(`^${SERVER}/api/fs/read/${file}\\?`)) + release.resolve() + await expect(loading).toHaveCount(0) await expect(video).toBeVisible() await expect(toolbar).toContainText("64 × 36") await expect(toolbar).toContainText("0:01") diff --git a/packages/app/src/runtime/server/api.test.ts b/packages/app/src/runtime/server/api.test.ts index 2ba3c8ce594..996abb048e6 100644 --- a/packages/app/src/runtime/server/api.test.ts +++ b/packages/app/src/runtime/server/api.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test" -import { authFromToken, authTokenFromCredentials } from "./api" +import { authFromToken, authTokenFromCredentials, fileStreamUrl } from "./api" test.each([ [btoa("opencode:secret"), { password: "secret" }], @@ -15,3 +15,11 @@ test("authTokenFromCredentials ignores usernames in legacy saved credentials", ( const credentials = { username: "legacy", password: "secret" } expect(authTokenFromCredentials(credentials)).toBe(btoa("opencode:secret")) }) + +test("fileStreamUrl never puts a credential in a media URL", () => { + const input = { path: "clips/final cut.mp4", directory: "/work/app" } + expect(fileStreamUrl({ url: "http://127.0.0.1:4096", password: "secret" }, input)).toBeUndefined() + expect(fileStreamUrl({ url: "http://127.0.0.1:4096" }, input)).toBe( + "http://127.0.0.1:4096/api/fs/read/clips/final%20cut.mp4?location%5Bdirectory%5D=%2Fwork%2Fapp", + ) +}) diff --git a/packages/app/src/runtime/server/api.ts b/packages/app/src/runtime/server/api.ts index dfdde8805a6..d0b2eea0794 100644 --- a/packages/app/src/runtime/server/api.ts +++ b/packages/app/src/runtime/server/api.ts @@ -36,4 +36,21 @@ export function createApiForServer(input: { }) } +/** + * The server's ranged read of one file, for media elements. They cannot send the Authorization header, and a credential + * never goes in a DOM URL, so a server that needs a password has none: its media downloads through the API instead. + */ +export function fileStreamUrl(server: ServerConnection.HttpBase, input: { path: string; directory: string }) { + if (server.password) return + + const url = new URL( + `api/fs/read/${input.path.split("/").map(encodeURIComponent).join("/")}`, + server.url.endsWith("/") ? server.url : `${server.url}/`, + ) + + url.searchParams.set("location[directory]", input.directory) + + return url.href +} + export type ServerApi = OpenCodeClient diff --git a/packages/app/src/runtime/server/client.tsx b/packages/app/src/runtime/server/client.tsx index 94e52285cbd..ea83db55416 100644 --- a/packages/app/src/runtime/server/client.tsx +++ b/packages/app/src/runtime/server/client.tsx @@ -62,6 +62,8 @@ type ServerSDKBase = { server: ServerConnection.Any scope: ServerScope url: string + /** The connection the API currently uses, which a managed server's reconnect may replace. */ + http: ServerConnection.HttpBase api: ServerApi pty: ReturnType connection: { @@ -110,6 +112,9 @@ function createServerSdkContextBase(server: ServerConnection.Any, scope: ServerS get url() { return transport.url }, + get http() { + return transport.http + }, get api() { return transport.api }, @@ -121,12 +126,18 @@ function createServerSdkContextBase(server: ServerConnection.Any, scope: ServerS } } -export function createServerTransport(input: { http: ServerConnection.HttpBase; fetch?: typeof globalThis.fetch }): { +export type ServerTransport = { update(http: ServerConnection.HttpBase): ServerApi readonly url: string + readonly http: ServerConnection.HttpBase readonly api: ServerApi readonly pty: ReturnType -} { +} + +export function createServerTransport(input: { + http: ServerConnection.HttpBase + fetch?: typeof globalThis.fetch +}): ServerTransport { const queue = createRequestQueue({ fetch: input.fetch ?? globalThis.fetch }) const build = (http: ServerConnection.HttpBase) => { @@ -146,6 +157,9 @@ export function createServerTransport(input: { http: ServerConnection.HttpBase; get url() { return state.current.http.url }, + get http() { + return state.current.http + }, get api() { return state.current.api }, diff --git a/packages/app/src/workspaces/files/artifact.ts b/packages/app/src/workspaces/files/artifact.ts index 2f769f0c98e..625f93886ef 100644 --- a/packages/app/src/workspaces/files/artifact.ts +++ b/packages/app/src/workspaces/files/artifact.ts @@ -13,6 +13,18 @@ const binaryKinds = new Set([ "presentation", ]) +/** Kinds a media element reads straight from the server, so it shows the first frame before the rest arrives. */ +export const streamedKinds = new Set(["image", "audio", "video"]) + +/** Content for a streamed file from a HEAD of its URL. A changed file gets a new URL, so its element reloads. */ +export function streamedFileContent(path: string, url: string, response: Response): FileContent { + const size = Number(response.headers.get("content-length") ?? 0) + const versioned = new URL(url) + versioned.searchParams.set("v", `${size}-${Date.parse(response.headers.get("last-modified") ?? "") || 0}`) + + return { type: "binary", content: "", size, mimeType: artifactMime(path), url: versioned.href } +} + /** Text files never contain NUL; a NUL in the first 8 KiB marks an unknown binary. */ function isBinaryBytes(bytes: Uint8Array) { return bytes.subarray(0, 8192).includes(0) diff --git a/packages/app/src/workspaces/files/model.tsx b/packages/app/src/workspaces/files/model.tsx index 97ba50a6bec..ba5be1f4b7f 100644 --- a/packages/app/src/workspaces/files/model.tsx +++ b/packages/app/src/workspaces/files/model.tsx @@ -9,7 +9,10 @@ import { useWorkspaceLocation } from "@/workspaces/location" import { useLanguage } from "@/runtime/i18n/language" import { useExtensionAttachment } from "@/runtime/extension/host-apis" import { createPathHelpers } from "./path" -import { fileContentFromBytes } from "./artifact" +import { artifactKind } from "@opencode/util/artifact" +import { usePlatform } from "@/runtime/platform/platform" +import { fileStreamUrl } from "@/runtime/server/api" +import { fileContentFromBytes, streamedFileContent, streamedKinds } from "./artifact" import { approxBytes, evictContentLru, @@ -42,6 +45,7 @@ export const { use: useFile, provider: FileProvider } = createSimpleContext({ const sdk = useWorkspaceLocation() const params = useParams() const serverSDK = useServerSDK() + const platform = usePlatform() const language = useLanguage() const extensions = useExtensionAttachment() @@ -164,6 +168,22 @@ export const { use: useFile, provider: FileProvider } = createSimpleContext({ }) } + // Media plays from the server's ranged reads, so only a HEAD runs here, for the file's size. A failed HEAD has no + // body; the full read that follows reports the server's error. + const read = (file: string, request: { path: string; location: { directory: string } }) => { + const download = () => serverSDK.api.file.read(request).then((data) => fileContentFromBytes(file, data)) + + const url = streamedKinds.has(artifactKind(file)) + ? fileStreamUrl(serverSDK.http, { path: request.path, directory: request.location.directory }) + : undefined + + if (!url) return download() + + return (platform.fetch ?? fetch)(url, { method: "HEAD" }).then((response) => + response.ok ? streamedFileContent(file, url, response) : download(), + ) + } + const load = (input: string, options?: { force?: boolean; silent?: boolean }) => { const file = path.normalize(input) @@ -199,11 +219,9 @@ export const { use: useFile, provider: FileProvider } = createSimpleContext({ promise: Promise.resolve(), } - entry.promise = serverSDK.api.file - .read(request) - .then((data) => { + entry.promise = read(file, request) + .then((content) => { if (scope() !== directory) return - const content = fileContentFromBytes(file, data) setLoaded(file, content) touchFileContent(file, approxBytes(content)) evictContent(new Set([file])) diff --git a/packages/gui-extensions/src/file/artifact-view.tsx b/packages/gui-extensions/src/file/artifact-view.tsx index c64eb76dc5f..7bfff4f2474 100644 --- a/packages/gui-extensions/src/file/artifact-view.tsx +++ b/packages/gui-extensions/src/file/artifact-view.tsx @@ -18,6 +18,7 @@ import { Button } from "@opencode/ui/button" import { FileIcon } from "@opencode/ui/file-icon" import { SegmentedControl, SegmentedControlItem } from "@opencode/ui/segmented-control" import { ScrollView } from "@opencode/ui/scroll-view" +import { Spinner } from "@opencode/ui/spinner" import { Markdown } from "@opencode/session-ui/markdown" import { MarkdownProvider, useMarkdown } from "@opencode/session-ui/context/markdown" import { artifactKind, type ArtifactKind } from "@opencode/util/artifact" @@ -123,7 +124,7 @@ export default function ArtifactView(props: { // A file that fails also drops the facts its viewer reported, such as a page count. const fail = (reason?: string) => change({ undecodable: true, info: {}, reason }) - const media = { onInfo: (info: ArtifactInfo) => change({ info }), onError: () => fail() } + const media = { onInfo: (info: ArtifactInfo) => change({ info }), onError: () => fail(ctx.t("view.loadFailed")) } const size = () => formatBytes(locale.locale(), contentBytes(props.content)) @@ -295,8 +296,11 @@ function OpenInBrowserButton(props: { session: MountedSession; path: string }) { ) } -function createBlobUrl(content: () => FileContent) { +function createMediaUrl(content: () => FileContent) { return createMemo(() => { + const streamed = content().url + + if (streamed) return streamed const value = blobUrlFromContent(content()) onCleanup(() => URL.revokeObjectURL(value)) @@ -304,9 +308,24 @@ function createBlobUrl(content: () => FileContent) { }) } +/** Holds the pane until the media's first frame, so a file still streaming never shows as blank. */ +function MediaLoading() { + const ctx = useExtension() + + return ( +
+ +
+ ) +} + /** Images and SVG previews: each decoded image owns its viewport. */ function ArtifactImage(props: MediaProps) { - const url = createBlobUrl(() => props.content) + const url = createMediaUrl(() => props.content) return ( @@ -485,6 +504,9 @@ function ImageStage(props: MediaProps & { src: string }) { zoomAt(1 / fit(), event.clientX, event.clientY) }} /> + + +