🤖 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

@@ -1246,7 +1246,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
@@ -1259,15 +1259,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/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,

src/browser/utils/messages/StreamingMessageAggregator.ts

@@ -27,6 +27,7 @@ import {
   type StreamLifecycleSnapshot,
 } from "@/common/types/stream";
 import type { LanguageModelV2Usage } from "@ai-sdk/provider";
+import type { StreamErrorType } from "@/common/types/errors";
 import type { TodoItem, StatusSetToolResult, NotifyToolResult } from "@/common/types/tools";
 import { completeInProgressTodoItems } from "@/common/utils/todoList";
 import { getToolOutputUiOnly } from "@/common/utils/tools/toolOutputUiOnly";
@@ -3064,20 +3065,37 @@ export class StreamingMessageAggregator {
         }
       });
 
-      // Create stream-error DisplayedMessage if message has error metadata
-      // This happens after all parts are displayed, so error appears at the end
-      if (message.metadata?.error) {
+      // Both stream-error rows (real error metadata + synthesized
+      // max_tokens truncation) share the same parent-message-derived
+      // fields. Capture them in one place so adding a new branch later
+      // can't accidentally drift on `model` / `routedThroughGateway` /
+      // `historySequence` / `timestamp`.
+      const pushStreamErrorRow = (
+        idSuffix: string,
+        error: string,
+        errorType: StreamErrorType
+      ): void => {
         displayedMessages.push({
           type: "stream-error",
-          id: `${message.id}-error`,
+          id: `${message.id}-${idSuffix}`,
           historyId: message.id,
-          error: message.metadata.error,
-          errorType: message.metadata.errorType ?? "unknown",
+          error,
+          errorType,
           historySequence,
-          model: message.metadata.model,
+          model: message.metadata?.model,
           routedThroughGateway: message.metadata?.routedThroughGateway,
           timestamp: baseTimestamp,
         });
+      };
+
+      // Create stream-error DisplayedMessage if message has error metadata
+      // This happens after all parts are displayed, so error appears at the end
+      if (message.metadata?.error) {
+        pushStreamErrorRow(
+          "error",
+          message.metadata.error,
+          message.metadata.errorType ?? "unknown"
+        );
       } else if (
         // Stream ended cleanly *but* the provider truncated us at max_tokens.
         // The backend's stream-end path treats this as a successful completion
@@ -3090,19 +3108,12 @@ export class StreamingMessageAggregator {
         !hasActiveStream &&
         message.metadata?.finishReason === "length"
       ) {
-        displayedMessages.push({
-          type: "stream-error",
-          id: `${message.id}-length`,
-          historyId: message.id,
-          error:
-            "The model hit its max output token limit before finishing this response. " +
+        pushStreamErrorRow(
+          "length",
+          "The model hit its max output token limit before finishing this response. " +
             "Lower the thinking level (or split the turn into smaller steps) to give it more headroom.",
-          errorType: "max_output_tokens",
-          historySequence,
-          model: message.metadata.model,
-          routedThroughGateway: message.metadata?.routedThroughGateway,
-          timestamp: baseTimestamp,
-        });
+          "max_output_tokens"
+        );
       }
     }