feat(studio): say why the SDK session's file read came back empty (#4161)

* feat(studio): say why the SDK session's file read came back empty

`stage: read` was the largest remaining class of SDK-session failures and a
single opaque string covering four different outcomes. It is a hard block, not
a blip: 56 users hit it over a 7-day window and, between them, landed zero
successful SDK edits — while staying active in Studio (3107 playback events,
138 design inputs, 20 commit transactions). Every edit in those sessions fell
back to the server path with nothing recorded, because no session also means no
resolver-shadow event.

readProjectFileOptional now returns a discriminated result instead of
`string | undefined`, and the telemetry carries the branch that fired:

- unsafe_path      — the traversal/NUL guard rejected the path client-side
- http_error       — the request failed, with its status
- missing_content  — 2xx whose body was not the shape we expect
- empty_file       — `optional=1` answers an absent file with 200 + "", which
                     parses into a session with no elements; previously this
                     looked like a healthy session that merely went unused

Two candidate causes were investigated and ruled out first, with tests kept as
regression cover in studio-server (projectIdEncoding.test.ts): project ids that
percent-encode (the CLI derives the id from the folder name, so a space or
non-ASCII character produces one) and sub-composition paths whose separator
encodes as %2F. Both resolve correctly today because Hono hands over an
already-decoded `c.req.path`. The decoded/encoded seam in resolveProjectPath is
a real latent hazard, hence the tests, but it is not this bug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(studio): fold the absent-file read reason into the result union

Review feedback on #4161. `empty_file` was synthesized by the caller after
the helper returned `{ok: true, content: ""}`, so reason attribution was split
across two places and `ProjectFileReadResult` was not the full reason surface.
It now joins the union, and the caller has one branch.

Renamed to `absent_or_empty`: the optional=1 absent-file shim and a genuinely
0-byte file are the same 200 on the wire and cannot be told apart here, so the
literal name says what the tag actually means. For a composition it is always
the former.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

packages/studio-server/src/routes/projectIdEncoding.test.ts

@@ -0,0 +1,94 @@
+import { describe, expect, it } from "vitest";
+import { Hono } from "hono";
+import { mkdtempSync, writeFileSync, rmSync, mkdirSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { registerFileRoutes } from "./files.js";
+import type { StudioApiAdapter } from "../types.js";
+
+/**
+ * Pins the path-resolution contract of the project-file read that Studio's SDK
+ * session depends on. If this route fails, `openComposition` never runs and
+ * every edit in that project silently falls back to the server path.
+ *
+ * The shapes here are the ones that looked most likely to break it, because
+ * `resolveProjectPath` strips a prefix built from the *decoded* `project.id`
+ * out of `c.req.path`, and the CLI derives that id straight from the folder
+ * name (`projectId = projectName || basename(projectDir)`). A folder named with
+ * a space or non-ASCII character therefore produces an id that percent-encodes,
+ * and sub-compositions — the house pattern, one per scene — put a separator in
+ * the file path that encodes as %2F.
+ *
+ * All of them pass today: Hono hands `c.req.path` over already decoded. These
+ * are regression tests, not a reproduction — they were written while hunting a
+ * production failure (`stage: read`) that turned out to be something else, and
+ * they are kept because the decoded/encoded seam is a real latent hazard that a
+ * refactor of `resolveProjectPath` could reopen silently.
+ */
+function createAdapter(projectDir: string): StudioApiAdapter {
+  return {
+    listProjects: () => [],
+    resolveProject: async (id: string) => ({ id, dir: projectDir }),
+    bundle: async () => null,
+    lint: async () => ({ findings: [] }),
+    runtimeUrl: "/api/runtime.js",
+    rendersDir: () => "/tmp/renders",
+    startRender: () => ({
+      id: "job-1",
+      status: "rendering",
+      progress: 0,
+      outputPath: "/tmp/out.mp4",
+    }),
+  } as unknown as StudioApiAdapter;
+}
+
+function projectWithComposition(): { dir: string; cleanup: () => void } {
+  const dir = mkdtempSync(join(tmpdir(), "hf-projectid-"));
+  writeFileSync(join(dir, "index.html"), "<html><body>COMPOSITION</body></html>");
+  mkdirSync(join(dir, "scenes"), { recursive: true });
+  writeFileSync(join(dir, "scenes", "scene-1.html"), "<html><body>SCENE ONE</body></html>");
+  return { dir, cleanup: () => rmSync(dir, { recursive: true, force: true }) };
+}
+
+async function readComposition(projectId: string, compPath = "index.html") {
+  const { dir, cleanup } = projectWithComposition();
+  try {
+    const app = new Hono();
+    registerFileRoutes(app, createAdapter(dir));
+    const url = `http://localhost/projects/${encodeURIComponent(projectId)}/files/${encodeURIComponent(compPath)}?optional=1`;
+    const response = await app.request(url);
+    const body = (await response.json()) as { content?: string };
+    return { status: response.status, content: body.content };
+  } finally {
+    cleanup();
+  }
+}
+
+describe("project ids that percent-encode in a URL", () => {
+  it("serves a composition from a plain ASCII project id", async () => {
+    const result = await readComposition("demo-project");
+    expect(result.status).toBe(200);
+    expect(result.content).toContain("COMPOSITION");
+  });
+
+  it("serves a composition when the project folder name has a space", async () => {
+    const result = await readComposition("my video");
+    expect(result.status).toBe(200);
+    expect(result.content).toContain("COMPOSITION");
+  });
+
+  it("serves a composition when the project folder name is non-ASCII", async () => {
+    const result = await readComposition("用故事板做视频");
+    expect(result.status).toBe(200);
+    expect(result.content).toContain("COMPOSITION");
+  });
+
+  // Sub-compositions are the house pattern — one per scene — so the active
+  // composition is very often in a subdirectory. encodeURIComponent turns the
+  // separator into %2F, which is what the client sends.
+  it("serves a sub-composition in a subdirectory", async () => {
+    const result = await readComposition("demo-project", "scenes/scene-1.html");
+    expect(result.status).toBe(200);
+    expect(result.content).toContain("SCENE ONE");
+  });
+});

packages/studio/src/hooks/useSdkSession.lifecycle.test.tsx

@@ -164,17 +164,62 @@ describe("useSdkSession unavailable telemetry", () => {
 
   // Every cutover chokepoint silently takes the server path when there is no
   // session, and the shadow never runs either — so a missing session is a total,
-  // otherwise-invisible SDK bypass. These three exits are its only origin.
-  it("reports an unreadable composition", async () => {
+  // otherwise-invisible SDK bypass. These exits are its only origin.
+  //
+  // `stage: read` carries a reason because it was the largest remaining class
+  // and a single opaque string: 56 users hit it in a 7-day window and never
+  // landed one successful SDK edit between them, with no way to tell a genuinely
+  // absent file from a request that never reached one.
+  it("reports the HTTP status when the read fails", async () => {
     vi.stubGlobal(
       "fetch",
-      vi.fn(async () => ({ ok: false, json: async () => ({}) }) as Response),
+      vi.fn(async () => ({ ok: false, status: 404, json: async () => ({}) }) as Response),
     );
     const root = createRoot(document.createElement("div"));
     await act(async () => root.render(<Probe projectId="project-a" />));
     await flushAsyncEffects();
 
-    expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", { stage: "read" });
+    expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
+      stage: "read",
+      reason: "http_error",
+      status: 404,
+    });
+    await act(async () => root.unmount());
+  });
+
+  it("separates an unexpected response shape from a failed request", async () => {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async () => ({ ok: true, json: async () => ({}) }) as Response),
+    );
+    const root = createRoot(document.createElement("div"));
+    await act(async () => root.render(<Probe projectId="project-a" />));
+    await flushAsyncEffects();
+
+    expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
+      stage: "read",
+      reason: "missing_content",
+    });
+    await act(async () => root.unmount());
+  });
+
+  // `optional=1` answers a file that is not on disk with 200 + an empty string,
+  // so this is what "the composition genuinely is not there" looks like on the
+  // wire — previously indistinguishable from a broken request. The shim and a
+  // real 0-byte file are the same response, hence the name.
+  it("separates a file that is not on disk", async () => {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async () => ({ ok: true, json: async () => ({ content: "" }) }) as Response),
+    );
+    const root = createRoot(document.createElement("div"));
+    await act(async () => root.render(<Probe projectId="project-a" />));
+    await flushAsyncEffects();
+
+    expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
+      stage: "read",
+      reason: "absent_or_empty",
+    });
     await act(async () => root.unmount());
   });
 

packages/studio/src/hooks/useSdkSession.ts

@@ -7,25 +7,53 @@ import type { PublishSdkSession } from "../utils/sdkCutover";
 import { addExternalFileReloadListener } from "./externalFileReloadBus";
 
 /**
- * Read a project file's content, or undefined on a non-2xx (optional read).
- * Replaces the removed SDK http adapter's `read()` — the only thing Studio used
- * it for (Studio is the sole writer, so the adapter's write path was dead).
+ * Why an optional project-file read produced no usable content. `stage: "read"`
+ * was a single opaque reason covering all of these, which made the largest
+ * remaining class of SDK-session failures undiagnosable: 56 users in a 7-day
+ * window hit it and, between them, never landed a single successful SDK edit.
+ * Knowing which branch fired is the difference between "the file legitimately
+ * is not there" and "the request never reached the file".
+ *
+ * Every reason lives in this union so the full surface is readable from one
+ * place — `absent_or_empty` included, even though it is a 2xx.
+ */
+type ProjectFileReadFailure =
+  | { ok: false; reason: "unsafe_path" }
+  | { ok: false; reason: "http_error"; status: number }
+  | { ok: false; reason: "missing_content" }
+  | { ok: false; reason: "absent_or_empty" };
+
+type ProjectFileReadResult = { ok: true; content: string } | ProjectFileReadFailure;
+
+/**
+ * Read a project file's content (optional read — a missing file is not an
+ * error). Replaces the removed SDK http adapter's `read()` — the only thing
+ * Studio used it for (Studio is the sole writer, so the adapter's write path
+ * was dead).
  */
 async function readProjectFileOptional(
   projectId: string,
   path: string,
-): Promise<string | undefined> {
+): Promise<ProjectFileReadResult> {
   // Reject traversal / NUL before building the request URL — `path` is a
   // user-influenced composition path (mirrors the guard in timelineEditingHelpers,
   // and closes the CodeQL client-side-request-forgery flag). encodeURIComponent
   // already confines both values to single segments of this same-origin URL.
-  if (path.includes("\0") || path.includes("..")) return undefined;
+  if (path.includes("\0") || path.includes("..")) return { ok: false, reason: "unsafe_path" };
   const res = await fetch(
     `/api/projects/${encodeURIComponent(projectId)}/files/${encodeURIComponent(path)}?optional=1`,
   );
-  if (!res.ok) return undefined;
+  if (!res.ok) return { ok: false, reason: "http_error", status: res.status };
   const data = (await res.json()) as { content?: string };
-  return typeof data.content === "string" ? data.content : undefined;
+  // `optional=1` answers a missing file with 200 + `content: ""`, so a
+  // non-string here means a response shape we did not expect, not absence.
+  if (typeof data.content !== "string") return { ok: false, reason: "missing_content" };
+  // An empty body parses into a session with no elements, which declines every
+  // edit wholesale — not a session worth opening. The absent-file shim and a
+  // genuinely 0-byte file are the same 200 on the wire and cannot be told
+  // apart here, hence the name; for a composition it is always the former.
+  if (data.content === "") return { ok: false, reason: "absent_or_empty" };
+  return { ok: true, content: data.content };
 }
 
 /**
@@ -180,16 +208,21 @@ export function useSdkSession(
     };
 
     readProjectFileOptional(projectId, activeCompPath)
-      .then(async (content) => {
+      .then(async (read) => {
         if (cancelled) return;
-        if (typeof content !== "string") {
+        if (!read.ok) {
           // No SDK session follows, so EVERY cutover chokepoint below takes the
           // server path and emits nothing — the shadow never runs either. This
           // is the only place a missing session can originate, so a broken read
           // would otherwise be a silent, total SDK bypass.
-          trackStudioEvent("sdk_session_unavailable", { stage: "read" });
+          trackStudioEvent("sdk_session_unavailable", {
+            stage: "read",
+            reason: read.reason,
+            ...(read.reason === "http_error" ? { status: read.status } : {}),
+          });
           return;
         }
+        const content = read.content;
         // No persist queue: Studio's writeProjectFile (via sdkCutover's
         // persistSdkSerialize) is the SINGLE writer. Wiring the SDK persist
         // queue too would double-write the file (queue auto-writes on every