feat(viewer): download a surface as the file it came from

.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.

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");
+});

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",
   ]);

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

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<Record<Surface["kind"], string>, "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<Record<Surface["kind"], string>> = {
+  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<string, string> = {
+  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;
+  }
+}