From bccd53e108a5aefa003c7c11740928480b4d3d5f Mon Sep 17 00:00:00 2001 From: "mux-bot[bot]" <264182336+mux-bot[bot]@users.noreply.github.com> Date: Thu, 30 Apr 2026 20:26:07 +0000 Subject: [PATCH 01/11] refactor: route ThemeContext color-scheme through isLightThemeMode Replace the inline `theme === "light" || theme === "flexoki-light"` check in `getColorScheme` with the shared `isLightThemeMode` helper from `shiki-shared.ts`, so the `-light` suffix convention has a single source of truth and stays consistent with the syntax-highlighting call sites that already use it. --- src/browser/contexts/ThemeContext.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/browser/contexts/ThemeContext.tsx b/src/browser/contexts/ThemeContext.tsx index 5e83061b65..fc20eaa609 100644 --- a/src/browser/contexts/ThemeContext.tsx +++ b/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) { From ae731ca89cd9d0049d27abfb2c3c009545df5884 Mon Sep 17 00:00:00 2001 From: mux-bot Date: Fri, 1 May 2026 12:22:54 +0000 Subject: [PATCH 02/11] refactor: drop unused appendSpace literals on skill/model alias suggestions The skill and model alias build callbacks in getSlashCommandSuggestions hardcode the trailing space, so the appendSpace: true property on those SuggestionDefinition literals is dead. Remove it and add a brief comment explaining why we don't propagate appendSpace through these paths so a future reader doesn't assume the field is consulted there. --- src/browser/utils/slashCommands/suggestions.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/browser/utils/slashCommands/suggestions.ts b/src/browser/utils/slashCommands/suggestions.ts index 35d6751886..d43b896c43 100644 --- a/src/browser/utils/slashCommands/suggestions.ts +++ b/src/browser/utils/slashCommands/suggestions.ts @@ -82,12 +82,14 @@ function buildTopLevelSuggestions( return scope; }; + // The skill build callback below hardcodes the trailing space, so we omit + // `appendSpace` here — leaving it set would be a no-op and falsely suggest + // the build path consults it. const skillDefinitions: SuggestionDefinition[] = (context.agentSkills ?? []) .filter((skill) => !SLASH_COMMAND_DEFINITION_MAP.has(skill.name)) .map((skill) => ({ key: skill.name, description: `${skill.description} (${formatScopeLabel(skill.scope)})`, - appendSpace: true, })); const skillSuggestions = filterAndMapSuggestions(skillDefinitions, partial, (definition) => { @@ -100,12 +102,13 @@ function buildTopLevelSuggestions( }; }); - // Model alias one-shot suggestions (e.g., /haiku, /sonnet, /opus+high) + // Model alias one-shot suggestions (e.g., /haiku, /sonnet, /opus+high). + // The build callback below hardcodes the trailing space, so `appendSpace` + // is intentionally omitted here. const modelAliasDefinitions: SuggestionDefinition[] = Object.entries(MODEL_ABBREVIATIONS).map( ([alias, modelId]) => ({ key: alias, description: `Send with ${formatModelDisplayName(modelId.split(":")[1] ?? modelId)} (one message, +level for thinking)`, - appendSpace: true, }) ); From 4bba939b9a34ff7fb55130ad3384577ed6768736 Mon Sep 17 00:00:00 2001 From: mux Date: Fri, 1 May 2026 16:23:22 +0000 Subject: [PATCH 03/11] refactor: extract shared ServiceTier type from ServiceTierSchema The OpenAI service-tier literal union ('auto' | 'default' | 'flex' | 'priority') was duplicated in three places: the CLI options interface (src/cli/run.ts), the providerModelFactory cast at the providers.jsonc boundary, and a local OpenAIServiceTier alias in ProvidersSection.tsx. ServiceTierSchema (src/common/config/schemas/providersConfig.ts) already defines this enum as the runtime source of truth, so derive a TypeScript ServiceTier alias via z.infer once and import it at each site. Pure type-only refactor; the emitted JS and the schema remain unchanged. --- src/browser/features/Settings/Sections/ProvidersSection.tsx | 3 ++- src/cli/run.ts | 3 ++- src/common/config/schemas/providersConfig.ts | 1 + src/node/services/providerModelFactory.ts | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/browser/features/Settings/Sections/ProvidersSection.tsx b/src/browser/features/Settings/Sections/ProvidersSection.tsx index b7c140a04c..7d82c7b08e 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.tsx @@ -78,6 +78,7 @@ 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"; @@ -85,7 +86,7 @@ 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 { diff --git a/src/cli/run.ts b/src/cli/run.ts index 8037588351..233c9f0283 100644 --- a/src/cli/run.ts +++ b/src/cli/run.ts @@ -35,6 +35,7 @@ import { type SendMessageOptions, type WorkspaceChatMessage, } from "../common/orpc/types"; +import type { ServiceTier } from "../common/config/schemas/providersConfig"; import { createDisplayUsage } from "../common/utils/tokens/displayUsage"; import { getTotalCost, @@ -298,7 +299,7 @@ interface CLIOptions { mcpConfig: boolean; experiment: string[]; budget?: number; - serviceTier?: "auto" | "default" | "flex" | "priority"; + serviceTier?: ServiceTier; use1m?: boolean; keepBackgroundProcesses?: boolean; } diff --git a/src/common/config/schemas/providersConfig.ts b/src/common/config/schemas/providersConfig.ts index 8748293e27..949ab787a2 100644 --- a/src/common/config/schemas/providersConfig.ts +++ b/src/common/config/schemas/providersConfig.ts @@ -5,6 +5,7 @@ import { ProviderModelEntrySchema } from "./providerModelEntry"; export const CacheTtlSchema = z.enum(["5m", "1h"]); export const ServiceTierSchema = z.enum(["auto", "default", "flex", "priority"]); +export type ServiceTier = z.infer; export const CodexOauthDefaultAuthSchema = z.enum(["oauth", "apiKey"]); export const BaseProviderConfigSchema = z diff --git a/src/node/services/providerModelFactory.ts b/src/node/services/providerModelFactory.ts index fea730ba1f..493d3ba97d 100644 --- a/src/node/services/providerModelFactory.ts +++ b/src/node/services/providerModelFactory.ts @@ -20,6 +20,7 @@ import { import { parseCodexOauthAuth } from "@/node/utils/codexOauthAuth"; import type { Config, ProviderConfig, ProvidersConfig } from "@/node/config"; import type { MuxProviderOptions } from "@/common/types/providerOptions"; +import type { ServiceTier } from "@/common/config/schemas/providersConfig"; import type { ExternalSecretResolver } from "@/common/types/secrets"; import { isOpReference } from "@/common/utils/opRef"; import { isProviderDisabledInConfig } from "@/common/utils/providers/isProviderDisabled"; @@ -1279,7 +1280,7 @@ export class ProviderModelFactory { if (configServiceTier && muxProviderOptions.openai?.serviceTier == null) { muxProviderOptions.openai = { ...muxProviderOptions.openai, - serviceTier: configServiceTier as "auto" | "default" | "flex" | "priority", + serviceTier: configServiceTier as ServiceTier, }; } if (configWireFormat === "responses" || configWireFormat === "chatCompletions") { From 7d8f5e5e905854470b9be2c274dfc84d5463432c Mon Sep 17 00:00:00 2001 From: "mux-bot[bot]" <264182336+mux-bot[bot]@users.noreply.github.com> Date: Sat, 2 May 2026 00:28:07 +0000 Subject: [PATCH 04/11] refactor: extract ResolvedWorkspaceAiSettings type alias in taskService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shape `{ model: string; thinkingLevel?: ThinkingLevel }` (used internally to read per-agent AI settings off partial workspace metadata) was duplicated 7 times across the parameter and return types of `resolveWorkspaceAISettings`, `resolveTaskAISettings`, and `resolveParentAutoResumeOptions`. The new sub-agent defaults split (#3215) made this duplication especially visible because it added a third method copying the same inline shape. Introduce a private `ResolvedWorkspaceAiSettings` interface at module scope and use it everywhere. Pure type-level cleanup — emitted JS, runtime behavior, and the schema-derived `WorkspaceAISettings` type (where `thinkingLevel` is required) are all unchanged. 🤖 _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh`_ --- src/node/services/taskService.ts | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index bea5c35487..90ec917d5a 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -88,6 +88,17 @@ export type TaskKind = "agent"; export type AgentTaskStatus = NonNullable; +/** + * Resolved per-agent AI settings (canonical model + optional thinking level). + * + * `thinkingLevel` is optional because internal callers read these settings off of + * partial workspace metadata where the field may be missing on older entries. + */ +interface ResolvedWorkspaceAiSettings { + model: string; + thinkingLevel?: ThinkingLevel; +} + export interface AgentTaskStatusLookup { exists: boolean; taskStatus: AgentTaskStatus | null; @@ -382,11 +393,11 @@ export class TaskService { // fall back to legacy workspace settings for older configs. private resolveWorkspaceAISettings( workspace: { - aiSettingsByAgent?: Record; - aiSettings?: { model: string; thinkingLevel?: ThinkingLevel }; + aiSettingsByAgent?: Record; + aiSettings?: ResolvedWorkspaceAiSettings; }, agentId: string | undefined - ): { model: string; thinkingLevel?: ThinkingLevel } | undefined { + ): ResolvedWorkspaceAiSettings | undefined { const normalizedAgentId = typeof agentId === "string" && agentId.trim().length > 0 ? normalizeAgentId(agentId, "") @@ -400,8 +411,8 @@ export class TaskService { private resolveTaskAISettings(params: { cfg: ReturnType; parentMeta: { - aiSettingsByAgent?: Record; - aiSettings?: { model: string; thinkingLevel?: ThinkingLevel }; + aiSettingsByAgent?: Record; + aiSettings?: ResolvedWorkspaceAiSettings; }; agentId: string; modelString?: string; @@ -450,8 +461,8 @@ export class TaskService { parentWorkspaceId: string, parentEntry: { workspace: { - aiSettingsByAgent?: Record; - aiSettings?: { model: string; thinkingLevel?: ThinkingLevel }; + aiSettingsByAgent?: Record; + aiSettings?: ResolvedWorkspaceAiSettings; }; }, fallbackModel: string, From 1964b0603183a92fa53de1112ba1f6156a5f1fe8 Mon Sep 17 00:00:00 2001 From: "mux-bot[bot]" <264182336+mux-bot[bot]@users.noreply.github.com> Date: Sat, 2 May 2026 05:04:35 +0000 Subject: [PATCH 05/11] refactor: drop redundant GuardAnchors type alias in file_edit_insert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In `src/node/services/tools/file_edit_insert.ts`, `GuardAnchors` was defined as `Pick`, but `InsertContentOptions` itself is already `Pick` after the `.nullish()` strict-mode refactor in #2250 stripped the `InsertContentOptions` interface down to those same two fields. The two aliases are now structurally identical, so `GuardAnchors` is dead. Drop the alias and use `InsertContentOptions` for the two callers (`insertWithGuards`, `resolveGuardAnchor`). Both names were file-local; no exports change. The function names (`insertWithGuards`, `resolveGuardAnchor`) already convey "guard" context, so the parameter type doesn't need to repeat it. Pure type-level cleanup — emitted JS, runtime behavior, and the public tool surface are all unchanged. --- src/node/services/tools/file_edit_insert.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/node/services/tools/file_edit_insert.ts b/src/node/services/tools/file_edit_insert.ts index fd0354b2b2..12a7f6a6f6 100644 --- a/src/node/services/tools/file_edit_insert.ts +++ b/src/node/services/tools/file_edit_insert.ts @@ -44,8 +44,6 @@ function guardFailure(error: string): InsertOperationFailure { }; } -type GuardAnchors = Pick; - export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) => { return tool({ description: TOOL_DEFINITIONS.file_edit_insert.description, @@ -175,7 +173,7 @@ function insertContent( function insertWithGuards( originalContent: string, contentToInsert: string, - anchors: GuardAnchors + anchors: InsertContentOptions ): InsertOperationSuccess | InsertOperationFailure { const anchorResult = resolveGuardAnchor(originalContent, anchors); if (!anchorResult.success) { @@ -216,7 +214,7 @@ function findUniqueSubstringIndex( function resolveGuardAnchor( originalContent: string, - { insert_before, insert_after }: GuardAnchors + { insert_before, insert_after }: InsertContentOptions ): GuardResolutionSuccess | InsertOperationFailure { const fileEol = detectFileEol(originalContent); From b88c7ad4c3c23c0036fd87d1f0689e67e34253ae Mon Sep 17 00:00:00 2001 From: ammar-agent Date: Sat, 2 May 2026 20:18:11 +0000 Subject: [PATCH 06/11] refactor: extract pushStreamErrorRow helper in StreamingMessageAggregator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both stream-error rows in `buildDisplayedMessagesForMessage` (the existing `message.metadata?.error` branch and the new `finishReason === "length"` branch added in #3223) push structurally identical objects, differing only in `id` suffix, `error` string, and `errorType`. The shared parent-message-derived fields (`historyId`, `historySequence`, `model`, `routedThroughGateway`, `timestamp`) were duplicated across both pushes. Extract a local `pushStreamErrorRow` closure that captures the shared fields once. Each branch now reduces to a single call passing the three differing values. Pure refactor — emitted DisplayedMessage objects are identical. --- .../messages/StreamingMessageAggregator.ts | 49 ++++++++++++------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index 76987e5bb6..655e924e4f 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/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" + ); } } From 3a73dfed3191ec6290494b57512344a50b797a0a Mon Sep 17 00:00:00 2001 From: ammar-agent Date: Sun, 3 May 2026 05:10:00 +0000 Subject: [PATCH 07/11] refactor: extract seedScrollDirectionBaseline helper in useAutoScroll --- src/browser/hooks/useAutoScroll.ts | 38 +++++++++++++++++------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/browser/hooks/useAutoScroll.ts b/src/browser/hooks/useAutoScroll.ts index dfe455c3c7..ac990558ca 100644 --- a/src/browser/hooks/useAutoScroll.ts +++ b/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; From e68beef4978a96469347a7b8c5e11d31c293652c Mon Sep 17 00:00:00 2001 From: "mux-bot[bot]" <264182336+mux-bot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 16:20:39 +0000 Subject: [PATCH 08/11] refactor: drop redundant isPlanHandoffAgent boolean in streamContextBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `isPlanHandoffAgent` boolean in `buildPlanInstructions` was extracted when the gate was `effectiveAgentId === "exec" || effectiveAgentId === "orchestrator"`. After #3224 ripped out the Orchestrator agent, the boolean collapsed to a single equality check (`effectiveAgentId === "exec"`), and the trailing `else if (isPlanHandoffAgent && chatHasStartHerePlanSummary)` redundantly re-evaluated the same gate just to log a debug line. Replace the flat `if/else if` with a nested `if (effectiveAgentId === "exec") { … }` that tests the Start Here summary inside, removing the duplicate gate re-check and the now-meaningless boolean. A short comment captures the rationale so a future reader doesn't reintroduce the alias. Pure refactor — emitted control flow, the debug log, and `planContentForTransition` assignments are identical, and the existing 8-test `streamContextBuilder.test.ts` suite passes unchanged (257 tests across the related taskService / modelMessageTransform / aiService suites also pass). --- src/node/services/streamContextBuilder.ts | 78 ++++++++++++----------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/src/node/services/streamContextBuilder.ts b/src/node/services/streamContextBuilder.ts index 7185bc8c75..22a422f355 100644 --- a/src/node/services/streamContextBuilder.ts +++ b/src/node/services/streamContextBuilder.ts @@ -167,48 +167,52 @@ export async function buildPlanInstructions( // Read plan content for agent transition (plan-like → exec). // Only read if switching to the built-in exec agent and last assistant was plan-like. + // The `effectiveAgentId === "exec"` gate used to also include "orchestrator" + // (extracted to an `isPlanHandoffAgent` boolean) before #3224 ripped that agent + // out; the boolean is now redundant with a single equality check, so inline it. let planContentForTransition: string | undefined; - const isPlanHandoffAgent = effectiveAgentId === "exec"; - if (isPlanHandoffAgent && !chatHasStartHerePlanSummary) { - const lastAssistantMessage = [...requestPayloadMessages] - .reverse() - .find((m) => m.role === "assistant"); - const lastAgentId = lastAssistantMessage?.metadata?.agentId; - if (lastAgentId && planResult.content.trim()) { - let lastAgentIsPlanLike = false; - if (lastAgentId === effectiveAgentId) { - lastAgentIsPlanLike = agentIsPlanLike; - } else { - try { - const lastDefinition = await readAgentDefinition( - runtime, - agentDiscoveryPath, - lastAgentId - ); - const lastChain = await resolveAgentInheritanceChain({ - runtime, - workspacePath: agentDiscoveryPath, - agentId: lastAgentId, - agentDefinition: lastDefinition, - workspaceId, - }); - lastAgentIsPlanLike = isPlanLikeInResolvedChain(lastChain); - } catch (error) { - workspaceLog.warn("Failed to resolve last agent definition for plan handoff", { - lastAgentId, - error: getErrorMessage(error), - }); + if (effectiveAgentId === "exec") { + if (chatHasStartHerePlanSummary) { + workspaceLog.debug( + "Skipping plan content injection for plan handoff transition: Start Here already includes the plan in chat history." + ); + } else { + const lastAssistantMessage = [...requestPayloadMessages] + .reverse() + .find((m) => m.role === "assistant"); + const lastAgentId = lastAssistantMessage?.metadata?.agentId; + if (lastAgentId && planResult.content.trim()) { + let lastAgentIsPlanLike = false; + if (lastAgentId === effectiveAgentId) { + lastAgentIsPlanLike = agentIsPlanLike; + } else { + try { + const lastDefinition = await readAgentDefinition( + runtime, + agentDiscoveryPath, + lastAgentId + ); + const lastChain = await resolveAgentInheritanceChain({ + runtime, + workspacePath: agentDiscoveryPath, + agentId: lastAgentId, + agentDefinition: lastDefinition, + workspaceId, + }); + lastAgentIsPlanLike = isPlanLikeInResolvedChain(lastChain); + } catch (error) { + workspaceLog.warn("Failed to resolve last agent definition for plan handoff", { + lastAgentId, + error: getErrorMessage(error), + }); + } } - } - if (lastAgentIsPlanLike) { - planContentForTransition = planResult.content; + if (lastAgentIsPlanLike) { + planContentForTransition = planResult.content; + } } } - } else if (isPlanHandoffAgent && chatHasStartHerePlanSummary) { - workspaceLog.debug( - "Skipping plan content injection for plan handoff transition: Start Here already includes the plan in chat history." - ); } return { effectiveAdditionalInstructions, planFilePath, planContentForTransition }; From 770e31428363931c7acdce9fbe6e5aa936b1cc84 Mon Sep 17 00:00:00 2001 From: "mux-bot[bot]" <264182336+mux-bot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 20:17:34 +0000 Subject: [PATCH 09/11] refactor: drop unused workspaceName param from parseRuntimeString MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second argument was named `_workspaceName` (underscore-prefixed = unused) since it was introduced in the original SSH runtime PR and has never been referenced by the function body. The /new simplification (#3230) made the noise especially visible: the new `createNewWorkspace` call site had to pass `options.workspaceName ?? "(auto-generated)"` purely to satisfy the signature, and added a comment claiming `parseRuntimeString only uses the name for error reporting context` — but the function never reads it. Drop the parameter from the signature, the placeholder + misleading comment at the only non-test caller, and the boilerplate `workspaceName` constant + 23 second-arguments in `chatCommands.test.ts`. Pure dead-parameter cleanup — emitted JS, error messages, and runtime behavior are all identical. Mobile already had the cleaner shape (`parseRuntimeStringForMobile(runtime?: string)`) so the desktop signature now matches it. --- src/browser/utils/chatCommands.test.ts | 55 +++++++++++--------------- src/browser/utils/chatCommands.ts | 14 ++----- 2 files changed, 26 insertions(+), 43 deletions(-) diff --git a/src/browser/utils/chatCommands.test.ts b/src/browser/utils/chatCommands.test.ts index 5af6130512..7a467f3b8c 100644 --- a/src/browser/utils/chatCommands.test.ts +++ b/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,7 +133,7 @@ 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", @@ -143,10 +141,7 @@ describe("parseRuntimeString", () => { }); 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", @@ -154,13 +149,13 @@ describe("parseRuntimeString", () => { }); 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", @@ -168,19 +163,15 @@ describe("parseRuntimeString", () => { }); 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 ', 'docker ', 'devcontainer ', 'worktree', or 'local'" ); - expect(() => parseRuntimeString("kubernetes", workspaceName)).toThrow( + expect(() => parseRuntimeString("kubernetes")).toThrow( "Unknown runtime type: 'kubernetes'. Use 'ssh ', 'docker ', 'devcontainer ', 'worktree', or 'local'" ); }); diff --git a/src/browser/utils/chatCommands.ts b/src/browser/utils/chatCommands.ts index bb51128e7d..a035b95a32 100644 --- a/src/browser/utils/chatCommands.ts +++ b/src/browser/utils/chatCommands.ts @@ -685,10 +685,7 @@ async function handleForkCommand( * - "devcontainer " -> 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, From a3a0b86bcbca534cde474624c0e227ca3703e2bb Mon Sep 17 00:00:00 2001 From: "mux-bot[bot]" <264182336+mux-bot[bot]@users.noreply.github.com> Date: Tue, 5 May 2026 20:28:19 +0000 Subject: [PATCH 10/11] refactor: drop dead length guard in parseBedrockModelName secondPart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The early `dotParts.length < 2` return at the top of `parseBedrockModelName` already guarantees `dotParts.length >= 2` by the time we compute `secondPart`, so the `dotParts.length > 1 ? dotParts[1].toLowerCase() : ""` ternary's empty-string branch is unreachable. The Deepseek V4 commit (#3237) extended the surrounding logic but didn't introduce this asymmetry — `firstPart` on the line above already accesses `dotParts[0].toLowerCase()` without a guard, so the redundant ternary on `secondPart` was the odd one out. Inline to a direct `dotParts[1].toLowerCase()` access (matching `firstPart`'s shape) and capture the rationale in a one-line comment so a future reader doesn't reintroduce the guard. Pure dead-code cleanup — emitted JS, runtime behavior, and the existing 18-test `modelDisplay` suite (including the new DeepSeek + Bedrock cases) are all unchanged. --- src/common/utils/ai/modelDisplay.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/utils/ai/modelDisplay.ts b/src/common/utils/ai/modelDisplay.ts index 216176ec5f..431d89e917 100644 --- a/src/common/utils/ai/modelDisplay.ts +++ b/src/common/utils/ai/modelDisplay.ts @@ -237,8 +237,10 @@ function parseBedrockModelName(modelId: string): string | null { const knownVendors = ["anthropic", "amazon", "meta", "cohere", "mistral", "ai21"]; const knownRegionPrefixes = ["global", "us", "eu", "ap", "sa"]; + // The early `dotParts.length < 2` return above guarantees both indices exist here, + // so neither access needs a length guard. const firstPart = dotParts[0].toLowerCase(); - const secondPart = dotParts.length > 1 ? dotParts[1].toLowerCase() : ""; + const secondPart = dotParts[1].toLowerCase(); // Format is either: vendor.model or region.vendor.model const isVendor = knownVendors.includes(firstPart); From cb730d352a11dc4abde3b17b65270c98d0187b94 Mon Sep 17 00:00:00 2001 From: mux-auto-cleanup Date: Wed, 6 May 2026 05:09:15 +0000 Subject: [PATCH 11/11] refactor: extract isAgentTaskActiveStatus predicate in task_await MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dedupe the three call sites in task_await.ts that gated on the active agent-task subset (queued | running | awaiting_report). The duplication became especially visible in #3234, which extended both the timeout=0 and "timed out" branches symmetrically with `getAgentTaskElapsedField`, making the two structurally identical `{ status, taskId, ...elapsed }` returns sit a few lines apart from a third copy in the ForegroundWaitBackgroundedError branch that picked between the same three values. Add a local `isAgentTaskActiveStatus` type predicate (with the `AgentTaskActiveStatus` subset alias) at the top of the file alongside the existing `coerceTimeoutMs` / `parseTimestampMs` helpers and replace all three inline checks with a single call. The predicate narrows the nullable `AgentTaskStatus | null` return of `getAgentTaskStatus` to the active subset so the existing returns keep their narrowed `status` field without `as const` gymnastics. Pure refactor — emitted return shapes (including `elapsed_ms`), narrowed `status` literals, and the existing 17-test `task_await` suite all pass unchanged. --- src/node/services/tools/task_await.ts | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/node/services/tools/task_await.ts b/src/node/services/tools/task_await.ts index b8c6680039..5d05b28d69 100644 --- a/src/node/services/tools/task_await.ts +++ b/src/node/services/tools/task_await.ts @@ -15,10 +15,22 @@ import { import { getErrorMessage } from "@/common/utils/errors"; import { ForegroundWaitBackgroundedError, + type AgentTaskStatus, type AgentTaskStatusLookup, type AgentTaskTimestamps, } from "@/node/services/taskService"; +// Status values for which task_await still treats an agent task as live and +// should surface the live status (plus an `elapsed_ms` field) instead of +// awaiting a report. Centralised here so the timeout=0 and "timed out" error +// branches below stay in lockstep when shared fields are added — see #3234, +// which extended both branches symmetrically with `getAgentTaskElapsedField`. +type AgentTaskActiveStatus = "queued" | "running" | "awaiting_report"; + +function isAgentTaskActiveStatus(status: AgentTaskStatus | null): status is AgentTaskActiveStatus { + return status === "queued" || status === "running" || status === "awaiting_report"; +} + function coerceTimeoutMs(timeoutSecs: unknown): number | undefined { if (typeof timeoutSecs !== "number" || !Number.isFinite(timeoutSecs)) return undefined; if (timeoutSecs < 0) return undefined; @@ -247,7 +259,7 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { // current task status instead of awaiting. if (timeoutMs === 0) { const status = taskService.getAgentTaskStatus(taskId); - if (status === "queued" || status === "running" || status === "awaiting_report") { + if (isAgentTaskActiveStatus(status)) { return { status, taskId, ...getAgentTaskElapsedField(taskId) }; } @@ -299,12 +311,9 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { } catch (error: unknown) { if (error instanceof ForegroundWaitBackgroundedError) { const currentStatus = taskService.getAgentTaskStatus(taskId); - const normalizedStatus = - currentStatus === "queued" || - currentStatus === "running" || - currentStatus === "awaiting_report" - ? currentStatus - : ("running" as const); + const normalizedStatus = isAgentTaskActiveStatus(currentStatus) + ? currentStatus + : ("running" as const); return { status: normalizedStatus, taskId, @@ -323,7 +332,7 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { } if (/timed out/i.test(message)) { const status = taskService.getAgentTaskStatus(taskId); - if (status === "queued" || status === "running" || status === "awaiting_report") { + if (isAgentTaskActiveStatus(status)) { return { status, taskId, ...getAgentTaskElapsedField(taskId) }; } if (!status) {