🤖 refactor: auto-cleanup

src/browser/contexts/ThemeContext.tsx

@@ -10,6 +10,7 @@ import React, {
 } from "react";
 import { readPersistedString, usePersistedState } from "@/browser/hooks/usePersistedState";
 import { UI_THEME_KEY } from "@/common/constants/storage";
+import { isLightThemeMode } from "@/browser/utils/highlighting/shiki-shared";
 
 export type ThemeMode = "light" | "dark" | "flexoki-light" | "flexoki-dark";
 export type ThemePreference = ThemeMode | "auto";
@@ -74,7 +75,8 @@ const FAVICON_BY_SCHEME: Record<"light" | "dark", string> = {
 
 /** Map theme mode to CSS color-scheme value */
 function getColorScheme(theme: ThemeMode): "light" | "dark" {
-  return theme === "light" || theme === "flexoki-light" ? "light" : "dark";
+  // Reuse the shared `-light` suffix convention so we have one source of truth for the light/dark mapping.
+  return isLightThemeMode(theme) ? "light" : "dark";
 }
 
 function applyThemeFavicon(theme: ThemeMode) {

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

@@ -1259,7 +1259,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
   filteredHunksRef.current = filteredHunks;
 
   // Ensure selectedHunkId is valid after filtering/sorting:
-  // - If no selection or selection not in filtered list, select first visible hunk
+  // - If no selection or selection not in the validity list, select first visible hunk
   // - This runs after sorting, so we always select the top-most hunk in current order
   //
   // Immersive review can intentionally navigate to a hunk that is hidden by
@@ -1272,15 +1272,11 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
   useEffect(() => {
     if (filteredHunks.length === 0) return;
 
-    if (isImmersive) {
-      const selectionExists = selectedHunkId && hunks.some((h) => h.id === selectedHunkId);
-      if (!selectionExists) {
-        setSelectedHunkId(filteredHunks[0].id);
-      }
-      return;
-    }
-
-    const selectionValid = selectedHunkId && filteredHunks.some((h) => h.id === selectedHunkId);
+    // Picking the validity list up front keeps the immersive and non-immersive
+    // behavior in lockstep — the only difference is which list we accept the
+    // current selection against.
+    const validityList = isImmersive ? hunks : filteredHunks;
+    const selectionValid = selectedHunkId && validityList.some((h) => h.id === selectedHunkId);
     if (!selectionValid) {
       setSelectedHunkId(filteredHunks[0].id);
     }

src/browser/features/Settings/Sections/ProvidersSection.tsx

@@ -78,14 +78,15 @@ import type {
   AddCustomOpenAICompatibleProviderInput,
   ProviderConfigInfo,
 } from "@/common/orpc/types";
+import type { ServiceTier } from "@/common/config/schemas/providersConfig";
 
 type MuxGatewayLoginStatus = "idle" | "starting" | "waiting" | "success" | "error";
 type CodexOauthFlowStatus = "idle" | "starting" | "waiting" | "error";
 type CopilotLoginStatus = "idle" | "starting" | "waiting" | "success" | "error";
 
 const OPENAI_SERVICE_TIER_UNSET = "unset";
 
-type OpenAIServiceTier = "auto" | "default" | "flex" | "priority";
+type OpenAIServiceTier = ServiceTier;
 type OpenAIServiceTierSelectValue = typeof OPENAI_SERVICE_TIER_UNSET | OpenAIServiceTier;
 
 function isOpenAIServiceTier(value: string): value is OpenAIServiceTier {

src/browser/features/Tools/FileEditToolCall.tsx

@@ -168,7 +168,9 @@ export const FileEditToolCall: React.FC<FileEditToolCallProps> = ({
   const diff = result && result.success ? (uiOnlyDiff ?? result.diff) : undefined;
   const filePath = extractToolFilePath(args);
   const largeDiffPreview = diff ? buildLargeDiffPreview(diff) : null;
-  const shouldShowLargeDiffPreview = Boolean(largeDiffPreview && !showRaw && !showFullDiff);
+  // Single nullable handle for the active preview so JSX truthiness checks narrow the type
+  // directly (no separate boolean + repeated `&& largeDiffPreview` guards).
+  const activeDiffPreview = largeDiffPreview && !showRaw && !showFullDiff ? largeDiffPreview : null;
 
   // Copy to clipboard with feedback
   const { copied, copyToClipboard } = useCopyToClipboard();
@@ -242,12 +244,12 @@ export const FileEditToolCall: React.FC<FileEditToolCallProps> = ({
 
               {result.success && diff && (
                 <>
-                  {shouldShowLargeDiffPreview && largeDiffPreview && (
+                  {activeDiffPreview && (
                     <DetailSection>
                       <div className="text-muted text-[11px]">
                         Large diff preview: showing{" "}
-                        {largeDiffPreview.displayedLines.toLocaleString()} of{" "}
-                        {largeDiffPreview.totalLines.toLocaleString()} lines. Full patch is still
+                        {activeDiffPreview.displayedLines.toLocaleString()} of{" "}
+                        {activeDiffPreview.totalLines.toLocaleString()} lines. Full patch is still
                         available from the menu.
                       </div>
                       <button
@@ -261,8 +263,8 @@ export const FileEditToolCall: React.FC<FileEditToolCallProps> = ({
                   )}
                   {showRaw
                     ? renderRawDiff(diff)
-                    : shouldShowLargeDiffPreview && largeDiffPreview
-                      ? renderRawDiff(largeDiffPreview.previewDiff)
+                    : activeDiffPreview
+                      ? renderRawDiff(activeDiffPreview.previewDiff)
                       : renderDiff(diff, filePath, onReviewNote)}
                 </>
               )}

src/browser/hooks/useAutoResizeTextarea.ts

@@ -25,6 +25,16 @@ function preservesPreviousValueAsInsertion(previousValue: string, nextValue: str
   return sharedPrefixLength + sharedSuffixLength === previousValue.length;
 }
 
+// Read the textarea's current inline `height` style as a finite px number, or null when the
+// style is empty / non-px / non-finite. Centralizing the parseFloat + isFinite pair keeps the
+// canOnlyGrow first-render fallback and the post-resize verification (which guards against a
+// stale cache after `auto` writes or external clears) from drifting on what counts as a
+// usable inline height.
+function readInlineHeightPx(el: HTMLTextAreaElement): number | null {
+  const value = Number.parseFloat(el.style.height);
+  return Number.isFinite(value) ? value : null;
+}
+
 /**
  * Auto-resize a textarea to fit its content.
  * Uses useLayoutEffect to measure and set height synchronously before paint.
@@ -57,11 +67,9 @@ export function useAutoResizeTextarea(
       // (including all chat rows) through layout even when the composer height is
       // unchanged. For pure insertions we only need to grow if scrollHeight exceeds
       // the currently applied height; shrinking paths still use the full reset below.
-      const appliedHeight = appliedHeightRef.current ?? Number.parseFloat(el.style.height);
+      const appliedHeight = appliedHeightRef.current ?? readInlineHeightPx(el);
       const scrollHeight = Math.min(el.scrollHeight, max);
-      nextHeight = Number.isFinite(appliedHeight)
-        ? Math.max(appliedHeight, scrollHeight)
-        : scrollHeight;
+      nextHeight = appliedHeight !== null ? Math.max(appliedHeight, scrollHeight) : scrollHeight;
     } else {
       // Deletions, same-length replacements, viewport changes, and first render may
       // shrink the textarea, so measure from its intrinsic content height.
@@ -72,9 +80,9 @@ export function useAutoResizeTextarea(
     // The cached height can match even after this effect temporarily set `auto`, or
     // after callers cleared the inline style. Verify the DOM still has the px height
     // before skipping the write; otherwise large drafts collapse to the CSS min-height.
-    const currentInlineHeight = Number.parseFloat(el.style.height);
+    const currentInlineHeight = readInlineHeightPx(el);
     const inlineHeightMatches =
-      Number.isFinite(currentInlineHeight) && Math.abs(currentInlineHeight - nextHeight) < 0.5;
+      currentInlineHeight !== null && Math.abs(currentInlineHeight - nextHeight) < 0.5;
 
     if (appliedHeightRef.current !== nextHeight || !inlineHeightMatches) {
       el.style.height = `${nextHeight}px`;

src/browser/hooks/useAutoScroll.ts

@@ -83,6 +83,19 @@ export function useAutoScroll() {
     setAutoScroll(enabled);
   }, []);
 
+  // Seed the baseline read by handleScroll's released-branch direction check
+  // (`currentScrollTop > previousScrollTop`). Call this from any code path that
+  // flips autoScrollRef / programmaticDisableRef without a guaranteed follow-up
+  // scroll event — e.g. jumpToBottom skips the write when scrollTop is already
+  // max, and disableAutoScroll never fires a scroll event itself. Without a
+  // fresh baseline, the next user-driven scroll event could compare against a
+  // stale value (carried across workspace switches or the prior session) and
+  // misread a small wheel-up notch as "moving toward bottom", spuriously
+  // relocking the lock that was just released.
+  const seedScrollDirectionBaseline = useCallback(() => {
+    lastScrollTopRef.current = contentRef.current?.scrollTop ?? 0;
+  }, []);
+
   const stickToBottom = useCallback(() => {
     const scrollContainer = contentRef.current;
     if (!scrollContainer) return;
@@ -142,29 +155,22 @@ export function useAutoScroll() {
     programmaticDisableRef.current = false;
     setAutoScrollEnabled(true);
     stickToBottom();
-    // Seed the direction baseline used by handleScroll's released-branch
-    // user-intent path. stickToBottom doesn't always emit a scroll event
-    // (it skips the write when scrollTop is already max), so without this
-    // seed the next user-driven scroll event could compare against a stale
-    // value carried across workspace switches or earlier sessions.
-    lastScrollTopRef.current = contentRef.current?.scrollTop ?? 0;
+    // stickToBottom skips the write when scrollTop is already max, so we may
+    // not get a follow-up scroll event to refresh lastScrollTopRef.
+    seedScrollDirectionBaseline();
     startBottomLockFrameLoop();
-  }, [setAutoScrollEnabled, startBottomLockFrameLoop, stickToBottom]);
+  }, [seedScrollDirectionBaseline, setAutoScrollEnabled, startBottomLockFrameLoop, stickToBottom]);
 
   const disableAutoScroll = useCallback(() => {
     userScrollIntentUntilRef.current = 0;
     programmaticDisableRef.current = true;
     setAutoScrollEnabled(false);
-    // Seed the direction baseline. The released-branch user-intent path in
-    // handleScroll compares the next scroll event's scrollTop against
-    // lastScrollTopRef. disableAutoScroll never fires a scroll event itself,
-    // so without this seed a small wheel-up notch following a programmatic
-    // disable would be misread as "moving toward bottom" (because
-    // previousScrollTop was 0 or some unrelated earlier value), spuriously
-    // relocking the lock that was just disabled.
-    lastScrollTopRef.current = contentRef.current?.scrollTop ?? 0;
+    // disableAutoScroll never fires a scroll event itself, so seed the
+    // baseline now to keep the next user-driven scroll event's direction
+    // check honest.
+    seedScrollDirectionBaseline();
     stopBottomLockFrameLoop();
-  }, [setAutoScrollEnabled, stopBottomLockFrameLoop]);
+  }, [seedScrollDirectionBaseline, setAutoScrollEnabled, stopBottomLockFrameLoop]);
 
   const markUserScrollIntent = useCallback(() => {
     programmaticDisableRef.current = false;

src/browser/utils/chatCommands.test.ts

@@ -53,27 +53,25 @@ beforeEach(() => {
 });
 
 describe("parseRuntimeString", () => {
-  const workspaceName = "test-workspace";
-
   test("returns undefined for undefined runtime (default to worktree)", () => {
-    expect(parseRuntimeString(undefined, workspaceName)).toBeUndefined();
+    expect(parseRuntimeString(undefined)).toBeUndefined();
   });
 
   test("returns undefined for explicit 'worktree' runtime", () => {
-    expect(parseRuntimeString("worktree", workspaceName)).toBeUndefined();
-    expect(parseRuntimeString("WORKTREE", workspaceName)).toBeUndefined();
-    expect(parseRuntimeString(" worktree ", workspaceName)).toBeUndefined();
+    expect(parseRuntimeString("worktree")).toBeUndefined();
+    expect(parseRuntimeString("WORKTREE")).toBeUndefined();
+    expect(parseRuntimeString(" worktree ")).toBeUndefined();
   });
 
   test("returns local config for explicit 'local' runtime", () => {
     // "local" now returns project-dir runtime config (no srcBaseDir)
-    expect(parseRuntimeString("local", workspaceName)).toEqual({ type: "local" });
-    expect(parseRuntimeString("LOCAL", workspaceName)).toEqual({ type: "local" });
-    expect(parseRuntimeString(" local ", workspaceName)).toEqual({ type: "local" });
+    expect(parseRuntimeString("local")).toEqual({ type: "local" });
+    expect(parseRuntimeString("LOCAL")).toEqual({ type: "local" });
+    expect(parseRuntimeString(" local ")).toEqual({ type: "local" });
   });
 
   test("parses valid SSH runtime", () => {
-    const result = parseRuntimeString("ssh user@host", workspaceName);
+    const result = parseRuntimeString("ssh user@host");
     expect(result).toEqual({
       type: "ssh",
       host: "user@host",
@@ -82,7 +80,7 @@ describe("parseRuntimeString", () => {
   });
 
   test("preserves case in SSH host", () => {
-    const result = parseRuntimeString("ssh User@Host.Example.Com", workspaceName);
+    const result = parseRuntimeString("ssh User@Host.Example.Com");
     expect(result).toEqual({
       type: "ssh",
       host: "User@Host.Example.Com",
@@ -91,7 +89,7 @@ describe("parseRuntimeString", () => {
   });
 
   test("handles extra whitespace", () => {
-    const result = parseRuntimeString("  ssh   user@host  ", workspaceName);
+    const result = parseRuntimeString("  ssh   user@host  ");
     expect(result).toEqual({
       type: "ssh",
       host: "user@host",
@@ -100,12 +98,12 @@ describe("parseRuntimeString", () => {
   });
 
   test("throws error for SSH without host", () => {
-    expect(() => parseRuntimeString("ssh", workspaceName)).toThrow("SSH runtime requires host");
-    expect(() => parseRuntimeString("ssh ", workspaceName)).toThrow("SSH runtime requires host");
+    expect(() => parseRuntimeString("ssh")).toThrow("SSH runtime requires host");
+    expect(() => parseRuntimeString("ssh ")).toThrow("SSH runtime requires host");
   });
 
   test("accepts SSH with hostname only (user will be inferred)", () => {
-    const result = parseRuntimeString("ssh hostname", workspaceName);
+    const result = parseRuntimeString("ssh hostname");
     // Uses tilde path - backend will resolve it via runtime.resolvePath()
     expect(result).toEqual({
       type: "ssh",
@@ -115,7 +113,7 @@ describe("parseRuntimeString", () => {
   });
 
   test("accepts SSH with hostname.domain only", () => {
-    const result = parseRuntimeString("ssh dev.example.com", workspaceName);
+    const result = parseRuntimeString("ssh dev.example.com");
     // Uses tilde path - backend will resolve it via runtime.resolvePath()
     expect(result).toEqual({
       type: "ssh",
@@ -125,7 +123,7 @@ describe("parseRuntimeString", () => {
   });
 
   test("uses tilde path for root user too", () => {
-    const result = parseRuntimeString("ssh root@hostname", workspaceName);
+    const result = parseRuntimeString("ssh root@hostname");
     // Backend will resolve ~ to /root for root user
     expect(result).toEqual({
       type: "ssh",
@@ -135,52 +133,45 @@ describe("parseRuntimeString", () => {
   });
 
   test("parses docker runtime with image", () => {
-    const result = parseRuntimeString("docker ubuntu:22.04", workspaceName);
+    const result = parseRuntimeString("docker ubuntu:22.04");
     expect(result).toEqual({
       type: "docker",
       image: "ubuntu:22.04",
     });
   });
 
   test("parses devcontainer runtime with config path", () => {
-    const result = parseRuntimeString(
-      "devcontainer .devcontainer/devcontainer.json",
-      workspaceName
-    );
+    const result = parseRuntimeString("devcontainer .devcontainer/devcontainer.json");
     expect(result).toEqual({
       type: "devcontainer",
       configPath: ".devcontainer/devcontainer.json",
     });
   });
 
   test("throws error for devcontainer without config path", () => {
-    expect(() => parseRuntimeString("devcontainer", workspaceName)).toThrow(
+    expect(() => parseRuntimeString("devcontainer")).toThrow(
       "Dev container runtime requires a config path"
     );
   });
 
   test("parses docker with registry image", () => {
-    const result = parseRuntimeString("docker ghcr.io/myorg/dev:latest", workspaceName);
+    const result = parseRuntimeString("docker ghcr.io/myorg/dev:latest");
     expect(result).toEqual({
       type: "docker",
       image: "ghcr.io/myorg/dev:latest",
     });
   });
 
   test("throws error for docker without image", () => {
-    expect(() => parseRuntimeString("docker", workspaceName)).toThrow(
-      "Docker runtime requires image"
-    );
-    expect(() => parseRuntimeString("docker ", workspaceName)).toThrow(
-      "Docker runtime requires image"
-    );
+    expect(() => parseRuntimeString("docker")).toThrow("Docker runtime requires image");
+    expect(() => parseRuntimeString("docker ")).toThrow("Docker runtime requires image");
   });
 
   test("throws error for unknown runtime type", () => {
-    expect(() => parseRuntimeString("remote", workspaceName)).toThrow(
+    expect(() => parseRuntimeString("remote")).toThrow(
       "Unknown runtime type: 'remote'. Use 'ssh <host>', 'docker <image>', 'devcontainer <config>', 'worktree', or 'local'"
     );
-    expect(() => parseRuntimeString("kubernetes", workspaceName)).toThrow(
+    expect(() => parseRuntimeString("kubernetes")).toThrow(
       "Unknown runtime type: 'kubernetes'. Use 'ssh <host>', 'docker <image>', 'devcontainer <config>', 'worktree', or 'local'"
     );
   });

src/browser/utils/chatCommands.ts

@@ -685,10 +685,7 @@ async function handleForkCommand(
  * - "devcontainer <configPath>" -> Dev container runtime
  * - undefined -> Worktree runtime (default)
  */
-export function parseRuntimeString(
-  runtime: string | undefined,
-  _workspaceName: string
-): RuntimeConfig | undefined {
+export function parseRuntimeString(runtime: string | undefined): RuntimeConfig | undefined {
   // Use shared parser from common/types/runtime
   const parsed = parseRuntimeModeAndHost(runtime);
 
@@ -801,13 +798,8 @@ export async function createNewWorkspace(
     }
   }
 
-  // Parse runtime config if provided. Use a placeholder when no caller-provided
-  // workspace name is available (auto-name path); parseRuntimeString only uses
-  // the name for error reporting context.
-  const runtimeConfig = parseRuntimeString(
-    effectiveRuntime,
-    options.workspaceName ?? "(auto-generated)"
-  );
+  // Parse runtime config if provided.
+  const runtimeConfig = parseRuntimeString(effectiveRuntime);
 
   const result = await options.client.workspace.create({
     projectPath: options.projectPath,