mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 09:16:55 +00:00
fix(studio): cutover gate classification, SSE cross-project bleed, benign races (#4269)
Implements the fixes from the 2026-09-21 cutover dashboard read
(plans/sdk/2026-09-21-cutover-dashboard-fixes-plan.md):
- sdk_cutover_failed now carries `family` and `error_kind`
(network/conflict/http/sdk). The one production gate event was a raw
`TypeError: Failed to fetch` from an unwrapped fetch (readProjectFile /
writeProjectFile's preflight), which an instanceof-only classifier would
still file as "sdk" and trip the rollback gate — added a message-based
fallback for the browser's fetch-rejection strings.
- studio:sdk_session_unavailable's `network` reason now carries `elapsed_ms`
and `hidden` (document.visibilityState at rejection), separating a
near-instant policy block (CSP/PNA/extension) from a dropped connection
the tab or server going away mid-flight.
- The ownership-race branch in useSdkSession's open effect now emits
sdk_session_superseded instead of sdk_session_unavailable{stage:ownership}
— it's a benign race (identity moved on mid-open), not a broken
precondition, and was being counted as one on the dashboard.
- Root-caused and fixed the read-404 "storm": /api/events (CLI host) and
the vite-dev hot-reload channel are one connection per SERVER, not per
project. A stale tab left open after `hyperframes preview` restarts on a
different folder (same port) received every save the CURRENT project
made, reloading its preview (which then 404s) and re-reading its own
composition on each one. Both transports now carry `projectId`; the
client-side coordinator drops a delivery whose projectId doesn't match
the tab's (absent field still passes, for one release of skew).
- studio-server's file routes report `why` on 403/404: `project_dir_missing`
(new — the CLI host's static resolveProject kept answering for a project
whose folder was renamed/deleted under a running server, which used to
fail closed as an indistinguishable path-traversal 403) vs `nul` vs
`outside_project`.
- absent_or_empty reads now carry `path_in_tree` (against the caller's
loaded file tree). Instrumentation only, per the graveyard page
(clearing-activecomppath-on-tree-absence) that refuted the "clear
activeCompPath" fix this was the proposed next step for.
Tests: unit coverage added for every change (sdkCutover.test.ts,
useSdkSession.lifecycle.test.tsx, useExternalFileChangeCoordinator.test.tsx,
studioServer.test.ts, files.pathSafety.test.ts). oxlint/oxfmt clean,
tsc --noEmit clean on studio/studio-server/cli.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
01734fae3c
commit
24c3e197ef
@@ -269,4 +269,23 @@ describe("Studio file-change SSE", () => {
|
||||
expect(payload).toContain(encodedVersion("<html>agent</html>"));
|
||||
}
|
||||
});
|
||||
|
||||
// `/api/events` is one connection per SERVER, not per project: a tab left
|
||||
// open from a `preview` run whose port was later reused by a DIFFERENT
|
||||
// project shares this exact stream. Without `projectId` on the wire, that
|
||||
// stale tab cannot tell "my project changed" from "the other project this
|
||||
// server now serves changed" — see useExternalFileChangeCoordinator's
|
||||
// cross-project filter, which reads this field.
|
||||
it("labels every file-change with this server's project id", async () => {
|
||||
const projectDir = tmpProject();
|
||||
writeFileSync(join(projectDir, "index.html"), "<html>before</html>");
|
||||
server = createStudioServer({ projectDir, projectName: "demo-project" });
|
||||
const streams = await subscribe(1);
|
||||
|
||||
writeFileSync(join(projectDir, "index.html"), "<html>agent</html>");
|
||||
mockWatcher.emit("change", "change", "index.html");
|
||||
|
||||
const [payload] = await Promise.all(streams.map(nextEvent));
|
||||
expect(payload).toContain('"projectId":"demo-project"');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -788,8 +788,20 @@ export function createStudioServer(options: StudioServerOptions): StudioServer {
|
||||
// identity for an unlabelled change, and without it every duplicate
|
||||
// delivery of one watcher event drains and reloads again.
|
||||
const receipt = version ? identifyFileWrite(absPath, version) : null;
|
||||
// `projectId` so a stale tab — one still pointed at a project this
|
||||
// server no longer serves, because `hyperframes preview` reused this
|
||||
// port for a different folder (see ProjectUnreachableBanner's doc
|
||||
// comment) — can tell "my project changed" from "some OTHER project,
|
||||
// now served on this same connection, changed". Every subscriber on
|
||||
// this port shares one `/api/events` stream regardless of which
|
||||
// project their tab was opened for; without this field a stale tab
|
||||
// reloads its preview and re-reads its composition on every save the
|
||||
// CURRENT project makes, 404ing each time.
|
||||
stream
|
||||
.writeSSE({ event: "file-change", data: JSON.stringify({ path, version, ...receipt }) })
|
||||
.writeSSE({
|
||||
event: "file-change",
|
||||
data: JSON.stringify({ path, version, projectId: project.id, ...receipt }),
|
||||
})
|
||||
.catch(() => {});
|
||||
};
|
||||
// Re-applied here because the watcher now also emits the signature
|
||||
|
||||
@@ -203,6 +203,40 @@ describe("file route containment", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveProjectPath why", () => {
|
||||
// The CLI host's `resolveProject` is static — it answers with this project
|
||||
// for as long as the server runs, even after its folder is renamed or
|
||||
// deleted. Before this fix that produced a 403 "forbidden" (isSafePath
|
||||
// fails closed when its base doesn't exist), indistinguishable from a real
|
||||
// path-traversal attempt. This must be a 404 with its own `why`, checked
|
||||
// BEFORE the NUL/traversal checks so it wins when both are true.
|
||||
it("reports a missing project directory as 404, not 403", async () => {
|
||||
const { app, project } = fixture();
|
||||
rmSync(project, { recursive: true, force: true });
|
||||
|
||||
const response = await app.request(fileUrl("inside.txt"));
|
||||
|
||||
expect(response.status).toBe(404);
|
||||
expect(await response.json()).toMatchObject({ why: "project_dir_missing" });
|
||||
});
|
||||
|
||||
it("still reports a NUL byte as 403 with its own why when the project exists", async () => {
|
||||
const response = await fixture().app.request(fileUrl("inside.txt\0"));
|
||||
|
||||
expect(response.status).toBe(403);
|
||||
expect(await response.json()).toMatchObject({ why: "nul" });
|
||||
});
|
||||
|
||||
it("still reports an escaping path as 403 with its own why when the project exists", async () => {
|
||||
const { app, project, outside } = fixture();
|
||||
|
||||
const response = await app.request(fileUrl(relative(project, join(outside, "secret.txt"))));
|
||||
|
||||
expect(response.status).toBe(403);
|
||||
expect(await response.json()).toMatchObject({ why: "outside_project" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("upload collision races", () => {
|
||||
function raceDuringRead(filename: string, collide: () => void) {
|
||||
const file = new File(["new upload"], filename);
|
||||
|
||||
@@ -138,14 +138,28 @@ async function resolveProjectPath(
|
||||
return { error: c.json({ error: "not found" }, 404) } as const;
|
||||
}
|
||||
|
||||
// The CLI host's `resolveProject` is static (`id === projectId ? project : null`),
|
||||
// so it keeps answering with this project even after its folder is renamed
|
||||
// or deleted out from under a running `preview` — mount-time validation
|
||||
// (`/api/projects/:id`) still passes, so nothing upstream catches this.
|
||||
// Every subsequent read then failed closed inside `isSafePath` (its
|
||||
// `realpathSync(base)` throws when the base itself is gone) and reported as
|
||||
// `403 forbidden` — indistinguishable from a real path-traversal attempt.
|
||||
// Checked here, once, so every route built on this shares the fix.
|
||||
if (!existsSync(project.dir)) {
|
||||
return {
|
||||
error: c.json({ error: "not found", why: "project_dir_missing" }, 404),
|
||||
} as const;
|
||||
}
|
||||
|
||||
const filePath = decodeURIComponent(c.req.path.replace(pathPrefix(project.id), ""));
|
||||
if (filePath.includes("\0")) {
|
||||
return { error: c.json({ error: "forbidden" }, 403) } as const;
|
||||
return { error: c.json({ error: "forbidden", why: "nul" }, 403) } as const;
|
||||
}
|
||||
|
||||
const absPath = resolveWithinProject(project.dir, filePath);
|
||||
if (!absPath) {
|
||||
return { error: c.json({ error: "forbidden" }, 403) } as const;
|
||||
return { error: c.json({ error: "forbidden", why: "outside_project" }, 403) } as const;
|
||||
}
|
||||
|
||||
if (opts?.mustExist && !existsSync(absPath)) {
|
||||
|
||||
@@ -130,6 +130,8 @@ export function StudioApp({ readOnlyPreview = false, readOnlyPreviewReason }: St
|
||||
projectId,
|
||||
activeCompPath,
|
||||
masterCompPath,
|
||||
fileManager.fileTree,
|
||||
fileManager.fileTreeLoaded,
|
||||
);
|
||||
const activeCompPathRef = useRef(activeCompPath);
|
||||
activeCompPathRef.current = activeCompPath;
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
// fallow-ignore-file code-duplication
|
||||
// @vitest-environment happy-dom
|
||||
import { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
@@ -376,4 +377,59 @@ describe("external file change coordinator", () => {
|
||||
expect(reloadPreview).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// `/api/events` is one connection per SERVER (CLI host), not per project — a
|
||||
// tab left open from a `preview` run whose port was later reused by a
|
||||
// different project's `preview` shares this stream with it. Both projects
|
||||
// commonly use the same default composition path, so without the filter a
|
||||
// stale tab reloads its preview and re-reads its own composition on every
|
||||
// save the OTHER project makes.
|
||||
describe("cross-project deliveries on a shared connection", () => {
|
||||
it("ignores a delivery whose projectId does not match this tab's", async () => {
|
||||
const drainPendingChanges = vi.fn(async () => ({ status: "clean" as const }));
|
||||
const reloadPreview = vi.fn();
|
||||
const reloadSdkSession = vi.fn();
|
||||
await mountCoordinator({ drainPendingChanges, reloadPreview, reloadSdkSession });
|
||||
|
||||
await act(async () =>
|
||||
handler?.({
|
||||
path: "index.html",
|
||||
content: "other project",
|
||||
version: "v9",
|
||||
projectId: "project-b",
|
||||
}),
|
||||
);
|
||||
|
||||
expect(drainPendingChanges).not.toHaveBeenCalled();
|
||||
expect(reloadPreview).not.toHaveBeenCalled();
|
||||
expect(reloadSdkSession).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still reloads for a delivery whose projectId matches this tab's", async () => {
|
||||
const reloadPreview = vi.fn();
|
||||
await mountCoordinator({ reloadPreview });
|
||||
|
||||
await act(async () =>
|
||||
handler?.({
|
||||
path: "index.html",
|
||||
content: "same project",
|
||||
version: "v9",
|
||||
projectId: "project-a",
|
||||
}),
|
||||
);
|
||||
|
||||
expect(reloadPreview).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("still reloads when projectId is absent (older server, one release of skew)", async () => {
|
||||
const reloadPreview = vi.fn();
|
||||
await mountCoordinator({ reloadPreview });
|
||||
|
||||
await act(async () =>
|
||||
handler?.({ path: "index.html", content: "no project id", version: "v9" }),
|
||||
);
|
||||
|
||||
expect(reloadPreview).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -349,6 +349,19 @@ export function useExternalFileChangeCoordinator({
|
||||
logReload("file-change", { path: null, why: path ? "no project" : "no path in payload" });
|
||||
return;
|
||||
}
|
||||
// `/api/events` is one connection per SERVER, not per project: under the
|
||||
// CLI host, a tab left open from a `preview` run that has since exited
|
||||
// shares this stream with whatever project now runs on that port. Both
|
||||
// commonly use the same default composition path, so without this check
|
||||
// a stale tab reloads its preview (which then 404s — the CLI host is
|
||||
// single-project, so it can't resolve the OTHER project it's serving)
|
||||
// and re-reads its own composition on every save the CURRENT project
|
||||
// makes. Absent field (older server, one release of skew) still passes.
|
||||
const deliveredProjectId = readFileChangeField(payload, "projectId");
|
||||
if (deliveredProjectId && deliveredProjectId !== projectId) {
|
||||
logReload("suppressed", { path, why: "other project" });
|
||||
return;
|
||||
}
|
||||
pendingTimelineEditPathRef.current.delete(path);
|
||||
|
||||
const content = readFileChangeContent(payload);
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
// fallow-ignore-file code-duplication
|
||||
// @vitest-environment happy-dom
|
||||
import { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
@@ -263,6 +264,64 @@ describe("useSdkSession unavailable telemetry", () => {
|
||||
stage: "read",
|
||||
reason: "http_error",
|
||||
status: 404,
|
||||
why: undefined,
|
||||
});
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
// `why` distinguishes the studio-server route's own 403/404 causes (the
|
||||
// project's folder having been renamed or deleted out from under a running
|
||||
// server, vs. a NUL byte, vs. a path escaping the project) from a bare
|
||||
// status code. Optional and best-effort: an older server or a non-JSON body
|
||||
// just omits it, which must not crash the read.
|
||||
it("carries the server's why when the error body has one", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
async () =>
|
||||
({
|
||||
ok: false,
|
||||
status: 404,
|
||||
json: async () => ({ error: "not found", why: "project_dir_missing" }),
|
||||
}) 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: "http_error",
|
||||
status: 404,
|
||||
why: "project_dir_missing",
|
||||
});
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
it("stays quiet about why when the response body cannot be parsed", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
async () =>
|
||||
({
|
||||
ok: false,
|
||||
status: 500,
|
||||
json: async () => {
|
||||
throw new Error("not json");
|
||||
},
|
||||
}) as unknown 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: "http_error",
|
||||
status: 500,
|
||||
why: undefined,
|
||||
});
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
@@ -287,6 +346,8 @@ describe("useSdkSession unavailable telemetry", () => {
|
||||
expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "read",
|
||||
reason: "network",
|
||||
elapsed_ms: expect.any(Number),
|
||||
hidden: expect.any(Boolean),
|
||||
});
|
||||
expect(trackMock).not.toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "open",
|
||||
@@ -295,6 +356,70 @@ describe("useSdkSession unavailable telemetry", () => {
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
// `elapsed_ms` is the discriminator this event exists for: a policy block
|
||||
// (CSP, PNA, an extension rewriting fetch) rejects near-instantly; a dropped
|
||||
// connection (the tab or `preview` server going away mid-flight) rejects
|
||||
// after a real delay. Both look identical without the timing.
|
||||
it("times the network rejection from fetch start to reject", async () => {
|
||||
// Fake only the clock `performance.now` reads, not timers: other code in
|
||||
// this tree (React's own scheduler included) also calls `performance.now`,
|
||||
// so pinning return values by call order (`mockReturnValueOnce`) is
|
||||
// unreliable — a real run showed React consuming the queued values first.
|
||||
// A fake clock that only advances when we say so sidesteps that entirely.
|
||||
vi.useFakeTimers({ toFake: ["performance"] });
|
||||
let rejectFetch: ((error: unknown) => void) | undefined;
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
() =>
|
||||
new Promise((_resolve, reject) => {
|
||||
rejectFetch = reject;
|
||||
}),
|
||||
),
|
||||
);
|
||||
const root = createRoot(document.createElement("div"));
|
||||
// The effect runs synchronously up to `await fetch(...)`, capturing
|
||||
// `fetchStarted` at the current (fake) clock value before this returns.
|
||||
await act(async () => root.render(<Probe projectId="project-a" />));
|
||||
await vi.advanceTimersByTimeAsync(3_200);
|
||||
await act(async () => rejectFetch?.(new TypeError("Failed to fetch")));
|
||||
|
||||
expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "read",
|
||||
reason: "network",
|
||||
elapsed_ms: 3_200,
|
||||
hidden: false,
|
||||
});
|
||||
vi.useRealTimers();
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
it("records the page as hidden when the rejection lands after the tab is backgrounded", async () => {
|
||||
const originalDescriptor = Object.getOwnPropertyDescriptor(document, "visibilityState");
|
||||
Object.defineProperty(document, "visibilityState", {
|
||||
configurable: true,
|
||||
get: () => "hidden",
|
||||
});
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => {
|
||||
throw new TypeError("Failed to fetch");
|
||||
}),
|
||||
);
|
||||
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: "network",
|
||||
elapsed_ms: expect.any(Number),
|
||||
hidden: true,
|
||||
});
|
||||
await act(async () => root.unmount());
|
||||
if (originalDescriptor) Object.defineProperty(document, "visibilityState", originalDescriptor);
|
||||
});
|
||||
|
||||
it("separates an unexpected response shape from a failed request", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
@@ -324,13 +449,67 @@ describe("useSdkSession unavailable telemetry", () => {
|
||||
await act(async () => root.render(<Probe projectId="project-a" />));
|
||||
await flushAsyncEffects();
|
||||
|
||||
// No fileTree passed (this Probe doesn't have one) — path_in_tree stays
|
||||
// null, same as before the tree loads. See the tree-aware tests below.
|
||||
expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "read",
|
||||
reason: "absent_or_empty",
|
||||
path_in_tree: null,
|
||||
});
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
// The graveyard-refuted fix's proposed next step: instrument, don't act.
|
||||
// `path_in_tree` separates "genuinely not in the loaded tree" from
|
||||
// "concurrent/duplicate open" without deciding anything on Studio's behalf.
|
||||
// Two separate mounts (not a re-render of one): fileTree/fileTreeLoaded are
|
||||
// deliberately outside the open effect's deps — re-running the whole
|
||||
// open/dispose cycle on every tree refresh would drop a perfectly good
|
||||
// session far more often than the tree actually changes — so a prop-only
|
||||
// change on an already-mounted probe would never re-fire the read this
|
||||
// event comes from.
|
||||
it("reports path_in_tree against the caller's loaded file tree", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => ({ ok: true, json: async () => ({ content: "" }) }) as Response),
|
||||
);
|
||||
function TreeProbe({
|
||||
fileTree,
|
||||
fileTreeLoaded,
|
||||
}: {
|
||||
fileTree: string[];
|
||||
fileTreeLoaded: boolean;
|
||||
}) {
|
||||
useSdkSession("project-a", "index.html", fileTree, fileTreeLoaded);
|
||||
return null;
|
||||
}
|
||||
|
||||
const rootA = createRoot(document.createElement("div"));
|
||||
await act(async () =>
|
||||
rootA.render(<TreeProbe fileTree={["other.html"]} fileTreeLoaded={true} />),
|
||||
);
|
||||
await flushAsyncEffects();
|
||||
expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "read",
|
||||
reason: "absent_or_empty",
|
||||
path_in_tree: false,
|
||||
});
|
||||
await act(async () => rootA.unmount());
|
||||
|
||||
trackMock.mockClear();
|
||||
const rootB = createRoot(document.createElement("div"));
|
||||
await act(async () =>
|
||||
rootB.render(<TreeProbe fileTree={["index.html"]} fileTreeLoaded={true} />),
|
||||
);
|
||||
await flushAsyncEffects();
|
||||
expect(trackMock).toHaveBeenCalledWith("sdk_session_unavailable", {
|
||||
stage: "read",
|
||||
reason: "absent_or_empty",
|
||||
path_in_tree: true,
|
||||
});
|
||||
await act(async () => rootB.unmount());
|
||||
});
|
||||
|
||||
it("reports a parse failure with its message", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
|
||||
@@ -26,13 +26,22 @@ import { addExternalFileReloadListener } from "./externalFileReloadBus";
|
||||
* "Failed to fetch", "Load failed", "NetworkError when attempting to fetch
|
||||
* resource." — so the class was a network problem filed under a parser one, and
|
||||
* unaddressable in that bucket.
|
||||
*
|
||||
* `network` carries `elapsedMs` (time from fetch start to rejection) and
|
||||
* `hidden` (`document.visibilityState` at the moment of rejection) because
|
||||
* "the fetch rejected" alone conflates two very different situations: a
|
||||
* closing tab or a server that exited under a live one (rejects after a real
|
||||
* delay, `hidden` often true by the time it lands) versus a request blocked
|
||||
* before it left the browser — CSP `connect-src`, Private Network Access, an
|
||||
* extension rewriting `fetch` (rejects near-instantly, tab stays visible and
|
||||
* alive). Telemetry before this change could not tell the two apart.
|
||||
*/
|
||||
type ProjectFileReadFailure =
|
||||
| { ok: false; reason: "unsafe_path" }
|
||||
| { ok: false; reason: "http_error"; status: number }
|
||||
| { ok: false; reason: "http_error"; status: number; why?: string }
|
||||
| { ok: false; reason: "missing_content" }
|
||||
| { ok: false; reason: "absent_or_empty" }
|
||||
| { ok: false; reason: "network" };
|
||||
| { ok: false; reason: "network"; elapsedMs: number; hidden: boolean };
|
||||
|
||||
type ProjectFileReadResult = { ok: true; content: string } | ProjectFileReadFailure;
|
||||
|
||||
@@ -54,16 +63,61 @@ type ProjectFileReadResult = { ok: true; content: string } | ProjectFileReadFail
|
||||
* called from is a long pre-existing async body already near the complexity
|
||||
* threshold, and this branch is one coherent unit.
|
||||
*/
|
||||
function reportReadFailure(read: ProjectFileReadFailure, projectId: string): string | null {
|
||||
function reportReadFailure(
|
||||
read: ProjectFileReadFailure,
|
||||
projectId: string,
|
||||
pathInTree: boolean | null,
|
||||
): string | null {
|
||||
trackStudioEvent("sdk_session_unavailable", {
|
||||
stage: "read",
|
||||
reason: read.reason,
|
||||
...(read.reason === "http_error" ? { status: read.status } : {}),
|
||||
...(read.reason === "http_error" ? { status: read.status, why: read.why } : {}),
|
||||
...(read.reason === "network" ? { elapsed_ms: read.elapsedMs, hidden: read.hidden } : {}),
|
||||
// Only meaningful for `absent_or_empty` — the graveyard-refuted "clear
|
||||
// activeCompPath when it's not in the tree" fix's proposed next step,
|
||||
// scoped to instrumentation only. `null` while the tree hasn't loaded
|
||||
// yet: a false-negative there would read as "genuinely absent" when it is
|
||||
// really "haven't looked".
|
||||
...(read.reason === "absent_or_empty" ? { path_in_tree: pathInTree } : {}),
|
||||
});
|
||||
if (read.reason !== "http_error") return null;
|
||||
return read.status === 404 ? projectId : null;
|
||||
}
|
||||
|
||||
// The request never produced a response: offline, the dev server gone, a
|
||||
// CSP/Private-Network-Access block, or an extension rewriting fetch. Called
|
||||
// from the fetch's own catch so it is reported as a READ failure — left to
|
||||
// propagate, the effect's outer catch reported it as `stage: "open"`, which
|
||||
// claims the composition failed to parse. Every `stage: open` event before
|
||||
// this change was this branch, so the label made the largest class
|
||||
// unaddressable.
|
||||
//
|
||||
// `elapsedMs` and `hidden` (read synchronously, right at the moment of
|
||||
// rejection, so they survive whenever the event itself does) are the
|
||||
// discriminator between "blocked before send" (near-zero elapsed) and "the
|
||||
// tab/server went away mid-flight" (real elapsed, often already hidden) —
|
||||
// see the type's doc comment.
|
||||
function networkReadFailure(
|
||||
fetchStarted: number,
|
||||
): Extract<ProjectFileReadFailure, { reason: "network" }> {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "network",
|
||||
elapsedMs: Math.round(performance.now() - fetchStarted),
|
||||
hidden: typeof document !== "undefined" && document.visibilityState === "hidden",
|
||||
};
|
||||
}
|
||||
|
||||
// `why` distinguishes the studio-server route's own 403/404 causes (a NUL
|
||||
// byte, a path escaping the project, or — the one that used to read as a
|
||||
// plain path-traversal 403 — this project's folder having been renamed or
|
||||
// deleted out from under a still-running server) that a bare status code
|
||||
// cannot. Best-effort: an older server or a non-JSON error body just omits it.
|
||||
async function httpReadFailureWhy(res: Response): Promise<string | undefined> {
|
||||
const body = (await res.json().catch(() => null)) as { why?: unknown } | null;
|
||||
return typeof body?.why === "string" ? body.why : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
@@ -80,20 +134,18 @@ async function readProjectFileOptional(
|
||||
// already confines both values to single segments of this same-origin URL.
|
||||
if (path.includes("\0") || path.includes("..")) return { ok: false, reason: "unsafe_path" };
|
||||
let res: Response;
|
||||
const fetchStarted = performance.now();
|
||||
try {
|
||||
res = await fetch(
|
||||
`/api/projects/${encodeURIComponent(projectId)}/files/${encodeURIComponent(path)}?optional=1`,
|
||||
);
|
||||
} catch {
|
||||
// The request never produced a response: offline, the dev server gone, a
|
||||
// CSP/Private-Network-Access block, or an extension rewriting fetch. Caught
|
||||
// here so it is reported as a READ failure — left to propagate, the effect's
|
||||
// outer catch reported it as `stage: "open"`, which claims the composition
|
||||
// failed to parse. Every `stage: open` event before this change was this
|
||||
// branch, so the label made the largest class unaddressable.
|
||||
return { ok: false, reason: "network" };
|
||||
return networkReadFailure(fetchStarted);
|
||||
}
|
||||
if (!res.ok) {
|
||||
const why = await httpReadFailureWhy(res);
|
||||
return { ok: false, reason: "http_error", status: res.status, ...(why ? { why } : {}) };
|
||||
}
|
||||
if (!res.ok) return { ok: false, reason: "http_error", status: res.status };
|
||||
const data = (await res.json()) as { content?: string };
|
||||
// `optional=1` answers a missing file with 200 + `content: ""`, so a
|
||||
// non-string here means a response shape we did not expect, not absence.
|
||||
@@ -224,6 +276,13 @@ function disposeSdkSession(session: Composition): void {
|
||||
export function useSdkSession(
|
||||
projectId: string | null,
|
||||
activeCompPath: string | null,
|
||||
// Optional: only the app's primary session (App.tsx, wired to
|
||||
// useFileManager's tree) can answer `path_in_tree` on an `absent_or_empty`
|
||||
// read. A secondary session opened for a promote/bind target
|
||||
// (DesignPanelPromoteProvider) has no tree of its own to check against and
|
||||
// reports `null`, same as before the tree loads.
|
||||
fileTree: readonly string[] = [],
|
||||
fileTreeLoaded = false,
|
||||
): SdkSessionHandle {
|
||||
const [ownedSession, setOwnedSession] = useState<OwnedSdkSession | null>(null);
|
||||
const ownedSessionRef = useRef<OwnedSdkSession | null>(null);
|
||||
@@ -276,7 +335,8 @@ export function useSdkSession(
|
||||
.then(async (read) => {
|
||||
if (cancelled) return;
|
||||
if (!read.ok) {
|
||||
setUnreachableProject(reportReadFailure(read, projectId));
|
||||
const pathInTree = fileTreeLoaded ? fileTree.includes(activeCompPath) : null;
|
||||
setUnreachableProject(reportReadFailure(read, projectId, pathInTree));
|
||||
return;
|
||||
}
|
||||
setUnreachableProject(null);
|
||||
@@ -305,7 +365,11 @@ export function useSdkSession(
|
||||
)
|
||||
) {
|
||||
disposeSdkSession(comp);
|
||||
trackStudioEvent("sdk_session_unavailable", { stage: "ownership" });
|
||||
// Not a failure: project/path/reloadToken moved on while this open was
|
||||
// in flight, and the effect that owns the new identity already opened
|
||||
// (or is opening) its own session. Emitting this as `sdk_session_unavailable`
|
||||
// counted a benign race as a broken precondition on the cutover dashboard.
|
||||
trackStudioEvent("sdk_session_superseded", {});
|
||||
return;
|
||||
}
|
||||
const displaced = ownedSessionRef.current;
|
||||
@@ -343,6 +407,12 @@ export function useSdkSession(
|
||||
disposeSdkSession(owned.session);
|
||||
}
|
||||
};
|
||||
// fileTree/fileTreeLoaded deliberately excluded: they only annotate a
|
||||
// read-failure event fired from this same effect run (the diagnostic
|
||||
// "was the path in the last-loaded tree" snapshot), and re-running the
|
||||
// whole open/dispose cycle on every tree refresh would drop and reopen a
|
||||
// perfectly good session far more often than the tree actually changes.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [projectId, activeCompPath, reloadToken]);
|
||||
|
||||
const forceReload = useCallback(() => setReloadToken((t) => t + 1), []);
|
||||
|
||||
@@ -18,12 +18,19 @@ export function useStudioSdkSessions(
|
||||
projectId: string | null,
|
||||
activeCompPath: string | null,
|
||||
masterCompPath: string | null,
|
||||
fileTree: readonly string[] = [],
|
||||
fileTreeLoaded = false,
|
||||
) {
|
||||
// On the master view (no explicit comp) the schema panels target the project's
|
||||
// resolved main composition — the first `.html` in the tree, not a hardcoded
|
||||
// "index.html" that may not exist. `null` when the project has no composition
|
||||
// yet, which correctly leaves the session (and the panels) empty.
|
||||
const sdkHandle = useSdkSession(projectId, activeCompPath ?? masterCompPath);
|
||||
const sdkHandle = useSdkSession(
|
||||
projectId,
|
||||
activeCompPath ?? masterCompPath,
|
||||
fileTree,
|
||||
fileTreeLoaded,
|
||||
);
|
||||
const editFlowSdkSession = activeCompPath ? sdkHandle.session : null;
|
||||
useEffect(() => {
|
||||
usePreviewVariablesStore.getState().setValues(null);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
shouldUseSdkCutover,
|
||||
sdkCutoverPersist,
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
import { openComposition } from "@hyperframes/sdk";
|
||||
import { createMemoryAdapter } from "@hyperframes/sdk/adapters/memory";
|
||||
import type { PatchOperation } from "./sourcePatcher";
|
||||
import { StudioSaveNetworkError } from "./studioSaveDiagnostics";
|
||||
|
||||
vi.mock("../components/editor/manualEditingAvailability", () => ({
|
||||
STUDIO_SDK_CUTOVER_ENABLED: true,
|
||||
@@ -1404,3 +1405,103 @@ gsap.timeline().to('[data-hf-id="hf-layer"]', { duration: 1, x: 100 });
|
||||
expect(written).toContain("gsap.timeline()");
|
||||
});
|
||||
});
|
||||
|
||||
describe("sdk_cutover_failed classification (error_kind)", () => {
|
||||
const makeDeps = (overrides: Partial<Parameters<typeof sdkCutoverPersist>[5]> = {}) => ({
|
||||
editHistory: { recordEdit: vi.fn().mockResolvedValue(undefined) },
|
||||
writeProjectFile: vi.fn().mockResolvedValue(undefined),
|
||||
reloadPreview: vi.fn(),
|
||||
...candidateTestDeps(),
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const makeSession = () =>
|
||||
({
|
||||
getElement: vi.fn().mockReturnValue({ inlineStyles: {} }),
|
||||
dispatch: vi.fn(),
|
||||
serialize: vi
|
||||
.fn()
|
||||
.mockReturnValueOnce("<html>before</html>")
|
||||
.mockReturnValue("<html></html>"),
|
||||
batch: vi.fn((fn: () => void) => fn()),
|
||||
}) as unknown as Parameters<typeof sdkCutoverPersist>[4];
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(trackStudioEvent).mockClear();
|
||||
});
|
||||
|
||||
// The production gate event this fix exists for: a bare `TypeError` from an
|
||||
// UNWRAPPED raw fetch (`readProjectFile`'s implementation in
|
||||
// useProjectFileWriter.ts, or writeProjectFile's own preflight — neither
|
||||
// wraps as StudioSaveNetworkError). Without the message-based fallback in
|
||||
// `cutoverErrorKind`, this files as `error_kind: "sdk"` and trips the
|
||||
// cutover-failure rollback gate on a network blip the SDK never owned.
|
||||
it("classifies a bare fetch TypeError from readProjectFile as network", async () => {
|
||||
const deps = makeDeps({
|
||||
readProjectFile: vi.fn().mockRejectedValue(new TypeError("Failed to fetch")),
|
||||
});
|
||||
const session = makeSession();
|
||||
const sel = { hfId: "hf-abc" } as never;
|
||||
|
||||
const result = await sdkCutoverPersist(
|
||||
sel,
|
||||
[styleOp("color", "red")],
|
||||
"before",
|
||||
"/path.html",
|
||||
session,
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result.status).toBe("failed");
|
||||
expect(trackStudioEvent).toHaveBeenCalledWith(
|
||||
"sdk_cutover_failed",
|
||||
expect.objectContaining({ family: "dom", error_kind: "network" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies StudioSaveNetworkError from writeProjectFile as network", async () => {
|
||||
const deps = makeDeps({
|
||||
writeProjectFile: vi.fn().mockRejectedValue(new StudioSaveNetworkError("Failed to save")),
|
||||
});
|
||||
const session = makeSession();
|
||||
const sel = { hfId: "hf-abc" } as never;
|
||||
|
||||
const result = await sdkCutoverPersist(
|
||||
sel,
|
||||
[styleOp("color", "red")],
|
||||
"before",
|
||||
"/path.html",
|
||||
session,
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result.status).toBe("failed");
|
||||
expect(trackStudioEvent).toHaveBeenCalledWith(
|
||||
"sdk_cutover_failed",
|
||||
expect.objectContaining({ family: "dom", error_kind: "network" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies a plain error as an sdk defect, not network", async () => {
|
||||
const deps = makeDeps({
|
||||
writeProjectFile: vi.fn().mockRejectedValue(new Error("disk full")),
|
||||
});
|
||||
const session = makeSession();
|
||||
const sel = { hfId: "hf-abc" } as never;
|
||||
|
||||
const result = await sdkCutoverPersist(
|
||||
sel,
|
||||
[styleOp("color", "red")],
|
||||
"before",
|
||||
"/path.html",
|
||||
session,
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result.status).toBe("failed");
|
||||
expect(trackStudioEvent).toHaveBeenCalledWith(
|
||||
"sdk_cutover_failed",
|
||||
expect.objectContaining({ family: "dom", error_kind: "sdk" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,8 +11,8 @@ import {
|
||||
shouldDeclineTextCutoverForTarget,
|
||||
} from "./sdkCutoverEligibility";
|
||||
import {
|
||||
asCutoverError,
|
||||
declinedCutover,
|
||||
failedCutover,
|
||||
persistSdkCandidateMutation,
|
||||
type CutoverDeps,
|
||||
type CutoverOptions,
|
||||
@@ -45,12 +45,13 @@ function sdkFamilyEnabled(family: StudioSdkOperationFamily): boolean {
|
||||
|
||||
function trackCutoverResult(
|
||||
result: CutoverResult,
|
||||
family: StudioSdkOperationFamily,
|
||||
context: { hfId?: string | null; opCount: number },
|
||||
): void {
|
||||
if (result.status === "committed") {
|
||||
trackStudioEvent("sdk_cutover_success", context);
|
||||
trackStudioEvent("sdk_cutover_success", { ...context, family });
|
||||
} else if (result.status === "failed") {
|
||||
trackStudioEvent("sdk_cutover_failed", { ...context, error: result.error.message });
|
||||
failedCutover(result.error, family, context);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -106,7 +107,7 @@ export async function sdkCutoverPersist(
|
||||
},
|
||||
options,
|
||||
);
|
||||
trackCutoverResult(result, { hfId, opCount: ops.length });
|
||||
trackCutoverResult(result, "dom", { hfId, opCount: ops.length });
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -148,12 +149,10 @@ export async function sdkTimingPersist(
|
||||
options,
|
||||
serializedBefore,
|
||||
);
|
||||
trackCutoverResult(result, { hfId, opCount: 1 });
|
||||
trackCutoverResult(result, "timing", { hfId, opCount: 1 });
|
||||
return result;
|
||||
} catch (error) {
|
||||
const failed = { status: "failed", error: asCutoverError(error) } as const;
|
||||
trackStudioEvent("sdk_cutover_failed", { hfId, error: failed.error.message });
|
||||
return failed;
|
||||
return failedCutover(error, "timing", { hfId, opCount: 1 });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -202,24 +201,22 @@ export async function sdkTimingBatchPersist(
|
||||
serializedBefore,
|
||||
);
|
||||
if (result.status === "failed") {
|
||||
trackStudioEvent("sdk_cutover_failed", {
|
||||
return failedCutover(result.error, "timing", {
|
||||
hfId: changes[0]?.hfId ?? null,
|
||||
error: result.error.message,
|
||||
opCount: changes.length,
|
||||
});
|
||||
return result;
|
||||
}
|
||||
trackStudioEvent("sdk_cutover_success", {
|
||||
hfId: changes[0]?.hfId ?? null,
|
||||
opCount: changes.length,
|
||||
family: "timing",
|
||||
});
|
||||
return result;
|
||||
} catch (error) {
|
||||
const failed = { status: "failed", error: asCutoverError(error) } as const;
|
||||
trackStudioEvent("sdk_cutover_failed", {
|
||||
return failedCutover(error, "timing", {
|
||||
hfId: changes[0]?.hfId ?? null,
|
||||
error: failed.error.message,
|
||||
opCount: changes.length,
|
||||
});
|
||||
return failed;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -321,15 +318,13 @@ async function dispatchGsapOpAndPersist(
|
||||
serializedBefore,
|
||||
);
|
||||
if (result.status === "committed") {
|
||||
trackStudioEvent("sdk_cutover_success", { opCount: 1 });
|
||||
trackStudioEvent("sdk_cutover_success", { opCount: 1, family });
|
||||
} else if (result.status === "failed") {
|
||||
trackStudioEvent("sdk_cutover_failed", { error: result.error.message });
|
||||
return failedCutover(result.error, family, { opCount: 1 });
|
||||
}
|
||||
return result;
|
||||
} catch (error) {
|
||||
const failed = { status: "failed", error: asCutoverError(error) } as const;
|
||||
trackStudioEvent("sdk_cutover_failed", { error: failed.error.message });
|
||||
return failed;
|
||||
return failedCutover(error, family, { opCount: 1 });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -595,6 +590,6 @@ export async function sdkDeletePersist(
|
||||
(session) => session.removeElement(hfId),
|
||||
{ label: "Delete element" },
|
||||
);
|
||||
trackCutoverResult(result, { hfId, opCount: 1 });
|
||||
trackCutoverResult(result, "lifecycle", { hfId, opCount: 1 });
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -4,6 +4,11 @@ import { hashContent, markSelfWrite } from "../hooks/sdkSelfWriteRegistry";
|
||||
import { trackStudioEvent } from "./studioTelemetry";
|
||||
import { serializeStudioFileMutation } from "./studioFileMutationCoordinator";
|
||||
import type { StudioSdkOperationFamily } from "./sdkCutoverPolicy";
|
||||
import {
|
||||
StudioFileConflictError,
|
||||
StudioSaveHttpError,
|
||||
StudioSaveNetworkError,
|
||||
} from "./studioSaveDiagnostics";
|
||||
|
||||
export type CutoverResult =
|
||||
| { status: "declined"; reason: string }
|
||||
@@ -91,10 +96,54 @@ export function declinedCutover(
|
||||
return { status: "declined", reason };
|
||||
}
|
||||
|
||||
export function asCutoverError(error: unknown): Error {
|
||||
function asCutoverError(error: unknown): Error {
|
||||
return error instanceof Error ? error : new Error(String(error));
|
||||
}
|
||||
|
||||
// Browser rejection strings for a fetch that never produced a response (offline,
|
||||
// the dev server gone, a CSP/PNA block, an extension rewriting fetch). See
|
||||
// `useSdkSession.ts`'s `readProjectFileOptional` catch for the same list.
|
||||
const FETCH_REJECTION_MESSAGE = /Failed to fetch|Load failed|NetworkError when attempting to fetch/;
|
||||
|
||||
/**
|
||||
* Classify a cutover failure for the gate dashboard. `writeProjectFile`'s own
|
||||
* fetch wraps a rejection as `StudioSaveNetworkError`, but two raw fetches in
|
||||
* this same transaction — `readProjectFile` (via `captureOnDiskBefore`) and
|
||||
* `writeProjectFile`'s own read-before-write preflight — throw an unwrapped
|
||||
* `TypeError` that reaches here undecorated. Without the message fallback,
|
||||
* those file as `"sdk"` and trip the cutover-failure rollback gate on a
|
||||
* network blip the SDK never had a chance to own.
|
||||
*/
|
||||
function cutoverErrorKind(error: Error): "network" | "conflict" | "http" | "sdk" {
|
||||
if (error instanceof StudioSaveNetworkError) return "network";
|
||||
if (error instanceof StudioFileConflictError) return "conflict";
|
||||
if (error instanceof StudioSaveHttpError) return "http";
|
||||
if (error.name === "TypeError" && FETCH_REJECTION_MESSAGE.test(error.message)) return "network";
|
||||
return "sdk";
|
||||
}
|
||||
|
||||
/**
|
||||
* Record a cutover that could not commit. Replaces ad hoc `trackStudioEvent("sdk_cutover_failed", ...)`
|
||||
* calls so every gate event carries `family` and `error_kind` — without them the
|
||||
* dashboard's rollback gate (any `sdk_cutover_failed` on a flipped version) cannot
|
||||
* distinguish a real SDK defect from a rejected fetch it doesn't own.
|
||||
*/
|
||||
export function failedCutover(
|
||||
error: unknown,
|
||||
family: StudioSdkOperationFamily,
|
||||
context: { hfId?: string | null; opCount?: number } = {},
|
||||
): CutoverResult {
|
||||
const err = asCutoverError(error);
|
||||
trackStudioEvent("sdk_cutover_failed", {
|
||||
...context,
|
||||
family,
|
||||
error_kind: cutoverErrorKind(err),
|
||||
...(err instanceof StudioSaveHttpError ? { status: err.statusCode } : {}),
|
||||
error: err.message,
|
||||
});
|
||||
return { status: "failed", error: err };
|
||||
}
|
||||
|
||||
/** Only an explicit decline may enter the legacy mutation backend. */
|
||||
export function cutoverCommittedOrThrow(result: CutoverResult): boolean {
|
||||
if (result.status === "failed") throw result.error;
|
||||
|
||||
@@ -9,7 +9,7 @@ import {
|
||||
lstatSync,
|
||||
realpathSync,
|
||||
} from "node:fs";
|
||||
import { join, resolve } from "node:path";
|
||||
import { join, relative, resolve, sep } from "node:path";
|
||||
import { readNodeRequestBody } from "./vite.request-body.js";
|
||||
import { watch } from "chokidar";
|
||||
import { createProjectSignatureCache, createViteAdapter } from "./vite.adapter";
|
||||
@@ -258,10 +258,17 @@ function devProjectApi(): Plugin {
|
||||
}
|
||||
const receipt =
|
||||
version && studioServer ? studioServer.identifyFileWrite(filePath, version) : null;
|
||||
// First path segment under `dataDir` is the project id (`data/projects/<id>/...`).
|
||||
// Mirrors the CLI host's `project.id` field on the same event — see its
|
||||
// doc comment for why a stale tab needs this to ignore another
|
||||
// project's saves on a shared connection. This host is multi-project
|
||||
// (any dir under `dataDir` resolves), so unlike the CLI host it can't
|
||||
// assume one fixed id.
|
||||
const projectId = relative(dataDir, filePath).split(sep)[0];
|
||||
server.ws.send({
|
||||
type: "custom",
|
||||
event: "hf:file-change",
|
||||
data: { path: filePath, version, ...receipt },
|
||||
data: { path: filePath, version, projectId, ...receipt },
|
||||
});
|
||||
});
|
||||
server.httpServer?.on("close", () => void projectWatcher.close());
|
||||
|
||||
Reference in New Issue
Block a user