Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/frontend/pnpm-workspace.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# `packages` is required by pnpm 9 (CI pins version 9 in ci.yml/release.yml —
# a settings-only workspace file errors with "packages field missing or
# empty"); pnpm 10+ would accept the file without it. `.` = this directory is
# the sole package, which matches the existing non-workspace lockfile exactly.
packages:
- '.'

allowBuilds:
esbuild: true
msw: true
42 changes: 41 additions & 1 deletion app/frontend/src/app.test.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand Down
108 changes: 96 additions & 12 deletions app/frontend/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import {
readStoredPanel,
type SurfaceName,
} from "@/lib/right-panel";
import {
readLatchedCodeFolder,
writeLatchedCodeFolder,
} from "@/lib/code-folder-latch";
import {
addSurface,
closeSurface,
Expand Down Expand Up @@ -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<T extends { gitRoot?: string }>(
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
Expand Down Expand Up @@ -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]);
Comment on lines +688 to +697

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipped — the cited rule does not exist. Plan R1 specifies the opposite explicitly: "GIVEN localStorage is unavailable (jsdom without storage, private mode, quota) / WHEN read or write is called / THEN read returns undefined and write is a silent no-op — no throw reaches the caller." Acceptance criterion A-013's phrase "degrade to per-session behavior" is defined by its own parenthetical — "(read undefined, write noop)" — meaning unpersisted, not in-memory-latched. docs/memory/run-kit/ui-patterns.md states it directly: "Absent, empty, and storage-unavailable all read alike as 'not latched'", and the intake scopes the module to a "storage-unavailable noop".

Falling back to the live derived gitRoot when storage is unavailable is the designed degradation — the window behaves exactly as it did before the latch existed. Adding an in-memory fallback would introduce a second source of truth, contradicting the app.tsx invariant that "localStorage stays the source of truth and is read AT RENDER".

// 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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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`
Expand All @@ -719,21 +787,24 @@ 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",
params: { server, window: windowParam },
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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).
Expand All @@ -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`.
Expand Down
108 changes: 107 additions & 1 deletion app/frontend/src/components/code-surface.test.tsx
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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(
<CodeSurface gitRoot="/repo" reachable={true} />,
);
const iframe = getByTitle("Code editor");
expect(iframe.getAttribute("src")).toBe("/code/?folder=%2Frepo");
rerender(<CodeSurface gitRoot="/other" reachable={true} />);
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(
<CodeSurface gitRoot="/repo" reachable={true} />,
);
rerender(<CodeSurface gitRoot="/other" reachable={false} />);
rerender(<CodeSurface gitRoot="/other" reachable={true} />);
// 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=<new>), 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(
<CodeSurface gitRoot="/repo" reachable={true} onFolderNavigated={onFolderNavigated} />,
);
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(
<CodeSurface gitRoot="/repo" reachable={true} onFolderNavigated={onFolderNavigated} />,
);
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(
<CodeSurface gitRoot="/repo" reachable={true} onFolderNavigated={onFolderNavigated} />,
);
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(
<CodeSurface gitRoot="/repo" reachable={true} onFolderNavigated={onFolderNavigated} />,
);
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(
<CodeSurface gitRoot="/repo" reachable={true} onFolderNavigated={onFolderNavigated} />,
);
const iframe = getByTitle("Code editor");
stubFrameSearch(iframe, "?folder=%2Fother");
fireEvent.load(iframe);
rerender(
<CodeSurface gitRoot="/other" reachable={true} onFolderNavigated={onFolderNavigated} />,
);
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.
Expand Down
Loading
Loading