🤖 refactor: auto-cleanup

src/browser/components/CommandPalette/CommandPalette.tsx

@@ -13,7 +13,7 @@ import {
   matchesKeybind,
 } from "@/browser/utils/ui/keybinds";
 import { stopKeyboardPropagation } from "@/browser/utils/events";
-import { resolveSlashCommandExperimentValue } from "@/browser/utils/slashCommands/experimentVisibility";
+import { createSlashCommandExperimentResolver } from "@/browser/utils/slashCommands/experimentVisibility";
 import { getSlashCommandSuggestions } from "@/browser/utils/slashCommands/suggestions";
 import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events";
 import { EXPERIMENT_IDS } from "@/common/constants/experiments";
@@ -290,10 +290,9 @@ export const CommandPalette: React.FC<CommandPaletteProps> = ({ getSlashContext
       const suggestions = getSlashCommandSuggestions(q, {
         agentSkills,
         variant: ctx.workspaceId ? "workspace" : "creation",
-        isExperimentEnabled: (experimentId) =>
-          resolveSlashCommandExperimentValue(experimentId, {
-            workspaceHeartbeats: workspaceHeartbeatsExperimentEnabled,
-          }),
+        isExperimentEnabled: createSlashCommandExperimentResolver({
+          workspaceHeartbeats: workspaceHeartbeatsExperimentEnabled,
+        }),
       });
       const section = "Slash Commands";
       const groups: PaletteGroup[] = [

src/browser/contexts/WorkspaceContext.tsx

@@ -73,6 +73,10 @@ import type { APIClient } from "@/browser/contexts/API";
 import { getErrorMessage } from "@/common/utils/errors";
 import type { WorkspaceCreationScope } from "@/common/utils/subProjects";
 
+// Single source of truth for the guard used when the IPC API client is unavailable, so the
+// message can't drift between the workspace-context operations that surface it.
+const API_NOT_CONNECTED_ERROR = "API not connected";
+
 /**
  * One-time best-effort migration: if the backend doesn't have model preferences yet,
  * persist explicit localStorage values so future port/origin changes keep them.
@@ -1290,7 +1294,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
       trunkBranch: string,
       runtimeConfig?: RuntimeConfig
     ) => {
-      if (!api) throw new Error("API not connected");
+      if (!api) throw new Error(API_NOT_CONNECTED_ERROR);
       console.assert(
         typeof trunkBranch === "string" && trunkBranch.trim().length > 0,
         "Expected trunk branch to be provided when creating a workspace"
@@ -1333,7 +1337,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
       workspaceId: string,
       options?: { force?: boolean }
     ): Promise<{ success: boolean; error?: string }> => {
-      if (!api) return { success: false, error: "API not connected" };
+      if (!api) return { success: false, error: API_NOT_CONNECTED_ERROR };
 
       // Capture state before the async operation.
       // We check currentWorkspaceId (from URL) rather than selectedWorkspace
@@ -1403,7 +1407,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
       workspaceId: string,
       newTitle: string
     ): Promise<{ success: boolean; error?: string }> => {
-      if (!api) return { success: false, error: "API not connected" };
+      if (!api) return { success: false, error: API_NOT_CONNECTED_ERROR };
       try {
         const result = await api.workspace.updateTitle({ workspaceId, title: newTitle });
         if (result.success) {
@@ -1428,7 +1432,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
     async (
       workspaceId: string
     ): Promise<{ success: boolean; error?: string; data?: ArchivePreflightResult }> => {
-      if (!api) return { success: false, error: "API not connected" };
+      if (!api) return { success: false, error: API_NOT_CONNECTED_ERROR };
 
       try {
         const result = await api.workspace.preflightArchive({ workspaceId });
@@ -1448,7 +1452,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
       workspaceId: string,
       options?: { acknowledgedUntrackedPaths?: string[] }
     ): Promise<{ success: boolean; error?: string; data?: ArchiveWorkspaceResult }> => {
-      if (!api) return { success: false, error: "API not connected" };
+      if (!api) return { success: false, error: API_NOT_CONNECTED_ERROR };
 
       try {
         const result = await api.workspace.archive({
@@ -1500,7 +1504,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
 
   const unarchiveWorkspace = useCallback(
     async (workspaceId: string): Promise<{ success: boolean; error?: string }> => {
-      if (!api) return { success: false, error: "API not connected" };
+      if (!api) return { success: false, error: API_NOT_CONNECTED_ERROR };
       try {
         const result = await api.workspace.unarchive({ workspaceId });
         if (result.success) {

src/browser/features/ChatInput/index.tsx

@@ -85,7 +85,7 @@ import {
   getSlashCommandSuggestions,
   type SlashSuggestion,
 } from "@/browser/utils/slashCommands/suggestions";
-import { resolveSlashCommandExperimentValue } from "@/browser/utils/slashCommands/experimentVisibility";
+import { createSlashCommandExperimentResolver } from "@/browser/utils/slashCommands/experimentVisibility";
 import { Tooltip, TooltipTrigger, TooltipContent } from "@/browser/components/Tooltip/Tooltip";
 import { AgentModePicker } from "@/browser/components/AgentModePicker/AgentModePicker";
 import { ContextUsageIndicatorButton } from "@/browser/components/ContextUsageIndicatorButton/ContextUsageIndicatorButton";
@@ -1427,10 +1427,9 @@ const ChatInputInner: React.FC<ChatInputProps> = (props) => {
     const suggestions = getSlashCommandSuggestions(input, {
       agentSkills: agentSkillDescriptors,
       variant,
-      isExperimentEnabled: (experimentId) =>
-        resolveSlashCommandExperimentValue(experimentId, {
-          workspaceHeartbeats: workspaceHeartbeatsExperimentEnabled,
-        }),
+      isExperimentEnabled: createSlashCommandExperimentResolver({
+        workspaceHeartbeats: workspaceHeartbeatsExperimentEnabled,
+      }),
     });
     setCommandSuggestions((prev) => replaceSuggestions(prev, suggestions));
     setShowCommandSuggestions(suggestions.length > 0);
@@ -1440,10 +1439,9 @@ const ChatInputInner: React.FC<ChatInputProps> = (props) => {
   // Show only when suggestions are hidden and the input is exactly "/command " with no args yet.
   const commandGhostHint = getCommandGhostHint(input, showCommandSuggestions, {
     variant,
-    isExperimentEnabled: (experimentId) =>
-      resolveSlashCommandExperimentValue(experimentId, {
-        workspaceHeartbeats: workspaceHeartbeatsExperimentEnabled,
-      }),
+    isExperimentEnabled: createSlashCommandExperimentResolver({
+      workspaceHeartbeats: workspaceHeartbeatsExperimentEnabled,
+    }),
   });
 
   // Load agent skills for suggestions
@@ -2849,13 +2847,13 @@ const ChatInputInner: React.FC<ChatInputProps> = (props) => {
                   onDrop={handleDrop}
                   onEscapeInNormalMode={handleEscapeInNormalMode}
                   suppressKeys={
-                    showAtMentionSuggestions
+                    // @file and $skill menus share the same key set; collapse the
+                    // duplicate FILE_SUGGESTION_KEYS branches into one condition.
+                    showAtMentionSuggestions || showSkillSuggestions
                       ? FILE_SUGGESTION_KEYS
-                      : showSkillSuggestions
-                        ? FILE_SUGGESTION_KEYS
-                        : showCommandSuggestions
-                          ? COMMAND_SUGGESTION_KEYS
-                          : undefined
+                      : showCommandSuggestions
+                        ? COMMAND_SUGGESTION_KEYS
+                        : undefined
                   }
                   placeholder={placeholder}
                   disabled={!editingMessageForUi && (disabled || sendInFlightBlocksInput)}

src/browser/features/ChatInput/placeholderTips.test.ts

@@ -1,12 +1,10 @@
 import { afterEach, describe, expect, test } from "bun:test";
-import { PLACEHOLDER_TIPS, getPlaceholderTip } from "./placeholderTips";
+import { PLACEHOLDER_TIPS, TIP_ROTATION_INTERVAL_MS, getPlaceholderTip } from "./placeholderTips";
 
 interface StorybookGlobal {
   __MUX_STORYBOOK__?: boolean;
 }
 
-const TWENTY_MIN_MS = 20 * 60 * 1000;
-
 describe("PLACEHOLDER_TIPS", () => {
   test("every tip references a slash command", () => {
     for (const tip of PLACEHOLDER_TIPS) {
@@ -43,25 +41,26 @@ describe("getPlaceholderTip", () => {
     // to the same tip. If they don't, switching workspaces / re-rendering
     // inside the same bucket would reshuffle the tip — which is the exact
     // flicker we're trying to prevent.
-    const bucketStart = TWENTY_MIN_MS * 100; // arbitrary aligned anchor
+    const bucketStart = TIP_ROTATION_INTERVAL_MS * 100; // arbitrary aligned anchor
     const tip = getPlaceholderTip(bucketStart);
     expect(getPlaceholderTip(bucketStart + 1)).toBe(tip);
-    expect(getPlaceholderTip(bucketStart + TWENTY_MIN_MS - 1)).toBe(tip);
+    expect(getPlaceholderTip(bucketStart + TIP_ROTATION_INTERVAL_MS - 1)).toBe(tip);
   });
 
   test("advances to the next tip when the bucket boundary crosses", () => {
     // Crossing the boundary must rotate — otherwise the carousel is silently
     // stuck and the discoverability rationale is broken.
-    const bucketStart = TWENTY_MIN_MS * 100;
+    const bucketStart = TIP_ROTATION_INTERVAL_MS * 100;
     const before = getPlaceholderTip(bucketStart);
-    const after = getPlaceholderTip(bucketStart + TWENTY_MIN_MS);
+    const after = getPlaceholderTip(bucketStart + TIP_ROTATION_INTERVAL_MS);
     expect(after).not.toBe(before);
   });
 
   test("wraps with modulo so long-running clocks never lose the placeholder", () => {
     // Far-future timestamps should still resolve to a tip rather than
     // undefined / out-of-bounds.
-    const bigFuture = TWENTY_MIN_MS * PLACEHOLDER_TIPS.length * 5 + TWENTY_MIN_MS * 3;
+    const bigFuture =
+      TIP_ROTATION_INTERVAL_MS * PLACEHOLDER_TIPS.length * 5 + TIP_ROTATION_INTERVAL_MS * 3;
     expect(PLACEHOLDER_TIPS).toContain(getPlaceholderTip(bigFuture));
   });
 
@@ -87,9 +86,9 @@ describe("getPlaceholderTip", () => {
     // Explicit nowMs must still rotate even with the flag set, otherwise
     // unit tests that depend on rotation math would silently no-op when
     // someone forgets to clear the flag.
-    const bucketStart = TWENTY_MIN_MS * 100;
+    const bucketStart = TIP_ROTATION_INTERVAL_MS * 100;
     const before = getPlaceholderTip(bucketStart);
-    const after = getPlaceholderTip(bucketStart + TWENTY_MIN_MS);
+    const after = getPlaceholderTip(bucketStart + TIP_ROTATION_INTERVAL_MS);
     expect(after).not.toBe(before);
   });
 });

src/browser/features/ChatInput/placeholderTips.ts

@@ -20,8 +20,9 @@
  * sure the command you're surfacing isn't gated.
  */
 
-/** Bucket length for tip rotation. */
-const TIP_ROTATION_INTERVAL_MS = 20 * 60 * 1000; // 20 minutes
+/** Bucket length for tip rotation. Exported so tests anchor their bucket math
+ * to the real interval instead of a hardcoded copy that silently drifts. */
+export const TIP_ROTATION_INTERVAL_MS = 20 * 60 * 1000; // 20 minutes
 
 /**
  * Tip index pinned for Storybook/Chromatic snapshots.

src/browser/features/ChatInput/types.ts

@@ -4,7 +4,10 @@ import type { Review } from "@/common/types/review";
 import type { EditingMessageState, PendingUserMessage } from "@/browser/utils/chatEditing";
 import type { SendMessageOptions } from "@/common/orpc/types";
 
-export type GoalInterventionPolicy = NonNullable<SendMessageOptions["goalInterventionPolicy"]>;
+// Re-export so `ChatInput/types` (the existing barrel for ChatInput-local
+// types) stays the single import surface for this feature, while the
+// canonical declaration lives next to `SendMessageOptions` itself.
+export type { GoalInterventionPolicy } from "@/common/orpc/types";
 export type QueueDispatchMode = NonNullable<SendMessageOptions["queueDispatchMode"]>;
 
 export interface ChatInputAPI {

src/browser/features/Messages/Mermaid.tsx

@@ -8,6 +8,11 @@ import { usePersistedState } from "@/browser/hooks/usePersistedState";
 const MIN_HEIGHT = 300;
 const MAX_HEIGHT = 1200;
 
+// Sanitizer lookup tables. Hoisted to module scope so they aren't reallocated on
+// every sanitizeMermaidSvg() call (which can run per render for large SVGs).
+const SANITIZER_URL_ATTRIBUTES = new Set(["href", "xlink:href", "src", "action", "formaction"]);
+const SANITIZER_BLOCKED_URL_SCHEMES = ["javascript:", "vbscript:", "data:text/html"];
+
 // Initialize mermaid
 mermaid.initialize({
   startOnLoad: false,
@@ -162,9 +167,6 @@ export function sanitizeMermaidSvg(svg: string): string | null {
     node.remove();
   });
 
-  const urlAttributes = new Set(["href", "xlink:href", "src", "action", "formaction"]);
-  const blockedSchemes = ["javascript:", "vbscript:", "data:text/html"];
-
   const elementsToScan: Element[] = [svgRoot, ...Array.from(svgRoot.querySelectorAll("*"))];
   elementsToScan.forEach((element) => {
     for (const attribute of Array.from(element.attributes)) {
@@ -177,8 +179,8 @@ export function sanitizeMermaidSvg(svg: string): string | null {
       }
 
       if (
-        urlAttributes.has(attributeName) &&
-        blockedSchemes.some((scheme) => canonicalUrlValue.startsWith(scheme))
+        SANITIZER_URL_ATTRIBUTES.has(attributeName) &&
+        SANITIZER_BLOCKED_URL_SCHEMES.some((scheme) => canonicalUrlValue.startsWith(scheme))
       ) {
         element.removeAttribute(attribute.name);
       }

src/browser/features/RightSidebar/CodeReview/ImmersiveMinimap.tsx

@@ -30,6 +30,18 @@ const readThemeColor = (cssVariableName: string, fallback: string): string => {
   return resolvedColor.length > 0 ? resolvedColor : fallback;
 };
 
+// The minimap reads thumb geometry from a scroll container in three places
+// (initial draw, thumb press, and drag-follow), each needing the same
+// scrollTop/scrollHeight/clientHeight triple. Funnel them through one wrapper
+// so the call sites stay in sync; pure forwarding, no behavior change.
+const getScrollContainerThumbMetrics = (scrollContainer: HTMLElement, trackHeight: number) =>
+  getThumbMetrics(
+    scrollContainer.scrollTop,
+    scrollContainer.scrollHeight,
+    scrollContainer.clientHeight,
+    trackHeight
+  );
+
 export const ImmersiveMinimap: React.FC<ImmersiveMinimapProps> = (props) => {
   // Computed synchronously during render — React Compiler memoizes based on
   // props.content. Avoids the useState+useEffect flash where the component
@@ -105,10 +117,8 @@ export const ImmersiveMinimap: React.FC<ImmersiveMinimapProps> = (props) => {
 
     const scrollContainer = props.scrollContainerRef.current;
     if (scrollContainer) {
-      const { thumbTop, thumbHeight } = getThumbMetrics(
-        scrollContainer.scrollTop,
-        scrollContainer.scrollHeight,
-        scrollContainer.clientHeight,
+      const { thumbTop, thumbHeight } = getScrollContainerThumbMetrics(
+        scrollContainer,
         trackHeight
       );
 
@@ -196,12 +206,7 @@ export const ImmersiveMinimap: React.FC<ImmersiveMinimapProps> = (props) => {
       const pointerY = event.clientY - bounds.top;
 
       if (scrollContainer) {
-        const thumbMetrics = getThumbMetrics(
-          scrollContainer.scrollTop,
-          scrollContainer.scrollHeight,
-          scrollContainer.clientHeight,
-          bounds.height
-        );
+        const thumbMetrics = getScrollContainerThumbMetrics(scrollContainer, bounds.height);
 
         const isPressingThumb =
           thumbMetrics.maxThumbTop > 0 &&
@@ -223,10 +228,8 @@ export const ImmersiveMinimap: React.FC<ImmersiveMinimapProps> = (props) => {
 
             const latestBounds = latestCanvas.getBoundingClientRect();
             const nextPointerY = moveEvent.clientY - latestBounds.top;
-            const nextMetrics = getThumbMetrics(
-              latestScrollContainer.scrollTop,
-              latestScrollContainer.scrollHeight,
-              latestScrollContainer.clientHeight,
+            const nextMetrics = getScrollContainerThumbMetrics(
+              latestScrollContainer,
               latestBounds.height
             );
             const nextThumbTop = clamp(nextPointerY - pointerOffset, 0, nextMetrics.maxThumbTop);

src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx

@@ -1561,6 +1561,22 @@ export const ImmersiveReviewView: React.FC<ImmersiveReviewViewProps> = (props) =
     const contentChanged = previousContentRef.current !== overlayData.content;
     previousContentRef.current = overlayData.content;
 
+    // Clear the file-switch reveal gate on the next frame once the initial scroll
+    // has been issued. Both the no-line-element retry path and the post-scroll path
+    // need the identical "cancel any pending frame, then schedule a one-shot clear
+    // guarded by the captured file path" sequence, so keep it in one place.
+    const scheduleRevealCompletion = (revealFilePath: string) => {
+      if (revealAnimationFrameRef.current !== null) {
+        cancelAnimationFrame(revealAnimationFrameRef.current);
+      }
+      revealAnimationFrameRef.current = window.requestAnimationFrame(() => {
+        setPendingRevealFilePath((pendingFilePath) =>
+          pendingFilePath === revealFilePath ? null : pendingFilePath
+        );
+        revealAnimationFrameRef.current = null;
+      });
+    };
+
     const previousLineElement = highlightedLineElementRef.current;
     if (previousLineElement) {
       previousLineElement.style.outline = "";
@@ -1596,17 +1612,7 @@ export const ImmersiveReviewView: React.FC<ImmersiveReviewViewProps> = (props) =
         return;
       }
 
-      if (revealAnimationFrameRef.current !== null) {
-        cancelAnimationFrame(revealAnimationFrameRef.current);
-      }
-
-      const revealFilePath = activeFilePath;
-      revealAnimationFrameRef.current = window.requestAnimationFrame(() => {
-        setPendingRevealFilePath((pendingFilePath) =>
-          pendingFilePath === revealFilePath ? null : pendingFilePath
-        );
-        revealAnimationFrameRef.current = null;
-      });
+      scheduleRevealCompletion(activeFilePath);
       return;
     }
 
@@ -1627,17 +1633,7 @@ export const ImmersiveReviewView: React.FC<ImmersiveReviewViewProps> = (props) =
       return;
     }
 
-    if (revealAnimationFrameRef.current !== null) {
-      cancelAnimationFrame(revealAnimationFrameRef.current);
-    }
-
-    const revealFilePath = activeFilePath;
-    revealAnimationFrameRef.current = window.requestAnimationFrame(() => {
-      setPendingRevealFilePath((pendingFilePath) =>
-        pendingFilePath === revealFilePath ? null : pendingFilePath
-      );
-      revealAnimationFrameRef.current = null;
-    });
+    scheduleRevealCompletion(activeFilePath);
   }, [
     activeFilePath,
     activeLineIndex,