From c59326d72eae2eaf3503e44be0e722568e19b210 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 23 Sep 2026 20:13:21 -0700 Subject: [PATCH] fix(studio): refresh the file tree on external change, and say when a composition is gone (#4370) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix the cutover-dashboard investigation this week was building toward. Proven 2026-09-23: `refreshFileTree` (useFileTree) is only ever called from Studio's OWN file operations (create/delete/rename/upload/block install) — never on an external change. An agent removing or replacing a composition updates the preview and the SDK session (useExternalFileChangeCoordinator already reloads both) but never the tree, so the file stays listed and clickable forever. Opening it opens a session that can never resolve (`reason: "absent"`), every edit through it silently takes the (also failing) server path, and nothing tells the user why. Two refresh triggers, belt and braces: - `useExternalFileChangeCoordinator` gains an optional `refreshFileTree` collaborator, called alongside `reloadPreview`/`reloadSdkSession` on every accepted external change — closes staleness at the source, for every consumer of the tree, not just the SDK read. Wired through `useStudioExternalFileChanges` from `fileManager.refreshFileTree` (already in scope there). - `useSdkSession` gains `onAbsentRead`, called once per (projectId, path) when a read reports `reason: "absent"` — the fallback for a change the SSE never delivered (server restart, a missed watcher event). Guarded by a ref-tracked set so a genuinely-gone file can't loop, cleared on project change so a different project's identical path can still fire. New: `sdkHandle.compositionMissing`, true for exactly that read result on the currently active path, and `CompositionMissingBanner` — sibling of `ProjectUnreachableBanner` (that one fires when the whole PROJECT is unreachable; this one when the project is fine and this one file inside it is not) — shown in App.tsx alongside it. The read failure alone is silent; this is what tells the user their edits stopped landing. Deliberately not touched: `activeCompPath` is never cleared when the path leaves the tree — the graveyard page (`clearing-activecomppath-on-tree- absence`) is binding, and its reasons still hold (`useAutoOpenRootComposition` latches once; a null path never recovers). Tests: coordinator refresh fires once per accepted change, not for a self-write echo, and no-ops when the collaborator is absent (4 new). useSdkSession: compositionMissing set/cleared, onAbsentRead once per path, fires again for a different path, guard resets on project change, no-ops without the collaborator (5 new). Banner render test (1 new). 25/25 in the touched suites; 2830/2831 across studio hooks+components overall (the one failure is a pre-existing happy-dom `window.confirm` gap in ExternalFileConflictBanner.test.tsx, confirmed unrelated via git stash). oxlint, oxfmt, tsc --noEmit clean. Co-authored-by: Claude Sonnet 5 --- packages/studio/src/App.tsx | 5 + .../CompositionMissingBanner.test.tsx | 35 +++++ .../components/CompositionMissingBanner.tsx | 32 ++++ .../useExternalFileChangeCoordinator.test.tsx | 37 ++++- .../hooks/useExternalFileChangeCoordinator.ts | 20 ++- .../hooks/useSdkSession.lifecycle.test.tsx | 146 ++++++++++++++++++ packages/studio/src/hooks/useSdkSession.ts | 64 +++++++- .../src/hooks/useStudioExternalFileChanges.ts | 2 + .../studio/src/hooks/useStudioSdkSessions.ts | 2 + 9 files changed, 333 insertions(+), 10 deletions(-) create mode 100644 packages/studio/src/components/CompositionMissingBanner.test.tsx create mode 100644 packages/studio/src/components/CompositionMissingBanner.tsx diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index d4c6f9792c..dff32fe210 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -8,6 +8,7 @@ import { StudioOverlays } from "./components/StudioOverlays"; import { SaveQueuePausedBanner } from "./components/SaveQueuePausedBanner"; import { ExternalFileConflictBanner } from "./components/ExternalFileConflictBanner"; import { ProjectUnreachableBanner } from "./components/ProjectUnreachableBanner"; +import { CompositionMissingBanner } from "./components/CompositionMissingBanner"; import { useCaptionStore } from "./captions/store"; import { useCaptionSync } from "./captions/hooks/useCaptionSync"; import { usePersistentEditHistory } from "./hooks/usePersistentEditHistory"; @@ -132,6 +133,7 @@ export function StudioApp({ readOnlyPreview = false, readOnlyPreviewReason }: St masterCompPath, fileManager.fileTree, fileManager.fileTreeLoaded, + fileManager.refreshFileTree, ); const activeCompPathRef = useRef(activeCompPath); activeCompPathRef.current = activeCompPath; @@ -481,6 +483,9 @@ export function StudioApp({ readOnlyPreview = false, readOnlyPreviewReason }: St {sdkHandle.unreachableProject && ( )} + {sdkHandle.compositionMissing && activeCompPath && ( + + )} { + if (root) act(() => root?.unmount()); + root = null; + document.body.replaceChildren(); +}); + +describe("CompositionMissingBanner", () => { + it("names the missing composition and does not claim the whole project is gone", async () => { + const host = document.createElement("div"); + document.body.append(host); + root = createRoot(host); + await act(async () => { + root?.render(); + }); + + expect(host.textContent).toContain("scenes/intro.html"); + expect(host.textContent).toContain("no longer on disk"); + // Distinct from ProjectUnreachableBanner's wording — this is one file + // inside a project that resolves fine, not the whole project. + expect(host.textContent).not.toContain("This Studio is serving"); + }); +}); diff --git a/packages/studio/src/components/CompositionMissingBanner.tsx b/packages/studio/src/components/CompositionMissingBanner.tsx new file mode 100644 index 0000000000..9dd645fcac --- /dev/null +++ b/packages/studio/src/components/CompositionMissingBanner.tsx @@ -0,0 +1,32 @@ +interface CompositionMissingBannerProps { + /** The composition this tab has open, that the server can no longer find. */ + path: string; +} + +/** + * Shown when the currently open composition's file has vanished from disk + * mid-session — the `absent` read reason (#4325): the file tree listed it and + * the read cannot get it. + * + * Distinct from `ProjectUnreachableBanner`: there the whole PROJECT fails to + * resolve; here the project is fine and this one file inside it is not. Proven + * 2026-09-23 to be a stale tree, not a placeholder or a path-encoding gap: + * `refreshFileTree` only ran after Studio's own file operations, so an agent + * removing or replacing a composition updated the preview and the SDK session + * but never the listing — the file stayed clickable, and every edit through + * it failed silently, with nothing to tell the user why. The tree is now + * refreshed as soon as this fires (`useSdkSession`'s `onAbsentRead`); this + * banner is the only thing that explains the failure while that catches up. + */ +export function CompositionMissingBanner({ path }: CompositionMissingBannerProps) { + return ( +
+ + {path} is no longer on disk. Select another composition to keep editing. + +
+ ); +} diff --git a/packages/studio/src/hooks/useExternalFileChangeCoordinator.test.tsx b/packages/studio/src/hooks/useExternalFileChangeCoordinator.test.tsx index 9a8c8e7188..bac6a98b8d 100644 --- a/packages/studio/src/hooks/useExternalFileChangeCoordinator.test.tsx +++ b/packages/studio/src/hooks/useExternalFileChangeCoordinator.test.tsx @@ -74,9 +74,44 @@ describe("external file change coordinator", () => { onAcceptedPersistedFileChange: () => order.push("thumbnail"), reloadPreview: () => order.push("preview"), reloadSdkSession: () => order.push("sdk"), + refreshFileTree: () => { + order.push("tree"); + }, }); await act(async () => handler?.({ path: "index.html", content: "external", version: "v2" })); - expect(order).toEqual(["drain", "thumbnail", "preview", "sdk"]); + expect(order).toEqual(["drain", "thumbnail", "preview", "sdk", "tree"]); + expect(captured.handle?.blocked).toBeNull(); + }); + + // The file tree (useFileTree) is only ever refreshed from Studio's own file + // operations (create/delete/rename/upload) — never on an external change. + // Without this call, an agent removing or replacing a composition updates + // the preview and the SDK session but leaves the listing stale forever. + it("refreshes the file tree on an accepted external change", async () => { + const refreshFileTree = vi.fn(); + await mountCoordinator({ refreshFileTree }); + await act(async () => handler?.({ path: "index.html", content: "external", version: "v2" })); + expect(refreshFileTree).toHaveBeenCalledOnce(); + }); + + it("does not refresh the tree for a suppressed self-write echo", async () => { + const refreshFileTree = vi.fn(); + await mountCoordinator({ refreshFileTree }); + markStudioWriteToken("studio-write-1"); + await act(async () => + handler?.({ + path: "index.html", + content: "studio", + version: "v2", + writeToken: "studio-write-1", + }), + ); + expect(refreshFileTree).not.toHaveBeenCalled(); + }); + + it("is optional — an accepted change with no refreshFileTree collaborator does not throw", async () => { + const { captured } = await mountCoordinator({ refreshFileTree: undefined }); + await act(async () => handler?.({ path: "index.html", content: "external", version: "v2" })); expect(captured.handle?.blocked).toBeNull(); }); diff --git a/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts b/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts index 3c0bd11a98..d15c4aabcf 100644 --- a/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts +++ b/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts @@ -64,6 +64,17 @@ interface ExternalFileChangeCoordinatorOptions { path: string, affectedCompositions: readonly string[] | null, ) => void; + /** + * Called alongside `reloadPreview`/`reloadSdkSession` on every accepted + * external change. The file tree (`useFileTree`) is only ever refreshed + * from Studio's OWN file operations (create/delete/rename/upload) — an + * external change (an agent writing outside Studio) reloads the preview + * and the SDK session but, without this, never the listing. A composition + * an agent removed or replaced then stays in the tree until the user does + * a Studio-side file op or reloads the tab; clicking it opens a session + * that can never resolve (`reason: "absent"`, proven stale-tree 2026-09-23). + */ + refreshFileTree?: () => void | Promise; } export interface ExternalFileChangeCoordinatorHandle { @@ -149,6 +160,7 @@ export function useExternalFileChangeCoordinator({ onUseExternalFile, resetSaveQueues, onAcceptedPersistedFileChange, + refreshFileTree, }: ExternalFileChangeCoordinatorOptions): ExternalFileChangeCoordinatorHandle { const [blocked, setBlocked] = useState(null); const generationRef = useRef(0); @@ -223,8 +235,14 @@ export function useExternalFileChangeCoordinator({ logReload("reload", { path, by: "external-change coordinator" }); reloadPreview(); reloadSdkSession(path); + // Fire-and-forget: a failed refresh leaves the tree as stale as it was, + // which is the status quo this exists to improve on, not a new failure + // mode to surface. The `absent`-triggered fallback in useSdkSession + // covers the case where this call is missed entirely (server restart, + // a watcher event the SSE never delivered). + void refreshFileTree?.(); }, - [reloadPreview, reloadSdkSession], + [reloadPreview, reloadSdkSession, refreshFileTree], ); const persistSnapshotInOrder = useCallback(async (write: () => Promise) => { diff --git a/packages/studio/src/hooks/useSdkSession.lifecycle.test.tsx b/packages/studio/src/hooks/useSdkSession.lifecycle.test.tsx index 7894614166..fc8919d273 100644 --- a/packages/studio/src/hooks/useSdkSession.lifecycle.test.tsx +++ b/packages/studio/src/hooks/useSdkSession.lifecycle.test.tsx @@ -587,6 +587,152 @@ describe("useSdkSession unavailable telemetry", () => { await act(async () => rootB.unmount()); }); + // `compositionMissing` and the once-per-path refresh fallback: proven + // 2026-09-23 that `absent` means a stale tree (refreshFileTree only runs + // after Studio's own file ops, never on an external change), so an `absent` + // read is the one signal that should make the tree self-correct even when + // the SSE-driven refresh in useExternalFileChangeCoordinator is missed. + describe("compositionMissing and the absent-read refresh fallback", () => { + function HandleProbe({ + projectId, + path, + onAbsentRead, + }: { + projectId: string; + path: string; + onAbsentRead?: (path: string) => void; + }) { + captured.handle = useSdkSession(projectId, path, [], false, onAbsentRead); + return null; + } + const captured: { handle: SdkSessionHandle | null } = { handle: null }; + + it("sets compositionMissing and calls onAbsentRead once for an absent read", async () => { + vi.stubGlobal( + "fetch", + vi.fn( + async () => + ({ ok: true, json: async () => ({ content: "", missing: true }) }) as Response, + ), + ); + const onAbsentRead = vi.fn(); + const root = createRoot(document.createElement("div")); + await act(async () => + root.render( + , + ), + ); + await flushAsyncEffects(); + + expect(captured.handle?.compositionMissing).toBe(true); + expect(onAbsentRead).toHaveBeenCalledOnce(); + expect(onAbsentRead).toHaveBeenCalledWith("index.html"); + + // A second absent read for the SAME path must not refresh again — the + // refresh already ran and didn't fix it (the file really is gone). + await act(async () => { + captured.handle?.forceReload(); + }); + await flushAsyncEffects(); + expect(onAbsentRead).toHaveBeenCalledOnce(); + + await act(async () => root.unmount()); + }); + + it("calls onAbsentRead again for a different path", async () => { + vi.stubGlobal( + "fetch", + vi.fn( + async () => + ({ ok: true, json: async () => ({ content: "", missing: true }) }) as Response, + ), + ); + const onAbsentRead = vi.fn(); + const root = createRoot(document.createElement("div")); + await act(async () => + root.render( + , + ), + ); + await flushAsyncEffects(); + await act(async () => + root.render( + , + ), + ); + await flushAsyncEffects(); + + expect(onAbsentRead).toHaveBeenCalledTimes(2); + expect(onAbsentRead).toHaveBeenNthCalledWith(1, "scenes/a.html"); + expect(onAbsentRead).toHaveBeenNthCalledWith(2, "scenes/b.html"); + await act(async () => root.unmount()); + }); + + it("resets the guard on project change, so the same path can refresh again", async () => { + vi.stubGlobal( + "fetch", + vi.fn( + async () => + ({ ok: true, json: async () => ({ content: "", missing: true }) }) as Response, + ), + ); + const onAbsentRead = vi.fn(); + const root = createRoot(document.createElement("div")); + await act(async () => + root.render( + , + ), + ); + await flushAsyncEffects(); + await act(async () => + root.render( + , + ), + ); + await flushAsyncEffects(); + + expect(onAbsentRead).toHaveBeenCalledTimes(2); + await act(async () => root.unmount()); + }); + + it("clears compositionMissing once a later read succeeds", async () => { + const fetchMock = vi.fn( + async () => ({ ok: true, json: async () => ({ content: "", missing: true }) }) as Response, + ); + vi.stubGlobal("fetch", fetchMock); + openComposition.mockResolvedValue(fakeSession()); + const root = createRoot(document.createElement("div")); + await act(async () => root.render()); + await flushAsyncEffects(); + expect(captured.handle?.compositionMissing).toBe(true); + + fetchMock.mockImplementation(async () => response("PROJECT_A")); + await act(async () => { + captured.handle?.forceReload(); + }); + await flushAsyncEffects(); + + expect(captured.handle?.compositionMissing).toBe(false); + await act(async () => root.unmount()); + }); + + it("does not throw when onAbsentRead is not supplied", async () => { + vi.stubGlobal( + "fetch", + vi.fn( + async () => + ({ ok: true, json: async () => ({ content: "", missing: true }) }) as Response, + ), + ); + const root = createRoot(document.createElement("div")); + await act(async () => root.render()); + await flushAsyncEffects(); + + expect(captured.handle?.compositionMissing).toBe(true); + await act(async () => root.unmount()); + }); + }); + it("reports a parse failure with its message", async () => { vi.stubGlobal( "fetch", diff --git a/packages/studio/src/hooks/useSdkSession.ts b/packages/studio/src/hooks/useSdkSession.ts index df327fe39a..cfd8c3d617 100644 --- a/packages/studio/src/hooks/useSdkSession.ts +++ b/packages/studio/src/hooks/useSdkSession.ts @@ -273,6 +273,16 @@ export interface SdkSessionHandle { * `missing_content` say nothing about the project at all. */ unreachableProject: string | null; + /** + * True when the most recent read of `activeCompPath` reported `reason: + * "absent"` — the file tree lists this path and the server cannot get it. + * Proven 2026-09-23 to mean a stale tree: `refreshFileTree` only runs after + * Studio's own file operations, so an external change (an agent removing + * or replacing the file) updates the preview and this session but never + * the listing, and the file stays clickable forever. Every edit then fails + * silently with nothing to tell the user why — this is that signal. + */ + compositionMissing: boolean; } interface SdkSessionOwner { @@ -345,6 +355,12 @@ export function useSdkSession( // reports `null`, same as before the tree loads. fileTree: readonly string[] = [], fileTreeLoaded = false, + // Fallback for the SSE-driven refresh in useExternalFileChangeCoordinator: + // called at most once per (projectId, path) so the tree self-corrects even + // when that delivery is missed (server restart, a watcher event the SSE + // never sent). Optional for the same reason fileTree is: a secondary + // session has no tree of its own to refresh. + onAbsentRead?: (path: string) => void, ): SdkSessionHandle { const [ownedSession, setOwnedSession] = useState(null); const ownedSessionRef = useRef(null); @@ -358,6 +374,35 @@ export function useSdkSession( const reloadTokenRef = useRef(reloadToken); reloadTokenRef.current = reloadToken; const [unreachableProject, setUnreachableProject] = useState(null); + const [compositionMissing, setCompositionMissing] = useState(false); + // Keyed `${projectId}:${path}` so a refresh that doesn't fix it (the file + // really is gone) can't loop, and so it fires again for a genuinely + // different path or project. + const refreshedAbsentPathsRef = useRef>(new Set()); + useEffect(() => { + refreshedAbsentPathsRef.current.clear(); + }, [projectId]); + + /** + * Update `unreachableProject`/`compositionMissing` for one failed read, and + * fire the once-per-path tree-refresh fallback on `absent`. Pulled out of + * the open effect's `.then` — the branching there was pushing that + * callback over the complexity threshold on its own. + */ + function handleReadFailure( + read: ProjectFileReadFailure, + forProjectId: string, + forPath: string, + ): void { + const pathInTree = fileTreeLoaded ? fileTree.includes(forPath) : null; + setUnreachableProject(reportReadFailure(read, forProjectId, pathInTree)); + setCompositionMissing(read.reason === "absent"); + if (read.reason !== "absent") return; + const key = `${forProjectId}:${forPath}`; + if (refreshedAbsentPathsRef.current.has(key)) return; + refreshedAbsentPathsRef.current.add(key); + onAbsentRead?.(forPath); + } useEffect( () => @@ -381,6 +426,7 @@ export function useSdkSession( if (!projectId || !activeCompPath) { setUnreachableProject(null); + setCompositionMissing(false); return () => { cancelled = true; }; @@ -397,11 +443,11 @@ export function useSdkSession( .then(async (read) => { if (cancelled) return; if (!read.ok) { - const pathInTree = fileTreeLoaded ? fileTree.includes(activeCompPath) : null; - setUnreachableProject(reportReadFailure(read, projectId, pathInTree)); + handleReadFailure(read, projectId, activeCompPath); return; } setUnreachableProject(null); + setCompositionMissing(false); const content = read.content; // No persist queue: Studio's writeProjectFile (via sdkCutover's // persistSdkSerialize) is the SINGLE writer. Wiring the SDK persist @@ -469,11 +515,13 @@ 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. + // fileTree/fileTreeLoaded/onAbsentRead deliberately excluded: they only + // annotate or react to a read-failure event fired from this same effect + // run (the diagnostic "was the path in the last-loaded tree" snapshot, + // and the tree-refresh fallback), and re-running the whole open/dispose + // cycle on every tree refresh or on every render (callers are not + // required to memoize onAbsentRead) 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]); @@ -511,5 +559,5 @@ export function useSdkSession( ownedSession.reloadToken === reloadToken ? ownedSession.session : null; - return { session, publish, forceReload, unreachableProject }; + return { session, publish, forceReload, unreachableProject, compositionMissing }; } diff --git a/packages/studio/src/hooks/useStudioExternalFileChanges.ts b/packages/studio/src/hooks/useStudioExternalFileChanges.ts index 17dd07921d..5e42cc9602 100644 --- a/packages/studio/src/hooks/useStudioExternalFileChanges.ts +++ b/packages/studio/src/hooks/useStudioExternalFileChanges.ts @@ -20,6 +20,7 @@ type FileManager = Pick< | "overwriteExternalConflict" | "readProjectFile" | "updateEditingFileContent" + | "refreshFileTree" >; type PreviewPersistence = Pick< @@ -85,5 +86,6 @@ export function useStudioExternalFileChanges({ onUseExternalFile: fileManager.updateEditingFileContent, resetSaveQueues: resetDomEditSaveQueueBreaker, onAcceptedPersistedFileChange, + refreshFileTree: fileManager.refreshFileTree, }); } diff --git a/packages/studio/src/hooks/useStudioSdkSessions.ts b/packages/studio/src/hooks/useStudioSdkSessions.ts index 302f44cea1..648b735a9a 100644 --- a/packages/studio/src/hooks/useStudioSdkSessions.ts +++ b/packages/studio/src/hooks/useStudioSdkSessions.ts @@ -20,6 +20,7 @@ export function useStudioSdkSessions( masterCompPath: string | null, fileTree: readonly string[] = [], fileTreeLoaded = false, + refreshFileTree?: () => void | Promise, ) { // 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 @@ -30,6 +31,7 @@ export function useStudioSdkSessions( activeCompPath ?? masterCompPath, fileTree, fileTreeLoaded, + refreshFileTree, ); const editFlowSdkSession = activeCompPath ? sdkHandle.session : null; useEffect(() => {