From 04b11f9ad192270d2e405a2dc24fd22a257b661a Mon Sep 17 00:00:00 2001 From: banozz <263121691+banozz0@users.noreply.github.com> Date: Sat, 12 Sep 2026 18:44:38 +0200 Subject: [PATCH] feat(viewer): download a surface as the file it came from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every surface on a card gets a row in its share menu — "Download diagram.mmd", "Download change.patch", "Download plan.md" — serving the RAW SOURCE behind it rather than a rendering. A diagram comes back editable and a diff comes back appliable; the rendered form already has a door in "Open as image", and a picture of a diagram is not a diagram. GET /api/posts/:id/surfaces/:target/raw serves the bytes, addressed by surface id or 0-based index like the PATCH/DELETE surface routes. It is always an attachment with nosniff, the content types are inert, and an html surface degrades to application/octet-stream — agent-authored content must never become a live document on the workspace origin. Asset-backed surfaces (image, and trace with an uploaded file) redirect to /a/:id, which already serves blobs under that policy. surfaceDownload.ts is runtime-agnostic so the Worker DO serves the route too, and the viewer imports surfaceDownloadName from it — the menu row's label and the file on disk come from one naming rule, not two that drift. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015AZe6cmKF4ZCjwRZN1w8Yn --- .changeset/fluffy-donuts-cheat.md | 10 ++ e2e/download.spec.ts | 55 ++++++++ e2e/viewer.spec.ts | 4 + server/app.ts | 40 +++++- server/surfaceDownload.ts | 221 ++++++++++++++++++++++++++++++ test/api.test.ts | 78 +++++++++++ test/surfaceDownload.test.ts | 152 ++++++++++++++++++++ viewer/src/ShareMenu.tsx | 45 ++++-- viewer/src/api.ts | 8 ++ viewer/src/icons.tsx | 11 ++ viewer/src/styles.css | 8 ++ 11 files changed, 622 insertions(+), 10 deletions(-) create mode 100644 .changeset/fluffy-donuts-cheat.md create mode 100644 e2e/download.spec.ts create mode 100644 server/surfaceDownload.ts create mode 100644 test/surfaceDownload.test.ts diff --git a/.changeset/fluffy-donuts-cheat.md b/.changeset/fluffy-donuts-cheat.md new file mode 100644 index 00000000..ae7659ef --- /dev/null +++ b/.changeset/fluffy-donuts-cheat.md @@ -0,0 +1,10 @@ +--- +"sideshow": minor +--- + +Download a surface as the file it came from. Every surface on a card now has a +row in its share menu — "Download diagram.mmd", "Download change.patch", +"Download plan.md" — serving the raw source behind it rather than a rendering, +so a diagram comes back editable and a diff comes back appliable. The bytes are +also available to the CLI and curl tiers at +`GET /api/posts/:id/surfaces/:target/raw`, addressed by surface id or index. diff --git a/e2e/download.spec.ts b/e2e/download.spec.ts new file mode 100644 index 00000000..af3dedd6 --- /dev/null +++ b/e2e/download.spec.ts @@ -0,0 +1,55 @@ +import { readFileSync } from "node:fs"; +import { expect, publishParts, test } from "./fixtures.ts"; + +const DIAGRAM = "graph TD\n A[Start] --> B[Done]"; + +// The share menu's download rows: one per surface, each saving the RAW SOURCE +// behind it rather than a rendering. The oracle is the saved file — its name and +// its bytes — because that is the whole promise: a diagram you can reopen and +// edit, not a picture of one. +test("a download row saves each surface as the file it came from", async ({ page, server }) => { + await publishParts(server.url, { + title: "Retry backoff", + parts: [ + { kind: "mermaid", mermaid: DIAGRAM }, + { kind: "markdown", markdown: "the plan" }, + ], + agent: "e2e", + }); + + await page.goto(server.url); + const card = page.locator(".card:not(#whatsNew)").first(); + await card.locator("button.share").click(); + + // One row per surface, labelled with the exact filename the save will use — + // the numbering is what tells two surfaces of one post apart. + const rows = page.locator(".share-menu .share-item", { hasText: "Download" }); + await expect(rows).toHaveCount(2); + await expect(rows.nth(0)).toHaveText("Download retry-backoff.mmd"); + await expect(rows.nth(1)).toHaveText("Download retry-backoff-2.md"); + + const [download] = await Promise.all([page.waitForEvent("download"), rows.nth(0).click()]); + expect(download.suggestedFilename()).toBe("retry-backoff.mmd"); + const saved = await download.path(); + expect(readFileSync(saved, "utf8")).toBe(DIAGRAM); + + // Saving happens in place: the feed must still be there behind the menu. + await expect(card).toBeVisible(); +}); + +test("the row is named after the code surface's own filename", async ({ page, server }) => { + await publishParts(server.url, { + title: "Review", + parts: [{ kind: "code", code: "export const a = 1;\n", language: "ts", title: "api.ts" }], + agent: "e2e", + }); + + await page.goto(server.url); + await page.locator(".card:not(#whatsNew)").first().locator("button.share").click(); + const row = page.locator(".share-menu .share-item", { hasText: "Download" }); + await expect(row).toHaveText("Download api.ts"); + + const [download] = await Promise.all([page.waitForEvent("download"), row.click()]); + expect(download.suggestedFilename()).toBe("api.ts"); + expect(readFileSync(await download.path(), "utf8")).toBe("export const a = 1;\n"); +}); diff --git a/e2e/viewer.spec.ts b/e2e/viewer.spec.ts index 25304193..e8beaa48 100644 --- a/e2e/viewer.spec.ts +++ b/e2e/viewer.spec.ts @@ -615,6 +615,10 @@ test("the share menu copies a link and a markdown flattening of the post", async await expect(menu.getByRole("menuitem")).toHaveText([ "Copy link", "Copy as markdown", + // One download row per surface, between the copy actions and the open + // actions. See download.spec.ts for what they actually save. + "Download retry-backoff.md", + "Download retry-backoff-2.html", "Open in new tab", "Open as image", ]); diff --git a/server/app.ts b/server/app.ts index 23478b4c..d29329aa 100644 --- a/server/app.ts +++ b/server/app.ts @@ -18,6 +18,7 @@ import { EventBus, type FeedEvent } from "./events.ts"; import { kitSummaries } from "./kits.ts"; import { registerMcp } from "./mcpHttp.ts"; import { postToMarkdown } from "./postMarkdown.ts"; +import { surfaceDownload } from "./surfaceDownload.ts"; import { escapeHtml, renderHtmlPage, @@ -125,8 +126,15 @@ function assetServeHeaders(asset: Asset): { contentType: string; disposition: st const contentType = ATTACH_SAFE_TYPES.has(asset.contentType) ? asset.contentType : "application/octet-stream"; - const name = (asset.filename || asset.id).replace(/[^\w.-]/g, "_"); - return { contentType, disposition: `attachment; filename="${name}"` }; + return { contentType, disposition: contentDisposition(asset.filename || asset.id) }; +} + +// A download header for a filename we may not control. Anything outside +// `[\w.-]` becomes an underscore, which keeps quotes and newlines (and so header +// injection) out of the value as much as it keeps the name portable. +function contentDisposition(filename: string): string { + const name = filename.replace(/[^\w.-]/g, "_") || "download"; + return `attachment; filename="${name}"`; } // Pick an AssetKind when the caller didn't specify one. @@ -1193,6 +1201,34 @@ export function createApp({ const markdown = postToMarkdown(post, { postUrl: `${base}/p/${post.id}`, assetBase: base }); return c.text(markdown, 200, { "content-type": "text/markdown; charset=utf-8" }); }); + // One surface as its own file — the `.mmd` behind a diagram, the `.patch` + // behind a diff, the `.md` behind prose. `:target` is a surface id or 0-based + // index, the same addressing the PATCH/DELETE surface routes take. What the + // viewer's share menu offers as a download row, and the same bytes on the + // CLI/HTTP tiers. + // + // Always an attachment, never a rendered document: agent-authored content must + // not become a live same-origin page (see the isolation rule in AGENTS.md), so + // the content types are inert, html degrades to octet-stream, and nosniff + // stops the browser second-guessing either. Asset-backed surfaces (image, and + // trace with an uploaded file) redirect to /a/:id, which already serves blobs + // under that same policy and keeps the LRU touch-on-serve honest. + app.get("/api/posts/:id/surfaces/:target/raw", async (c) => { + const post = await store.getPost(c.req.param("id")); + if (!post) return c.json({ error: "post not found" }, 404); + const index = findSurfaceIndex(post.surfaces, c.req.param("target")); + if (index < 0) return c.json({ error: "surface not found" }, 404); + const download = surfaceDownload(post.surfaces[index], index, post.title); + if (!download) return c.json({ error: "surface has nothing to download" }, 404); + if (download.via === "asset") { + const base = `${new URL(c.req.url).origin}${requestBasePath(c.req.raw)}`; + return c.redirect(`${base}/a/${download.assetId}`, 302); + } + c.header("Content-Type", download.contentType); + c.header("Content-Disposition", contentDisposition(download.filename)); + c.header("X-Content-Type-Options", "nosniff"); + return c.body(download.body); + }); app.get("/api/surfaces/:id", getPost); // legacy alias app.get("/api/posts/:id", getPost); app.get("/api/snippets/:id", getPost); // legacy alias diff --git a/server/surfaceDownload.ts b/server/surfaceDownload.ts new file mode 100644 index 00000000..02a3562c --- /dev/null +++ b/server/surfaceDownload.ts @@ -0,0 +1,221 @@ +// One surface, as the file it came from — what the viewer's share menu offers +// as "Download diagram.mmd" and what GET +// /api/posts/:id/surfaces/:target/raw serves so the CLI/HTTP tiers can have it +// too. +// +// The rule is RAW SOURCE, not rendered output: a mermaid surface downloads its +// `.mmd` source (re-editable), never the rendered SVG; a diff downloads a +// `.patch` you can `git apply`. The rendered form already has a door — "Open as +// image" — and a picture of a diagram is not a diagram. +// +// Runtime-agnostic (no `node:` imports, no DOM): the Worker DO serves this route +// as well, and the viewer imports `surfaceDownloadName` so the menu row can be +// labelled with the exact filename the download will have — one naming rule, not +// two that drift. +// +// Like postMarkdown.ts this reads a STORED post, so it sees full surface bodies; +// the viewer's hydrated posts deliberately omit sandboxed surface content (see +// apiViews.ts), which is why the bytes are assembled here and not in the viewer. +import { unifiedDiff } from "./postMarkdown.ts"; +import type { + CodeSurface, + DiffSurface, + HtmlSurface, + ImageSurface, + JsonSurface, + MarkdownSurface, + MermaidSurface, + Surface, + TerminalSurface, + TraceSurface, +} from "./types.ts"; + +// What a surface downloads as. `inline` carries the bytes; `asset` defers to the +// stored blob at /a/:id (image and file-backed trace surfaces are by reference — +// re-encoding them here would be a copy, not a download). An asset's `filename` +// is only a fallback: the stored asset usually carries the name it was uploaded +// under, which beats anything derived, and only the store can see it. +export type SurfaceDownload = + | { via: "inline"; filename: string; contentType: string; body: string } + | { via: "asset"; filename: string; assetId: string }; + +// The little a filename needs to know about a surface: its kind, and — for a +// code surface — the title and language its extension comes from. Loose on +// purpose so the viewer can pass a hydrated surface (which has no body) and the +// server a stored one. +export interface NamedSurface { + kind: Surface["kind"]; + title?: string; + language?: string; +} + +// Extension per kind. `code` is the exception — its extension comes from the +// surface's own filename or language, so it isn't listed here. +const KIND_EXTENSIONS = { + html: "html", + markdown: "md", + // `.mmd` is mermaid's own convention (mermaid-cli reads it); there is no + // registered media type, so it travels as plain text. + mermaid: "mmd", + diff: "patch", + terminal: "txt", + json: "json", + trace: "json", + // Asset-backed: a placeholder only. The route prefers the stored asset's own + // filename, which is the one the uploader chose and the only one that knows + // whether these bytes are a png or a jpeg. + image: "png", +} as const satisfies Omit, "code">; + +// Content types for downloads. Deliberately inert: every entry is a type no +// browser will execute, and `html` is absent on purpose — agent-authored markup +// is served as application/octet-stream so that even a mis-handled response can +// never run as a live document on the workspace origin. Same stance as the asset +// route's ATTACH_SAFE_TYPES (see app.ts). +const KIND_CONTENT_TYPES: Partial> = { + markdown: "text/markdown; charset=utf-8", + mermaid: "text/plain; charset=utf-8", + diff: "text/plain; charset=utf-8", + terminal: "text/plain; charset=utf-8", + json: "application/json; charset=utf-8", + trace: "application/json; charset=utf-8", +}; + +const FALLBACK_CONTENT_TYPE = "application/octet-stream"; + +// Common shiki language ids → the extension a developer expects back. Only the +// ones whose extension isn't just the id itself; everything else falls through +// to the language id, which is right far more often than it is wrong (ts, js, +// py… all name their own extension) and harmless when it isn't. +const LANGUAGE_EXTENSIONS: Record = { + javascript: "js", + typescript: "ts", + python: "py", + ruby: "rb", + rust: "rs", + markdown: "md", + shellscript: "sh", + shell: "sh", + bash: "sh", + yaml: "yml", + csharp: "cs", + kotlin: "kt", + golang: "go", + text: "txt", + plaintext: "txt", +}; + +// A filesystem-safe stem from a post title. Anything that isn't a word +// character becomes a single dash, so "Auth refactor: step 2" → "auth-refactor-step-2". +// Falls back to "post" when a title is empty or entirely punctuation, and is +// capped so a rambling title can't produce a name the OS refuses. +const MAX_STEM = 48; + +export function filenameStem(title: string | undefined): string { + const stem = (title ?? "") + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, MAX_STEM) + .replace(/-+$/, ""); + return stem || "post"; +} + +// A code surface's title is usually a filename ("api.ts"), and a filename the +// agent chose beats anything derived — but only when it really is one. A title +// like "The parser, annotated" has no extension and would download as a file the +// OS can't open, so it degrades to the title-derived stem plus the language's +// extension. +function codeFilename(surface: NamedSurface, stem: string, index: number): string { + const title = surface.title?.trim(); + if (title && /^[\w.\- ]+\.[a-z0-9]+$/i.test(title)) return title.replace(/[^\w.-]/g, "_"); + const language = surface.language?.toLowerCase() ?? ""; + const ext = LANGUAGE_EXTENSIONS[language] ?? (/^[a-z0-9]+$/.test(language) ? language : "txt"); + return `${stem}${suffix(index)}.${ext}`; +} + +// Multi-surface posts need the surfaces told apart, so every surface after the +// first carries its 1-based position. A single-surface post — the common case, +// and the one worth keeping tidy — downloads as a bare `title.md`. +const suffix = (index: number) => (index === 0 ? "" : `-${index + 1}`); + +// The filename a surface downloads as. Exported on its own because the viewer +// labels its menu row with it (and has only the hydrated surface, no body). +export function surfaceDownloadName( + surface: NamedSurface, + index: number, + postTitle?: string, +): string { + const stem = filenameStem(postTitle); + if (surface.kind === "code") return codeFilename(surface, stem, index); + const ext = KIND_EXTENSIONS[surface.kind as keyof typeof KIND_EXTENSIONS] ?? "txt"; + return `${stem}${suffix(index)}.${ext}`; +} + +function jsonBody(data: unknown): string { + try { + return JSON.stringify(data, null, 2) ?? "null"; + } catch { + // A cycle can't reach a stored surface (it arrived as JSON), but the store + // is not the only caller — degrade instead of throwing. + return String(data); + } +} + +// A diff surface sent as before/after file pairs has no patch of its own, so +// build one. Same fallback postMarkdown uses, and the reason `unifiedDiff` is +// exported: the point of a `.patch` download is that `git apply` takes it. +function diffBody(surface: DiffSurface): string | null { + if (surface.patch) return surface.patch; + if (!surface.files?.length) return null; + const patch = surface.files.map((f) => unifiedDiff(f.filename, f.before, f.after)).join(""); + return patch || null; +} + +// The bytes (or the asset reference) for one surface, or null when there is +// nothing to download — an empty body, a diff with neither patch nor files, an +// inline trace with no steps, or a kind this build doesn't know. Callers turn +// null into a 404 rather than serving a zero-byte file. +export function surfaceDownload( + surface: Surface, + index: number, + postTitle?: string, +): SurfaceDownload | null { + const filename = surfaceDownloadName(surface, index, postTitle); + const contentType = KIND_CONTENT_TYPES[surface.kind] ?? FALLBACK_CONTENT_TYPE; + const inline = (body: string | null | undefined): SurfaceDownload | null => + body ? { via: "inline", filename, contentType, body } : null; + + switch (surface.kind) { + case "html": + return inline((surface as HtmlSurface).html); + case "markdown": + return inline((surface as MarkdownSurface).markdown); + case "mermaid": + return inline((surface as MermaidSurface).mermaid); + case "code": + return inline((surface as CodeSurface).code); + // Raw, ANSI and all: the escapes ARE the original output, and a terminal + // replays them. postMarkdown strips them because a markdown fence can't + // render them; a file has no such excuse. + case "terminal": + return inline((surface as TerminalSurface).text); + case "diff": + return inline(diffBody(surface as DiffSurface)); + case "json": + return inline(jsonBody((surface as JsonSurface).data)); + case "image": { + const { assetId } = surface as ImageSurface; + return assetId ? { via: "asset", filename, assetId } : null; + } + case "trace": { + const trace = surface as TraceSurface; + // The uploaded file is the real artifact when there is one; inline steps + // are the smaller, structured form and download as the JSON they are. + if (trace.assetId) return { via: "asset", filename, assetId: trace.assetId }; + return trace.steps?.length ? inline(jsonBody(trace.steps)) : null; + } + default: + return null; + } +} diff --git a/test/api.test.ts b/test/api.test.ts index 1cefc4da..a0a4f169 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -335,6 +335,84 @@ test("GET /api/posts/:id/markdown flattens the post for the share menu", async ( assert.equal((await app.request("/api/posts/nope/markdown")).status, 404); }); +test("GET /api/posts/:id/surfaces/:target/raw serves one surface as its own file", async () => { + const app = makeApp(); + const created = (await ( + await app.request( + "/api/posts", + json({ + title: "Retry backoff", + surfaces: [ + { kind: "mermaid", mermaid: "graph TD;\n a-->b;" }, + { kind: "markdown", markdown: "the plan" }, + ], + }), + ) + ).json()) as any; + + const res = await app.request(`/api/posts/${created.id}/surfaces/0/raw`); + assert.equal(res.status, 200); + assert.equal(await res.text(), "graph TD;\n a-->b;"); + // Always a download, never a document the browser might render on our origin. + assert.equal(res.headers.get("content-disposition"), 'attachment; filename="retry-backoff.mmd"'); + assert.equal(res.headers.get("x-content-type-options"), "nosniff"); + + // A surface id addresses the same surface as its index — the addressing the + // PATCH/DELETE surface routes already take. + const full = (await (await app.request(`/api/posts/${created.id}`)).json()) as any; + const byId = await app.request(`/api/posts/${created.id}/surfaces/${full.surfaces[1].id}/raw`); + assert.equal(await byId.text(), "the plan"); + assert.equal( + byId.headers.get("content-disposition"), + 'attachment; filename="retry-backoff-2.md"', + ); + + assert.equal((await app.request(`/api/posts/nope/surfaces/0/raw`)).status, 404); + assert.equal((await app.request(`/api/posts/${created.id}/surfaces/9/raw`)).status, 404); +}); + +test("an html surface downloads as an inert type, and an image redirects to its asset", async () => { + const app = makeApp(); + const created = (await ( + await app.request( + "/api/posts", + json({ + title: "Drawn", + surfaces: [ + { kind: "html", html: "drawn" }, + { kind: "image", assetId: "sha" }, + ], + }), + ) + ).json()) as any; + + // Agent markup must never come back as text/html from the workspace origin. + const html = await app.request(`/api/posts/${created.id}/surfaces/0/raw`); + assert.equal(html.headers.get("content-type"), "application/octet-stream"); + assert.equal(await html.text(), "drawn"); + + // By-reference bytes stay by reference: /a/:id already serves blobs under the + // same attachment policy and keeps the asset LRU honest. + const image = await app.request(`https://board.test/api/posts/${created.id}/surfaces/1/raw`); + assert.equal(image.status, 302); + assert.equal(image.headers.get("location"), "https://board.test/a/sha"); +}); + +test("surface downloads follow the base path and reach public readers", async () => { + const app = makeApp("secret", { publicRead: "session", basePath: "/alice" }); + const created = (await ( + await app.request( + "/api/posts", + authedJson({ title: "T", surfaces: [{ kind: "image", assetId: "sha" }] }), + ) + ).json()) as any; + + // Downloading a shared post is a read — same gate as copying it as markdown. + const res = await app.request(`https://board.test/api/posts/${created.id}/surfaces/0/raw`); + assert.equal(res.status, 302); + assert.equal(res.headers.get("location"), "https://board.test/alice/a/sha"); +}); + test("post markdown resolves links against a base path and reaches public readers", async () => { const app = makeApp("secret", { publicRead: "session", basePath: "/alice" }); const created = (await ( diff --git a/test/surfaceDownload.test.ts b/test/surfaceDownload.test.ts new file mode 100644 index 00000000..d541f220 --- /dev/null +++ b/test/surfaceDownload.test.ts @@ -0,0 +1,152 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import { surfaceDownload, surfaceDownloadName } from "../server/surfaceDownload.ts"; +import type { Surface } from "../server/types.ts"; + +const TITLE = "Retry backoff"; + +function inline(surface: Surface, index = 0, title: string | undefined = TITLE) { + const download = surfaceDownload(surface, index, title); + assert.ok(download, "expected a download"); + assert.equal(download.via, "inline", "expected inline bytes, not an asset"); + return download; +} + +test("names a file from the kind's extension and the post title", () => { + assert.equal(surfaceDownloadName({ kind: "mermaid" }, 0, TITLE), "retry-backoff.mmd"); + assert.equal(surfaceDownloadName({ kind: "markdown" }, 0, TITLE), "retry-backoff.md"); + assert.equal(surfaceDownloadName({ kind: "diff" }, 0, TITLE), "retry-backoff.patch"); + assert.equal(surfaceDownloadName({ kind: "terminal" }, 0, TITLE), "retry-backoff.txt"); + assert.equal(surfaceDownloadName({ kind: "json" }, 0, TITLE), "retry-backoff.json"); + assert.equal(surfaceDownloadName({ kind: "html" }, 0, TITLE), "retry-backoff.html"); +}); + +test("numbers every surface after the first, so a multi-surface post can't collide", () => { + assert.equal(surfaceDownloadName({ kind: "mermaid" }, 0, TITLE), "retry-backoff.mmd"); + assert.equal(surfaceDownloadName({ kind: "mermaid" }, 1, TITLE), "retry-backoff-2.mmd"); + assert.equal(surfaceDownloadName({ kind: "markdown" }, 2, TITLE), "retry-backoff-3.md"); +}); + +test("falls back to a usable stem for a title that slugs to nothing", () => { + assert.equal(surfaceDownloadName({ kind: "markdown" }, 0, "!!! ???"), "post.md"); + assert.equal(surfaceDownloadName({ kind: "markdown" }, 0, ""), "post.md"); + assert.equal(surfaceDownloadName({ kind: "markdown" }, 0, undefined), "post.md"); +}); + +test("caps a rambling title instead of producing a name the OS would refuse", () => { + const name = surfaceDownloadName({ kind: "markdown" }, 0, "word ".repeat(40)); + assert.ok(name.length <= 52, `name too long: ${name}`); + assert.ok(!name.includes("-."), `stem must not end in a dash: ${name}`); +}); + +test("a code surface keeps the filename its agent gave it", () => { + assert.equal( + surfaceDownloadName({ kind: "code", title: "sqlStore.ts", language: "ts" }, 0, TITLE), + "sqlStore.ts", + ); +}); + +test("a code surface with a prose title falls back to the language's extension", () => { + assert.equal( + surfaceDownloadName( + { kind: "code", title: "The parser, annotated", language: "python" }, + 0, + TITLE, + ), + "retry-backoff.py", + ); + assert.equal( + surfaceDownloadName({ kind: "code", language: "rust" }, 1, TITLE), + "retry-backoff-2.rs", + ); + // An unknown language id is used as-is (ts, js, go all name their extension); + // a missing or nonsense one degrades to plain text rather than a broken name. + assert.equal( + surfaceDownloadName({ kind: "code", language: "zig" }, 0, TITLE), + "retry-backoff.zig", + ); + assert.equal(surfaceDownloadName({ kind: "code" }, 0, TITLE), "retry-backoff.txt"); + assert.equal( + surfaceDownloadName({ kind: "code", language: "c++ (old)" }, 0, TITLE), + "retry-backoff.txt", + ); +}); + +test("downloads a mermaid surface as its source, not as a rendering", () => { + const download = inline({ kind: "mermaid", mermaid: "graph TD;\n a-->b;" }); + assert.equal(download.filename, "retry-backoff.mmd"); + assert.equal(download.body, "graph TD;\n a-->b;"); + assert.match(download.contentType, /^text\/plain/); +}); + +test("downloads markdown and terminal text verbatim", () => { + assert.equal(inline({ kind: "markdown", markdown: "# hi\n" }).body, "# hi\n"); + // ANSI escapes survive: they ARE the original output, and a terminal replays + // them. (postMarkdown strips them because a markdown fence cannot render them.) + const ansi = "\u001b[31mred\u001b[0m\n"; + assert.equal(inline({ kind: "terminal", text: ansi }).body, ansi); +}); + +test("serializes a json surface as indented JSON", () => { + const download = inline({ kind: "json", data: { a: [1, 2] } }); + assert.equal(download.body, '{\n "a": [\n 1,\n 2\n ]\n}'); + assert.match(download.contentType, /^application\/json/); +}); + +test("serves html as an inert octet-stream, never as a live document type", () => { + const download = inline({ kind: "html", html: "hi" }); + assert.equal(download.contentType, "application/octet-stream"); + assert.equal(download.body, "hi"); +}); + +test("a diff surface downloads a patch git apply accepts", () => { + const surface: Surface = { + kind: "diff", + files: [{ filename: "f.txt", before: "a\nb\n", after: "a\nc\n" }], + }; + const patch = inline(surface).body; + const dir = mkdtempSync(join(tmpdir(), "sideshow-download-")); + try { + execFileSync("git", ["init", "-q", "."], { cwd: dir }); + writeFileSync(join(dir, "f.txt"), "a\nb\n"); + writeFileSync(join(dir, "p.patch"), patch); + execFileSync("git", ["apply", "p.patch"], { cwd: dir, stdio: "pipe" }); + assert.equal(readFileSync(join(dir, "f.txt"), "utf8"), "a\nc\n"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a diff surface prefers the patch it was published with", () => { + const patch = "--- a/x\n+++ b/x\n@@ -1 +1 @@\n-a\n+b\n"; + assert.equal(inline({ kind: "diff", patch }).body, patch); +}); + +test("asset-backed surfaces defer to the stored blob instead of re-encoding it", () => { + const image = surfaceDownload({ kind: "image", assetId: "asset1" }, 0, TITLE); + assert.deepEqual(image, { via: "asset", filename: "retry-backoff.png", assetId: "asset1" }); + const trace = surfaceDownload({ kind: "trace", assetId: "asset2", steps: [] }, 0, TITLE); + assert.equal(trace?.via, "asset"); +}); + +test("an inline trace downloads its steps as the JSON they are", () => { + const download = inline({ kind: "trace", steps: [{ label: "ran tests" }] }, 0); + assert.equal(download.filename, "retry-backoff.json"); + assert.equal(download.body, '[\n {\n "label": "ran tests"\n }\n]'); +}); + +test("returns null rather than serving an empty file", () => { + assert.equal(surfaceDownload({ kind: "markdown", markdown: "" }, 0, TITLE), null); + assert.equal(surfaceDownload({ kind: "mermaid", mermaid: "" }, 0, TITLE), null); + assert.equal(surfaceDownload({ kind: "diff" }, 0, TITLE), null); + assert.equal(surfaceDownload({ kind: "diff", files: [] }, 0, TITLE), null); + assert.equal(surfaceDownload({ kind: "trace" }, 0, TITLE), null); + assert.equal(surfaceDownload({ kind: "image", assetId: "" }, 0, TITLE), null); + // Forward compatibility: a kind this build doesn't know downloads nothing + // rather than throwing the request. + assert.equal(surfaceDownload({ kind: "hologram" } as unknown as Surface, 0, TITLE), null); +}); diff --git a/viewer/src/ShareMenu.tsx b/viewer/src/ShareMenu.tsx index f413856d..ef995023 100644 --- a/viewer/src/ShareMenu.tsx +++ b/viewer/src/ShareMenu.tsx @@ -1,16 +1,18 @@ import { createEffect, createSignal, For, type JSX, on, onCleanup, Show } from "solid-js"; +import { surfaceDownloadName } from "../../server/surfaceDownload.ts"; import { apiText, canScreenshot, postImageLink, postLink, postMarkdownPath, + surfaceDownloadLink, type Post, type ViewerPost, } from "./api.ts"; import { writeClipboard } from "./clipboard.ts"; import { root } from "./host.ts"; -import { ImageIcon, LinkIcon, MarkdownIcon, OpenIcon, ShareIcon } from "./icons.tsx"; +import { DownloadIcon, ImageIcon, LinkIcon, MarkdownIcon, OpenIcon, ShareIcon } from "./icons.tsx"; import { toast } from "./state.ts"; // The card's single "take this elsewhere" control: one labelled button opening a @@ -28,9 +30,9 @@ const MENU_WIDTH = 216; const MENU_GAP = 6; const VIEWPORT_PAD = 8; // Menu height, derived from the row metrics in styles.css (32px rows, a 9px -// separator block, 4px of panel padding either side) — the menu is measured +// separator block each, 4px of panel padding either side) — the menu is placed // before it can be measured, so the flip decision uses this estimate. -const MENU_HEIGHT = (rows: number) => rows * 32 + 9 + 8; +const MENU_HEIGHT = (rows: number, separators: number) => rows * 32 + separators * 9 + 8; type MenuAction = { key: string; @@ -41,6 +43,10 @@ type MenuAction = { run?: () => void | Promise; disabledReason?: string; separatorBefore?: boolean; + // A save-this-file link. Rendered in place rather than in a new tab, and with + // the filename the response will use anyway — so the row's label and the file + // on disk can't disagree. + download?: string; }; export function ShareMenu(props: { post: Post | ViewerPost }) { @@ -85,6 +91,7 @@ export function ShareMenu(props: { post: Post | ViewerPost }) { "Couldn't copy this post as markdown", ), }, + ...downloadActions(), { key: "open", label: "Open in new tab", @@ -105,6 +112,23 @@ export function ShareMenu(props: { post: Post | ViewerPost }) { }, ]; + // One row per surface: the raw source behind it, as the file it came from — + // `.mmd` for a diagram, `.patch` for a diff, `.md` for prose. The filename + // comes from the same server module that serves the bytes, so the label is + // the truth and not a second guess at it. + const downloadActions = (): MenuAction[] => + props.post.surfaces.map((surface, index) => { + const filename = surfaceDownloadName(surface, index, props.post.title); + return { + key: `download-${surface.id ?? index}`, + label: `Download ${filename}`, + icon: DownloadIcon, + href: surfaceDownloadLink(props.post.id, index), + download: filename, + separatorBefore: index === 0, + }; + }); + const fetchMarkdown = () => (markdown ??= apiText(postMarkdownPath(props.post.id))); const items = () => @@ -117,7 +141,8 @@ export function ShareMenu(props: { post: Post | ViewerPost }) { const openMenu = (focusFirst: boolean) => { const rect = button.getBoundingClientRect(); - const height = MENU_HEIGHT(actions().length); + const rows = actions(); + const height = MENU_HEIGHT(rows.length, rows.filter((a) => a.separatorBefore).length); const below = rect.bottom + MENU_GAP; setAt({ left: Math.max( @@ -251,7 +276,7 @@ export function ShareMenu(props: { post: Post | ViewerPost }) { onClick={() => action.run?.()} > {action.icon()} - {action.label} + {action.label} } > @@ -260,12 +285,16 @@ export function ShareMenu(props: { post: Post | ViewerPost }) { class="share-item" role="menuitem" href={href} - target="_blank" - rel="noopener" + // A download saves in place; anything else opens away from + // the feed, which must not be replaced by it. + {...(action.download + ? { download: action.download } + : { target: "_blank", rel: "noopener" })} + title={action.label} onClick={() => close(false)} > {action.icon()} - {action.label} + {action.label} )} diff --git a/viewer/src/api.ts b/viewer/src/api.ts index bfddc4e7..fe7a0b54 100644 --- a/viewer/src/api.ts +++ b/viewer/src/api.ts @@ -142,6 +142,14 @@ export function postMarkdownPath(id: string): string { return `/api/posts/${encodeURIComponent(id)}/markdown`; } +// One surface as its own file (GET /api/posts/:id/surfaces/:n/raw) — the `.mmd` +// behind a diagram, the `.patch` behind a diff. A real link, not a fetch: the +// response is an attachment, so the browser saves it, and cmd/middle-click still +// opens it the way a link should. +export function surfaceDownloadLink(id: string, index: number): string { + return `${location.origin}${appPath(`/api/posts/${encodeURIComponent(id)}/surfaces/${index}/raw`)}`; +} + // Whether the deployment can render post screenshots (the /p/:id.png route). // Host-first (cloud embed), falling back to the self-hosted global, mirroring // isReadonly(). False on a plain Node server, which has no Browser Rendering. diff --git a/viewer/src/icons.tsx b/viewer/src/icons.tsx index 1e51e1f3..7c092130 100644 --- a/viewer/src/icons.tsx +++ b/viewer/src/icons.tsx @@ -82,6 +82,17 @@ export function MarkdownIcon() { ); } +// lucide: download +export function DownloadIcon() { + return ( + + + + + + ); +} + // lucide: image export function ImageIcon() { return ( diff --git a/viewer/src/styles.css b/viewer/src/styles.css index 17358be8..f77dda09 100644 --- a/viewer/src/styles.css +++ b/viewer/src/styles.css @@ -1645,6 +1645,14 @@ iframe { cursor: pointer; text-decoration: none; } +/* A filename can be longer than the menu is wide; truncate the label rather + than letting it push the row past the panel. The full text stays reachable as + the row's tooltip. */ +.share-menu .share-item span { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} .share-menu .share-item svg { width: 14px; height: 14px;