coder · ibetitsmike · Aug 24, 2026 · Aug 24, 2026 · Aug 24, 2026 · Aug 24, 2026
diff --git a/.claude/docs/FRONTEND_PATTERNS.md b/.claude/docs/FRONTEND_PATTERNS.md
@@ -30,6 +30,9 @@ function actually exercises the interaction. Jest/RTL tests are for pure logic
 - When a component depends on the current time or date, accept it as a prop or
   via context instead of reading `new Date()` or `Date.now()` internally, so
   stories render deterministically without mocking globals.
+- `renderHook` suites for stateful UI hooks are interaction tests, not pure
+  logic. Cover that behavior through the story of the component that uses the
+  hook.
 
 **Incorrect (interaction test in Jest/RTL):**
 
@@ -89,6 +92,8 @@ const config: ChatModel = parseConfig(data);
   of existing ones.
 - Use existing wrapped primitives (Combobox, dialogs, tables) instead of
   hand-assembling the underlying pieces they already wrap.
+- Do not introduce a new React hook when an existing hook, a plain function,
+  or component state can express the logic.
 - Delete dead code and unreachable branches instead of carrying them along.
 - Keep the PR scoped to one change. Move unrelated cleanups, renames, and
   drive-by refactors to separate PRs.
@@ -209,6 +214,9 @@ Decide where logic goes before reaching for `useEffect`:
   is readable on its own. Share the entity fixture, not a pre-wired query
   object.
 - Query keys in mocks follow FE7: import the constant.
+- Never replace browser globals with `Object.defineProperty` in tests or
+  stories. Use `vi.stubGlobal` in unit tests and `spyOn` from
+  `storybook/test` in stories.
 
 ## FE10: Tests assert observable behavior
 

diff --git a/.claude/skills/frontend-review/SKILL.md b/.claude/skills/frontend-review/SKILL.md
@@ -38,14 +38,18 @@ before they see the PR.
   user-visible behavior? Then a changed or added `.stories.tsx` must exist,
   and its `play` function must perform the new interaction (open the menu,
   submit the form), not merely render. Interaction tests added to `.test.tsx`
-  files are a FAIL unless they cover pure logic.
+  files are a FAIL unless they cover pure logic; `renderHook` suites for
+  stateful UI hooks count as interaction tests and belong in the consuming
+  component's story.
 - **FE2 (types)**: Search the diff for `any`, `as unknown as`, non-null
   assertions in any form (`x!.y`, `items[0]!`, `fn()!`, `value! as T`), and
   new `as` casts. Check that API data uses types from `api/typesGenerated.ts`.
 - **FE3 (reuse/scope)**: For each new component, hook, or helper, search
   `site/src/components/` and sibling folders for an existing equivalent.
   Flag near-duplicates, hand-assembled versions of wrapped primitives, dead
-  branches, and unrelated changes bundled into the diff.
+  branches, and unrelated changes bundled into the diff. Flag new React hooks
+  that an existing hook, a plain function, or component state could replace;
+  several new single-use hooks in one diff is a FAIL.
 - **FE4 (comments)**: Read every comment line the diff adds or edits. Flag
   any comment that restates the identifier, assertion, or control flow.
   Verify surviving comments are factually correct.
@@ -68,7 +72,10 @@ before they see the PR.
   reads.
 - **FE9 (fixtures)**: Flag inline entity literals that duplicate or deviate
   from `Mock*` fixtures in `site/src/testHelpers/`, and shared pre-wired
-  query objects instead of per-story inline `{ key, data }` wiring.
+  query objects instead of per-story inline `{ key, data }` wiring. Flag any
+  `Object.defineProperty` replacement of a browser global in tests or
+  stories: unit tests stub with `vi.stubGlobal`, stories mock existing
+  globals with `spyOn` from `storybook/test`.
 - **FE10 (test queries)**: Flag `querySelector`, class-name substring
   matches, geometry assertions, `behavior: "smooth"` dependence, and
   locale-less `toLocaleString()` in changed tests and stories.

diff --git a/site/src/hooks/useIsBelowLgViewport.ts b/site/src/hooks/useIsBelowLgViewport.ts
diff --git a/site/src/hooks/useIsBelowMdViewport.ts b/site/src/hooks/useIsBelowMdViewport.ts
diff --git a/site/src/hooks/useMediaQuery.ts b/site/src/hooks/useMediaQuery.ts
@@ -0,0 +1,22 @@
+import { useCallback, useSyncExternalStore } from "react";
+
+/**
+ * Subscribes to a CSS media query and returns whether it currently
+ * matches, re-rendering on change. Pass a shared query constant from
+ * `utils/mobile.ts` so breakpoints stay aligned with Tailwind
+ * utilities.
+ */
+export const useMediaQuery = (query: string): boolean => {
+	const subscribe = useCallback(
+		(onStoreChange: () => void) => {
+			const mediaQuery = window.matchMedia(query);
+			mediaQuery.addEventListener("change", onStoreChange);
+			return () => mediaQuery.removeEventListener("change", onStoreChange);
+		},
+		[query],
+	);
+	return useSyncExternalStore(
+		subscribe,
+		() => window.matchMedia(query).matches,
+	);
+};
@@ -1934,6 +1934,13 @@ export const NarrowingSuppressesExpandedPanel: Story = {
 		expect(
 			canvas.queryByRole("tab", { name: "Summary" }),
 		).not.toBeInTheDocument();
+
+		// Widening again restores the persisted panel, still expanded.
+		narrowingMedia?.setMatches(belowLgViewportMediaQuery, false);
+		await waitFor(() => {
+			expect(canvas.getByRole("tab", { name: "Summary" })).toBeVisible();
+		});
+		expect(messagesRegion.checkVisibility()).toBe(false);
 	},
 };
 

@@ -18,7 +18,6 @@ import {
 	MockWorkspaceAgent,
 	MockWorkspaceApp,
 } from "#/testHelpers/entities";
-import { setupMatchMedia } from "#/testHelpers/matchMedia";
 import {
 	buildInactiveChatQueueReconciliation,
 	draftInputStorageKeyPrefix,
@@ -32,7 +31,6 @@ import {
 	settlePromotedQueueHead,
 	submitEdit,
 	useConversationEditingState,
-	useRightPanelNarrowSuppression,
 	waitForPendingChatSettingsSyncs,
 } from "./AgentChatPage";
 import type { ChatMessageInputRef } from "./components/AgentChatInput";
@@ -1407,52 +1405,3 @@ describe("isChatAgentBindingUnresolved", () => {
 		);
 	});
 });
-
-describe("useRightPanelNarrowSuppression", () => {
-	const belowLgQuery = "(max-width: 1023px)";
-
-	const setupBelowLg = (initialBelowLg: boolean) => {
-		const media = setupMatchMedia({ [belowLgQuery]: initialBelowLg });
-		return {
-			setBelowLg: (value: boolean) => media.setMatches(belowLgQuery, value),
-		};
-	};
-
-	it("suppresses the panel when mounted below the lg breakpoint", () => {
-		setupBelowLg(true);
-		const { result } = renderHook(() => useRightPanelNarrowSuppression());
-		expect(result.current.suppressed).toBe(true);
-	});
-
-	it("does not suppress the panel when mounted at or above lg", () => {
-		setupBelowLg(false);
-		const { result } = renderHook(() => useRightPanelNarrowSuppression());
-		expect(result.current.suppressed).toBe(false);
-	});
-
-	it("suppresses on narrowing and clears on widening", () => {
-		const media = setupBelowLg(false);
-		const { result } = renderHook(() => useRightPanelNarrowSuppression());
-
-		act(() => media.setBelowLg(true));
-		expect(result.current.suppressed).toBe(true);
-
-		act(() => media.setBelowLg(false));
-		expect(result.current.suppressed).toBe(false);
-	});
-
-	it("stays cleared after an explicit clearSuppression until the next narrowing", () => {
-		const media = setupBelowLg(false);
-		const { result } = renderHook(() => useRightPanelNarrowSuppression());
-
-		act(() => media.setBelowLg(true));
-		expect(result.current.suppressed).toBe(true);
-
-		act(() => result.current.clearSuppression());
-		expect(result.current.suppressed).toBe(false);
-
-		act(() => media.setBelowLg(false));
-		act(() => media.setBelowLg(true));
-		expect(result.current.suppressed).toBe(true);
-	});
-});
@@ -65,12 +65,12 @@ import type { ChatMessagePart } from "#/api/typesGenerated";
 import { useProxy } from "#/contexts/ProxyContext";
 import { useAuthenticated } from "#/hooks/useAuthenticated";
 import { useAIGatewayEnabled } from "#/hooks/useEmbeddedMetadata";
-import { useIsBelowLgViewport } from "#/hooks/useIsBelowLgViewport";
+import { useMediaQuery } from "#/hooks/useMediaQuery";
 import {
 	getDefaultOrganizationName,
 	useDashboard,
 } from "#/modules/dashboard/useDashboard";
-import { isMobileViewport } from "#/utils/mobile";
+import { belowLgViewportMediaQuery, isMobileViewport } from "#/utils/mobile";
 import { pageTitle } from "#/utils/page";
 import { rewriteLocalhostURL } from "#/utils/portForward";
 import { createReconnectingWebSocket } from "#/utils/reconnectingWebSocket";
@@ -134,29 +134,6 @@ import {
 /** localStorage key controlling whether the right panel is visible. */
 export const RIGHT_PANEL_OPEN_KEY = "agents.right-panel-open";
 
-/**
- * Below the `lg` breakpoint, chat and the right panel are mutually
- * exclusive, so a panel left open on a wide window would hide chat as
- * soon as the window narrows. This suppresses the panel while narrow
- * without touching the persisted preference: widening restores the
- * panel, and an explicit user action (clearSuppression) overrides it.
- */
-export function useRightPanelNarrowSuppression(): {
-	suppressed: boolean;
-	clearSuppression: () => void;
-} {
-	const isBelowLg = useIsBelowLgViewport();
-	const [suppressed, setSuppressed] = useState(isBelowLg);
-	const [prevIsBelowLg, setPrevIsBelowLg] = useState(isBelowLg);
-	// Render-time state adjustment on breakpoint crossings; see
-	// https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
-	if (isBelowLg !== prevIsBelowLg) {
-		setPrevIsBelowLg(isBelowLg);
-		setSuppressed(isBelowLg);
-	}
-	return { suppressed, clearSuppression: () => setSuppressed(false) };
-}
-
 const lastModelConfigIDStorageKey = "agents.last-model-config-id";
 
 const AGENT_BINDING_REPAIR_POLL_MS = 30_000;
@@ -886,15 +863,28 @@ const AgentChatPage: FC = () => {
 	const [sidebarPanelPreference, setSidebarPanelPreference] = useState(() => {
 		return localStorage.getItem(RIGHT_PANEL_OPEN_KEY) === "true";
 	});
-	const { suppressed: panelSuppressedOnNarrow, clearSuppression } =
-		useRightPanelNarrowSuppression();
+	// Below the lg breakpoint, chat and the right panel are mutually
+	// exclusive, so a panel left open on a wide window would hide chat
+	// as soon as the window narrows. Suppression hides the panel while
+	// narrow without touching the persisted preference: widening
+	// restores the panel, and an explicit toggle overrides it.
+	const isBelowLg = useMediaQuery(belowLgViewportMediaQuery);
+	const [panelSuppressedOnNarrow, setPanelSuppressedOnNarrow] =
+		useState(isBelowLg);
+	const [prevIsBelowLg, setPrevIsBelowLg] = useState(isBelowLg);
+	// Render-time state adjustment on breakpoint crossings; see
+	// https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
+	if (isBelowLg !== prevIsBelowLg) {
+		setPrevIsBelowLg(isBelowLg);
+		setPanelSuppressedOnNarrow(isBelowLg);
+	}
 	// Canonical panel visibility: the persisted preference gated by the
 	// narrow-viewport suppression. Only this derived value may be
 	// rendered or handed to children; the raw preference stays local.
 	const showSidebarPanel = sidebarPanelPreference && !panelSuppressedOnNarrow;
 
 	const handleSetShowSidebarPanel = (next: boolean) => {
-		clearSuppression();
+		setPanelSuppressedOnNarrow(false);
 		setSidebarPanelPreference(next);
 		localStorage.setItem(RIGHT_PANEL_OPEN_KEY, String(next));
 	};

@@ -35,7 +35,7 @@ import {
 } from "#/components/Tooltip/Tooltip";
 import { useProxy } from "#/contexts/ProxyContext";
 import { useClipboard } from "#/hooks/useClipboard";
-import { useIsBelowMdViewport } from "#/hooks/useIsBelowMdViewport";
+import { useMediaQuery } from "#/hooks/useMediaQuery";
 import {
 	getTerminalHref,
 	getVSCodeHref,
@@ -47,6 +47,7 @@ import {
 	usePortsData,
 } from "#/modules/resources/usePortsData";
 import { cn } from "#/utils/cn";
+import { belowMdViewportMediaQuery } from "#/utils/mobile";
 import { getWorkspaceStatus, StatusIcon } from "./StatusIcon";
 import { MobilePortsPanel, PortsMenuItem } from "./WorkspacePillPorts";
 
@@ -98,7 +99,7 @@ export const WorkspacePill: FC<WorkspacePillProps> = ({
 	// Flyout sub-menus clip on mobile.
 	const [view, setView] = useState<"main" | "ports">("main");
 	const [focusPortsOnMain, setFocusPortsOnMain] = useState(false);
-	const isBelowMd = useIsBelowMdViewport();
+	const isBelowMd = useMediaQuery(belowMdViewportMediaQuery);
 	const showPortsView = view === "ports" && isBelowMd;
 
 	const portsData = usePortsData(

diff --git a/site/src/testHelpers/matchMedia.ts b/site/src/testHelpers/matchMedia.ts
@@ -1,11 +1,16 @@
+import { spyOn } from "storybook/test";
+
 /**
- * Replaces `window.matchMedia` with a controllable stub for tests and
- * stories. Queries listed in `initialMatches` report their configured
- * value; every other query delegates to the real `matchMedia` (or
- * reports `false` where none exists, e.g. jsdom) so unrelated
+ * Replaces `window.matchMedia` with a controllable stub for stories.
+ * Queries listed in `initialMatches` report their configured value;
+ * every other query delegates to the real `matchMedia` so unrelated
  * responsive components keep behaving truthfully. `setMatches` updates
  * a query and notifies its registered change listeners; `restore` puts
  * the original `window.matchMedia` back.
+ *
+ * Story-only: stories run in a real browser, so a real `matchMedia` to
+ * delegate to always exists. jsdom has no `matchMedia`, so unit tests
+ * must install their own stub with `vi.stubGlobal` instead.
  */
 export const setupMatchMedia = (initialMatches: Record<string, boolean>) => {
 	const matches = { ...initialMatches };
@@ -18,15 +23,11 @@ export const setupMatchMedia = (initialMatches: Record<string, boolean>) => {
 		}
 		return set;
 	};
-	const original = window.matchMedia;
-	const originalFn =
-		typeof original === "function" ? original.bind(window) : undefined;
-	Object.defineProperty(window, "matchMedia", {
-		configurable: true,
-		writable: true,
-		value: (query: string): MediaQueryList => {
-			if (!(query in matches) && originalFn) {
-				return originalFn(query);
+	const original = window.matchMedia.bind(window);
+	const spy = spyOn(window, "matchMedia").mockImplementation(
+		(query: string): MediaQueryList => {
+			if (!(query in matches)) {
+				return original(query);
 			}
 			return {
 				get matches() {
@@ -51,7 +52,7 @@ export const setupMatchMedia = (initialMatches: Record<string, boolean>) => {
 				removeListener: () => {},
 			} satisfies MediaQueryList;
 		},
-	});
+	);
 	return {
 		setMatches: (query: string, value: boolean) => {
 			matches[query] = value;
@@ -64,12 +65,6 @@ export const setupMatchMedia = (initialMatches: Record<string, boolean>) => {
 				}
 			}
 		},
-		restore: () => {
-			Object.defineProperty(window, "matchMedia", {
-				configurable: true,
-				writable: true,
-				value: original,
-			});
-		},
+		restore: () => spy.mockRestore(),
 	};
 };