From 4c65f9cfe4a1b7e362b451b401140e1c7b02c5f1 Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Thu, 13 Aug 2026 16:22:42 +0000 Subject: [PATCH 1/4] feat: Latch code-surface folder at first open The code lens re-derives its folder from the active pane's cwd on every SSE tick, so switching panes in the terminal makes the embedded editor vanish or lose in-flight state. Latch the folder once at first open (localStorage, per-window) so only the editor's own File > Open Folder navigation moves it afterward. --- app/frontend/src/app.test.tsx | 42 +++- app/frontend/src/app.tsx | 108 ++++++++-- .../src/components/code-surface.test.tsx | 108 +++++++++- app/frontend/src/components/code-surface.tsx | 85 +++++++- .../src/components/surface-layout.test.tsx | 37 ++++ .../src/components/surface-layout.tsx | 15 +- .../src/lib/code-folder-latch.test.ts | 60 ++++++ app/frontend/src/lib/code-folder-latch.ts | 69 +++++++ .../tests/e2e/code-folder-latch.spec.md | 98 +++++++++ .../tests/e2e/code-folder-latch.spec.ts | 192 ++++++++++++++++++ docs/memory/run-kit/index.md | 2 +- docs/memory/run-kit/ui-patterns.md | 57 ++++-- docs/specs/right-panel.md | 30 ++- docs/specs/window-views.md | 2 +- .../.history.jsonl | 10 + .../.status.yaml | 55 +++++ .../intake.md | 145 +++++++++++++ .../plan.md | 173 ++++++++++++++++ 18 files changed, 1245 insertions(+), 43 deletions(-) create mode 100644 app/frontend/src/lib/code-folder-latch.test.ts create mode 100644 app/frontend/src/lib/code-folder-latch.ts create mode 100644 app/frontend/tests/e2e/code-folder-latch.spec.md create mode 100644 app/frontend/tests/e2e/code-folder-latch.spec.ts create mode 100644 fab/changes/260813-if5d-latch-code-surface-folder/.history.jsonl create mode 100644 fab/changes/260813-if5d-latch-code-surface-folder/.status.yaml create mode 100644 fab/changes/260813-if5d-latch-code-surface-folder/intake.md create mode 100644 fab/changes/260813-if5d-latch-code-surface-folder/plan.md diff --git a/app/frontend/src/app.test.tsx b/app/frontend/src/app.test.tsx index ee3335087..3aa20f68e 100644 --- a/app/frontend/src/app.test.tsx +++ b/app/frontend/src/app.test.tsx @@ -1,7 +1,8 @@ import { describe, it, expect, vi, afterEach } from "vitest"; import { render, screen, fireEvent, cleanup } from "@testing-library/react"; import { CommandPalette, type PaletteAction } from "@/components/command-palette"; -import { resolveServerView } from "@/app"; +import { resolveServerView, withLatchedCodeFolder } from "@/app"; +import { availableViews, hasCode } from "@/lib/window-view"; import type { ServerInfo } from "@/api/client"; // `@/app` transitively imports terminal-client → @xterm/addon-unicode-graphemes, @@ -544,6 +545,45 @@ describe("resolveServerView — three-way route guard", () => { }); }); +/** + * `withLatchedCodeFolder` is the one substitution point behind the code-folder + * latch (260813-if5d): every code-availability and code-render consumer in + * AppShell reads `gitRoot` from the window it returns, so the latch — not the + * live per-tick derivation — decides what the editor shows and whether the lens + * is offered at all. + */ +describe("withLatchedCodeFolder — the latch substitution seam", () => { + it("substitutes the latched folder for the live derivation", () => { + const win = { gitRoot: "/home/user/derived", rkUrl: "http://localhost:8080" }; + expect(withLatchedCodeFolder(win, "/home/user/latched")).toEqual({ + gitRoot: "/home/user/latched", + rkUrl: "http://localhost:8080", + }); + }); + + it("keeps the code lens available when the live derivation went empty (the pane-switch case)", () => { + // The intake's screenshot scenario: the active pane leaves the repo, so the + // next SSE tick derives "". The latch is what stops the strobe. + const win = { gitRoot: "" }; + expect(hasCode(win)).toBe(false); + expect(hasCode(withLatchedCodeFolder(win, "/home/user/latched"))).toBe(true); + expect(availableViews(withLatchedCodeFolder(win, "/home/user/latched"))).toEqual([ + "code", + "tty", + ]); + }); + + it("passes an unlatched window through by identity (no render churn pre-latch)", () => { + const win = { gitRoot: "/home/user/derived" }; + expect(withLatchedCodeFolder(win, undefined)).toBe(win); + }); + + it("tolerates a null window (pre-snapshot frames)", () => { + expect(withLatchedCodeFolder(null, "/home/user/latched")).toBeNull(); + expect(withLatchedCodeFolder(null, undefined)).toBeNull(); + }); +}); + /** * Tests for the ungated `View:` palette entries (R4) — `toggle-fixed-width` * from `viewActions` in `app.tsx` (AppShell's route list) plus the global diff --git a/app/frontend/src/app.tsx b/app/frontend/src/app.tsx index 4929ab140..07dc3b8e9 100644 --- a/app/frontend/src/app.tsx +++ b/app/frontend/src/app.tsx @@ -11,6 +11,10 @@ import { readStoredPanel, type SurfaceName, } from "@/lib/right-panel"; +import { + readLatchedCodeFolder, + writeLatchedCodeFolder, +} from "@/lib/code-folder-latch"; import { addSurface, closeSurface, @@ -598,6 +602,23 @@ export function resolveServerView( return "view"; } +/** + * The window as the CODE surface sees it (260813-if5d): its `gitRoot` replaced + * by the window's latched code folder when one exists, the live derivation + * otherwise. One substitution point is what makes `hasCode`, `availableTiles`, + * `degradeLayout`, the tile render guard, `tileMeta`, and `codeServerSrc` all + * follow the latch while the pure lib modules stay DOM-free and unchanged: the + * latch is read where the storage identity (server, window id) lives and fed in + * as an ordinary window record. Unlatched windows pass through by identity, so + * the substitution adds no render churn before the first code open. + */ +export function withLatchedCodeFolder( + win: T | null, + latch: string | undefined, +): T | null { + return win && latch ? { ...win, gitRoot: latch } : win; +} + /** * How long a pending window switch may stay unconfirmed before the failure * bounce-back fires (260715-38kg). If neither an explicit `selectWindow` POST @@ -652,6 +673,39 @@ function AppShell() { // snapshot (not the URL). Undefined until the snapshot resolves the window. const sessionName = currentSession?.name; + // Code-folder latch (260813-if5d; spec right-panel.md § The code lens): the + // code surface's folder is per-window LATCHED state, not the live SSE + // derivation — `gitRoot` follows the active pane's cwd, so a pane switch or a + // `cd` would otherwise retarget (or unmount) the embedded editor and lose its + // in-flight state. localStorage stays the source of truth and is read AT + // RENDER, keyed per (server, window id) — the `storedLayout` read below does + // the same. Deliberately not mirrored into state via an effect: an effect + // lands a frame late, so a window switch would render the PREVIOUS window's + // latch once, and the code iframe that mounts in that frame fixes its `src` + // for its whole mount generation — the stale folder would stick. The epoch + // bump is what re-reads after a write; `latchCodeFolder` is the only writer + // (the seed effect below at first open, then the editor's own navigation). + const [latchEpoch, setLatchEpoch] = useState(0); + const latchedCodeFolder = useMemo( + () => (windowParam ? readLatchedCodeFolder(server, windowParam) : undefined), + [server, windowParam, latchEpoch], + ); + const latchCodeFolder = useCallback((folder: string) => { + if (!windowParam || folder.length === 0) return; + writeLatchedCodeFolder(server, windowParam, folder); + setLatchEpoch((n) => n + 1); + }, [server, windowParam]); + // The live backend derivation (active pane's cwd walked to its repo root). Its + // ONLY remaining job is seeding the latch — nothing renders from it. + const derivedGitRoot = currentWindow?.gitRoot ?? ""; + // The window every code-availability/render consumer below sees (§ + // withLatchedCodeFolder): latched folder when latched, live derivation as the + // seed otherwise. + const effectiveWindow = useMemo( + () => withLatchedCodeFolder(currentWindow, latchedCodeFolder), + [currentWindow, latchedCodeFolder], + ); + // Surface-layout state (260812-ab5v-surface-layout-core; spec // surface-layout.md L1–L3). The terminal route's center is a LAYOUT of 1–3 // surface tiles; the retired `?view=`/`?panel=` params feed a permanent @@ -673,8 +727,8 @@ function AppShell() { // signal yet (treated as not-running until the first event lands). const codeServer = useCodeServer(); const currentViews = useMemo( - () => availableViews(currentWindow), - [currentWindow], + () => availableViews(effectiveWindow), + [effectiveWindow], ); // The URL's EFFECTIVE layout candidate: a carried `?layout=` wins; absent @@ -683,8 +737,8 @@ function AppShell() { const searchLayout = search.layout ?? translateLegacyParams(searchView, searchPanel); const storedLayout = windowParam ? readStoredLayout(server, windowParam) : undefined; const layout = useMemo( - () => resolveLayout(searchLayout, storedLayout, currentWindow), - [searchLayout, storedLayout, currentWindow], + () => resolveLayout(searchLayout, storedLayout, effectiveWindow), + [searchLayout, storedLayout, effectiveWindow], ); const serializedLayout = serializeLayout(layout); // The lens model's consumers (view-cycle chord, palette `View:` actions) @@ -701,6 +755,20 @@ function AppShell() { if (windowParam) seedLayoutFromLegacy(server, windowParam); }, [server, windowParam]); + // Seed rule (if5d R2): the first time the code surface actually renders for a + // window, the LIVE derivation latches — and derivation never moves the editor + // again (not on a pane switch, not on tile close/reopen, not on reload). Keyed + // on the resolved layout's order, the choke point every entry path (view + // switcher, rail toggle, `?view=code`/`?layout=` deep link, mobile sheet) + // resolves through — never on availability alone: a code lens that is merely + // OFFERED seeds nothing. An empty derivation seeds nothing either, so a window + // that was never inside a repo behaves exactly as it did before the latch. + const codeTileOpen = layout.order.includes("code"); + useEffect(() => { + if (latchedCodeFolder || !codeTileOpen) return; + latchCodeFolder(derivedGitRoot); + }, [latchedCodeFolder, codeTileOpen, derivedGitRoot, latchCodeFolder]); + // Mirror the APPLIED layout into the URL via replaceState (L2 — never // pushState for layout changes), so the address bar is at all times a valid // deep link to what is on screen. The window's DEFAULT layout (`hintLayout` @@ -719,13 +787,16 @@ function AppShell() { // post-seed so a just-migrated legacy window mirrors its SEEDED layout, not // the pre-seed fallback. localStorage is deliberately NOT written here — // arrival via a carried `?layout=` is not a user mutation (L3). + // Resolves against the LATCHED window (if5d) for the same reason the render + // does: keying the mirror off the live derivation would degrade a latched code + // tile away and rewrite the URL the moment the active pane left the repo. useEffect(() => { - if (!windowParam || !currentWindow) return; + if (!windowParam || !effectiveWindow) return; const target = serializeLayout( - resolveLayout(searchLayout, readStoredLayout(server, windowParam), currentWindow), + resolveLayout(searchLayout, readStoredLayout(server, windowParam), effectiveWindow), ); const desired = - target === serializeLayout(hintLayout(currentWindow)) ? undefined : target; + target === serializeLayout(hintLayout(effectiveWindow)) ? undefined : target; if (search.layout === desired) return; navigate({ to: "/$server/$window", @@ -733,7 +804,7 @@ function AppShell() { search: desired ? { layout: desired } : {}, replace: true, }); - }, [server, windowParam, currentWindow, search.layout, searchLayout, navigate]); + }, [server, windowParam, effectiveWindow, search.layout, searchLayout, navigate]); // The ONE mutation path (R3 write discipline — user-initiated mutations // only): persist per-window in localStorage AND mirror the URL via @@ -797,8 +868,8 @@ function AppShell() { // The surfaces the current window can tile (`tty` first — R8's shared // registry), consumed by the rail and the palette gating. const panelSurfaces = useMemo( - () => availableSurfaces(currentWindow), - [currentWindow], + () => availableSurfaces(effectiveWindow), + [effectiveWindow], ); // ⏶ Zoom palette seam (T012/R11): the zoom itself is SurfaceLayout-internal @@ -1735,7 +1806,14 @@ function AppShell() { readStoredPanel(server, fw.window.windowId), ), undefined, - fw.window, + // Latched (if5d) like the current window's own resolution: a + // window whose code folder is latched still resolves code-led + // after its active pane leaves the repo, so the classification + // matches what the target will actually render. + withLatchedCodeFolder( + fw.window, + readLatchedCodeFolder(server, fw.window.windowId), + ), ).order[0] !== "tty", ) .map((fw) => fw.window.windowId), @@ -3526,7 +3604,10 @@ function AppShell() { server={server} windowId={windowParam} sessionName={sessionName ?? ""} - window={currentWindow} + // The LATCHED window (if5d): the code tile's render guard, its + // header basename, and CodeSurface's `src` all read `gitRoot` from + // here, so none of them can follow the terminal. + window={effectiveWindow} isMobile={isMobile} // T014: on mobile the sheet tabs pick which slot renders // (transient — the layout itself is untouched). @@ -3550,6 +3631,9 @@ function AppShell() { busy: currentWindow?.agentState === "active", }} codeReachable={codeServer?.reachable ?? false} + // Follow rule (if5d R3): after the seed, the editor's own + // navigation is the ONLY writer of the latch. + onCodeFolderNavigated={latchCodeFolder} shouldReclaimChord={reclaimChord} // The web tile's `>_` affordance keeps the legacy "switch to // terminal" behavior: collapse to `single:tty`. diff --git a/app/frontend/src/components/code-surface.test.tsx b/app/frontend/src/components/code-surface.test.tsx index 8e65983be..3eaaf7b48 100644 --- a/app/frontend/src/components/code-surface.test.tsx +++ b/app/frontend/src/components/code-surface.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, afterEach, vi } from "vitest"; -import { render, cleanup } from "@testing-library/react"; +import { render, cleanup, fireEvent } from "@testing-library/react"; import { CodeSurface, codeServerSrc } from "./code-surface"; afterEach(cleanup); @@ -42,6 +42,112 @@ describe("CodeSurface", () => { expect(queryByTitle("Code editor")).toBeNull(); }); + // Latched folder (260813-if5d R3): the `src` is per iframe MOUNT GENERATION. + // A folder change on a LIVE frame must not touch the attribute — re-setting + // `src` re-navigates the frame and takes the editor state with it (P3). + it("never changes a mounted iframe's src when the folder prop changes", () => { + const { rerender, getByTitle } = render( + , + ); + const iframe = getByTitle("Code editor"); + expect(iframe.getAttribute("src")).toBe("/code/?folder=%2Frepo"); + rerender(); + expect(getByTitle("Code editor").getAttribute("src")).toBe("/code/?folder=%2Frepo"); + }); + + it("picks up the current folder when the iframe genuinely remounts (reachability flip)", () => { + const { rerender, getByTitle } = render( + , + ); + rerender(); + rerender(); + // A fresh workbench boots at where the editor last was, not the seed. + expect(getByTitle("Code editor").getAttribute("src")).toBe("/code/?folder=%2Fother"); + }); + + // Follow rule (if5d R3): the load seam reports where the EDITOR navigated + // itself (File > Open Folder → /code/?folder=), which the parent latches. + describe("onFolderNavigated (follow-the-editor seam)", () => { + /** Stub the frame's location — jsdom never navigates an iframe, so the + * post-navigation state is injected. `configurable` lets each test replace + * it (contentWindow is a prototype getter). */ + const stubFrameSearch = (el: HTMLElement, search: string) => { + Object.defineProperty(el, "contentWindow", { + configurable: true, + value: { location: { search } }, + }); + }; + + it("reports a different folder from the frame's ?folder= on load (decoded)", () => { + const onFolderNavigated = vi.fn(); + const { getByTitle } = render( + , + ); + const iframe = getByTitle("Code editor"); + stubFrameSearch(iframe, "?folder=%2Fhome%2Fuser%2Fother"); + fireEvent.load(iframe); + expect(onFolderNavigated).toHaveBeenCalledWith("/home/user/other"); + }); + + it("stays silent when the frame is already at the current folder", () => { + const onFolderNavigated = vi.fn(); + const { getByTitle } = render( + , + ); + const iframe = getByTitle("Code editor"); + stubFrameSearch(iframe, "?folder=%2Frepo"); + fireEvent.load(iframe); + expect(onFolderNavigated).not.toHaveBeenCalled(); + }); + + it("stays silent when the frame carries no folder param", () => { + const onFolderNavigated = vi.fn(); + const { getByTitle } = render( + , + ); + const iframe = getByTitle("Code editor"); + stubFrameSearch(iframe, "?other=1"); + fireEvent.load(iframe); + expect(onFolderNavigated).not.toHaveBeenCalled(); + }); + + it("skips a cross-origin frame silently (no throw, no report)", () => { + const onFolderNavigated = vi.fn(); + const { getByTitle } = render( + , + ); + const iframe = getByTitle("Code editor"); + Object.defineProperty(iframe, "contentWindow", { + configurable: true, + get() { + throw new Error("SecurityError: cross-origin"); + }, + }); + expect(() => fireEvent.load(iframe)).not.toThrow(); + expect(onFolderNavigated).not.toHaveBeenCalled(); + }); + + it("does not re-navigate the mounted frame after reporting a new folder", () => { + // The parent latches the reported folder and re-renders with it; the + // attribute must stay put (P3 — that is the whole hazard this guards). + const onFolderNavigated = vi.fn(); + const { rerender, getByTitle } = render( + , + ); + const iframe = getByTitle("Code editor"); + stubFrameSearch(iframe, "?folder=%2Fother"); + fireEvent.load(iframe); + rerender( + , + ); + expect(getByTitle("Code editor").getAttribute("src")).toBe("/code/?folder=%2Frepo"); + // …and the frame's own location is now the baseline: no repeat report. + onFolderNavigated.mockClear(); + fireEvent.load(getByTitle("Code editor")); + expect(onFolderNavigated).not.toHaveBeenCalled(); + }); + }); + // Chord-reclaim effect coverage (review rework): the listener must attach // when the iframe mounts AND re-attach after a reachability flip remounts // it — the []-deps version silently lost reclaim on false→true. diff --git a/app/frontend/src/components/code-surface.tsx b/app/frontend/src/components/code-surface.tsx index 300c9e7f2..74c33456f 100644 --- a/app/frontend/src/components/code-surface.tsx +++ b/app/frontend/src/components/code-surface.tsx @@ -32,6 +32,16 @@ import { useEffect, useRef } from "react"; * import graph, and only registry chords are reclaimed — the embedded app's * own Ctrl/⌘ bindings keep working. Failure mode is benign: a cross-origin * or pre-load frame simply skips the attach (click-out remains the escape). + * - **Latched folder (260813-if5d)**: `gitRoot` arrives already LATCHED (app.tsx + * substitutes it) — this component's contribution is the two halves the latch + * needs at the iframe itself. (a) The `src` is computed once per MOUNT + * GENERATION, never reactively from the prop: re-setting `src` on a live frame + * re-navigates it even to the URL it is already at, which would destroy the + * editor state the latch exists to protect (spec P3 — hide, never unmount). + * (b) The same-origin `load` seam reports where the EDITOR navigated itself + * (File > Open Folder → a full workbench navigation to `/code/?folder=…`) via + * `onFolderNavigated`, so the latch follows the editor. Derivation seeds the + * latch exactly once; thereafter only the editor moves it, never the terminal. */ /** @@ -46,7 +56,9 @@ export function codeServerSrc(gitRoot: string): string { } interface CodeSurfaceProps { - /** The window's derived git toplevel (absolute path). */ + /** The folder the editor opens (absolute path) — the window's LATCHED code + * folder, seeded once from the backend derivation (260813-if5d). Read at + * iframe MOUNT only; a later change never re-navigates a live frame. */ gitRoot: string; /** The host's TTL-cached code-server reachability probe result. */ reachable: boolean; @@ -59,14 +71,48 @@ interface CodeSurfaceProps { * click-to-focus case; keydowns never reach the parent document). Absent * ⇒ no reporting. */ onInteract?: () => void; + /** Follow-the-editor seam (260813-if5d R3): fired with the folder the EDITOR + * navigated itself to (File > Open Folder), read from the same-origin frame's + * `?folder=` on each `load`. Only fired for a present, non-empty folder that + * differs from `gitRoot` — the parent writes it to the window's latch. Absent + * ⇒ no reporting. */ + onFolderNavigated?: (folder: string) => void; } -export function CodeSurface({ gitRoot, reachable, shouldReclaimChord, onInteract }: CodeSurfaceProps) { +export function CodeSurface({ + gitRoot, + reachable, + shouldReclaimChord, + onInteract, + onFolderNavigated, +}: CodeSurfaceProps) { const iframeRef = useRef(null); const reclaimRef = useRef(shouldReclaimChord); reclaimRef.current = shouldReclaimChord; const interactRef = useRef(onInteract); interactRef.current = onInteract; + const folderNavigatedRef = useRef(onFolderNavigated); + folderNavigatedRef.current = onFolderNavigated; + // The comparison baseline for the load-event report below, read through a ref + // because the listener outlives the render that installed it. It tracks the + // latch, which after seeding tracks the editor — so it is exactly "the folder + // we believe the editor is in". + const gitRootRef = useRef(gitRoot); + gitRootRef.current = gitRoot; + + // P3: one `src` per iframe MOUNT GENERATION. The iframe mounts only while + // `reachable`, so recomputing exactly when that gate flips means a + // reachability false→true flip or a window-switch remount boots at the CURRENT + // latched folder (fresh workbench, right folder) while a mounted frame is + // never parent-navigated — a `src` React re-renders IS a navigation, even to + // the URL the frame already sits at. Held in a ref, not `useMemo`: a memo + // cache is a performance hint React may drop, and dropping this one would + // reload the editor out from under the user. + const srcRef = useRef({ mountGen: reachable, src: codeServerSrc(gitRoot) }); + if (srcRef.current.mountGen !== reachable) { + srcRef.current = { mountGen: reachable, src: codeServerSrc(gitRoot) }; + } + const src = srcRef.current.src; // Chord-reclaim spike: attach a capture-phase keydown listener to the // iframe's same-origin contentDocument after every load (each navigation @@ -82,7 +128,12 @@ export function CodeSurface({ gitRoot, reachable, shouldReclaimChord, onInteract // (260812-wfic): any in-editor interaction reports tile focus. useEffect(() => { const iframe = iframeRef.current; - if (!iframe || (!reclaimRef.current && !interactRef.current)) return; + if ( + !iframe || + (!reclaimRef.current && !interactRef.current && !folderNavigatedRef.current) + ) { + return; + } let attachedDoc: Document | null = null; const onKey = (e: KeyboardEvent) => { interactRef.current?.(); @@ -119,10 +170,32 @@ export function CodeSurface({ gitRoot, reachable, shouldReclaimChord, onInteract /* noop — spike stays silent */ } }; + // Follow rule (if5d R3): a workbench navigation replaces the frame's + // document, so every load is a chance the EDITOR moved itself to another + // folder (File > Open Folder). Same try/catch posture as the attach above — + // a cross-origin or pre-load frame silently reports nothing. + const reportFolder = () => { + try { + const search = iframe.contentWindow?.location.search; + if (!search) return; + // `URLSearchParams` decodes, so this compares decoded paths against the + // decoded prop — `encodeURIComponent` round-trips make raw-string + // comparison flaky. + const folder = new URLSearchParams(search).get("folder"); + if (!folder || folder === gitRootRef.current) return; + folderNavigatedRef.current?.(folder); + } catch { + /* noop — cross-origin or pre-load frame */ + } + }; + const onLoad = () => { + attach(); + reportFolder(); + }; attach(); - iframe.addEventListener("load", attach); + iframe.addEventListener("load", onLoad); return () => { - iframe.removeEventListener("load", attach); + iframe.removeEventListener("load", onLoad); try { attachedDoc?.removeEventListener("keydown", onKey, true); attachedDoc?.removeEventListener("pointerdown", onPointer, true); @@ -146,7 +219,7 @@ export function CodeSurface({ gitRoot, reachable, shouldReclaimChord, onInteract return (