🤖 refactor: auto-cleanup

[mux-bot]

Summary

Long-lived auto-cleanup PR that accumulates low-risk, behavior-preserving refactors picked from recent main commits.

Changes

Drop dead length guard in parseBedrockModelName secondPart

The early dotParts.length < 2 return at the top of parseBedrockModelName (in src/common/utils/ai/modelDisplay.ts) already guarantees dotParts.length >= 2 by the time secondPart is computed, which makes the dotParts.length > 1 ? dotParts[1].toLowerCase() : "" ternary's empty-string branch unreachable. The DeepSeek V4 commit (#3237) extended the surrounding formatter without touching this site, but the new DeepSeek branch made the asymmetry more obvious — the line directly above (firstPart) already accesses dotParts[0].toLowerCase() without a guard, so the 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 + existing Bedrock cases) are all unchanged.

Previous cleanups

Drop unused workspaceName param from parseRuntimeString

The second argument to parseRuntimeString (in src/browser/utils/chatCommands.ts) was named _workspaceName (underscore-prefixed = unused) when it was introduced in the original SSH runtime PR, and has never been referenced by the function body — error messages are all generic and don't include any workspace-specific context.

The /new-mirrors-/fork 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. Mobile already had the cleaner shape (parseRuntimeStringForMobile(runtime?: string)).

Drop the parameter from the signature, drop the placeholder + misleading comment at the only non-test caller (createNewWorkspace), and drop the workspaceName = "test-workspace" constant + 23 second-arguments in chatCommands.test.ts. Pure dead-parameter cleanup — emitted JS, error messages, and runtime behavior are all identical, and the desktop signature now matches mobile's.

Drop redundant isPlanHandoffAgent boolean in streamContextBuilder

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.

Extract seedScrollDirectionBaseline helper in useAutoScroll

useAutoScroll (touched by #3226) seeds lastScrollTopRef from two call sites — jumpToBottom and disableAutoScroll — to keep handleScroll's released-branch direction check (currentScrollTop > previousScrollTop) honest after a programmatic ownership transfer that may not produce a follow-up scroll event. Both sites repeated the same write (lastScrollTopRef.current = contentRef.current?.scrollTop ?? 0) and a multi-line block comment re-explaining the same shared rationale.

Extract a seedScrollDirectionBaseline useCallback with the shared rationale captured once on the helper itself. Each call site reduces to a single call plus a one-line comment naming the site-specific reason it doesn't get a free scroll-event refresh (stickToBottom skips the write at max; disableAutoScroll never fires a scroll event itself). The dependency arrays for jumpToBottom and disableAutoScroll add the new helper, which has empty deps (useCallback(..., [])) so its identity is stable across renders — no extra re-creations of the surrounding callbacks.

This shrinks the surface area for future drift: a third path that needs to flip autoScrollRef/programmaticDisableRef without a guaranteed scroll-event tail (e.g. the deferred workspace-switch hydration race called out in #3226's "Pains") can call the helper instead of duplicating the rationale a third time. Pure refactor — emitted writes, write order, and ref values are identical, and the existing 25-test useAutoScroll suite (including the 6 new regression tests added in #3226) passes unchanged.

Extract pushStreamErrorRow helper in StreamingMessageAggregator

buildDisplayedMessagesForMessage now has two branches that synthesize a stream-error DisplayedMessage: the existing message.metadata?.error path and the finishReason === "length" path added in #3223. Both pushes were structurally identical, differing only in id suffix, error string, and errorType — the parent-message-derived fields (historyId, historySequence, model, routedThroughGateway, timestamp) were duplicated across both call sites.

Extract a local pushStreamErrorRow closure that captures the shared fields once. Each branch reduces to a single call passing the three differing values. The model access switches from message.metadata.model (which relied on the outer if (message.metadata?.error) narrowing) to message.metadata?.model, which is functionally identical when metadata is defined and falls through to undefined otherwise — same emitted value either way.

This shrinks the surface area for future drift: the next branch added (e.g. a different finish-reason synthesis) can't accidentally pass a stale model or forget routedThroughGateway. Pure refactor — emitted DisplayedMessage objects are identical, and the existing 77-test SMA suite (including the 6 new max-tokens tests) passes unchanged.

Drop redundant GuardAnchors type alias in file_edit_insert

In src/node/services/tools/file_edit_insert.ts, GuardAnchors was defined as Pick<InsertContentOptions, "insert_before" | "insert_after">, but InsertContentOptions itself is already Pick<FileEditInsertToolArgs, "insert_before" | "insert_after"> after the .nullish() strict-mode refactor in #2250 stripped the InsertContentOptions interface down to those same two fields. The two aliases became 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 already convey "guard" context, so the parameter type doesn't need to repeat it. This noise was especially visible to the new guardless-empty-file path added in #3220 since it sits next to insertContent which uses InsertContentOptions.

Pure type-level cleanup — emitted JS, runtime behavior, and the public tool surface are all unchanged.

Extract ResolvedWorkspaceAiSettings type alias in taskService

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 in src/node/services/taskService.ts. The new sub-agent defaults split (#3215) made this duplication especially visible because it added a third method copying the same inline shape.

A private ResolvedWorkspaceAiSettings interface at module scope replaces all 7 inline occurrences. Pure type-level cleanup — emitted JS, runtime behavior, and the schema-derived WorkspaceAISettings type (where thinkingLevel is required) are all unchanged. The new interface is intentionally local to this file (not exported) since it captures the looser internal-reader shape, distinct from the canonical schema-derived type.

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 (src/node/services/providerModelFactory.ts), and a local OpenAIServiceTier alias in ProvidersSection.tsx. ServiceTierSchema (src/common/config/schemas/providersConfig.ts) already defines the same enum as the runtime source of truth, so this change derives a TypeScript ServiceTier alias via z.infer once and imports it at each site.

The Settings UI keeps its OpenAIServiceTier local alias (used by OpenAIServiceTierSelectValue and the isOpenAIServiceTier type guard) but it is now type OpenAIServiceTier = ServiceTier, so the disambiguating name and call sites stay intact.

Drop unused appendSpace literals on skill/model alias suggestions

In src/browser/utils/slashCommands/suggestions.ts, the SuggestionDefinition literals built for agent skills and model alias one-shots set appendSpace: true, but the build callbacks for those two paths hardcode the trailing space and never read definition.appendSpace. Only the top-level command and subcommand build callbacks consult the field. Remove the no-op appendSpace: true from the skill and model alias mappings and leave a short comment near each so a future reader doesn't assume the field is consulted on these paths.

Behavior is preserved — the field is unread on these paths, and the existing suggestions.test.ts / inlineSkillSuggestions.test.ts / suggestionMatching.test.ts suites all still pass.

Route ThemeContext color-scheme through isLightThemeMode

Replace the inline theme === "light" || theme === "flexoki-light" check in getColorScheme (in src/browser/contexts/ThemeContext.tsx) with the shared isLightThemeMode helper from src/browser/utils/highlighting/shiki-shared.ts.

The helper was just introduced as the single source of truth for the -light suffix → light-theme mapping, and the highlighting call sites (MarkdownComponents, HighlightedCode, highlightDiffChunk) already use it. This change extends that convention to ThemeContext so a future palette like solarized-light would automatically map to colorScheme: "light" without revisiting this site.

Behavior is preserved: for the four valid ThemeMode values ("light", "dark", "flexoki-light", "flexoki-dark"), isLightThemeMode returns the same result as the previous explicit comparison.

Validation

• bun test src/common/utils/ai/modelDisplay.test.ts — 18/18 pass (covers the touched Bedrock branch plus the new DeepSeek cases).
• make static-check — eslint, tsgo (×2), prettier all pass; only shfmt fails because the binary is unavailable in this environment, and no shell files are touched.

Risks

Minimal — purely a dead-code cleanup. The dotParts.length < 2 early return above the touched line guarantees dotParts[1] exists, so removing the ternary's unreachable empty-string fallback cannot change which model IDs parseBedrockModelName accepts or rejects. The change is also symmetrical with the pre-existing unguarded firstPart = dotParts[0].toLowerCase() access on the line directly above.

---

Auto-cleanup checkpoint: 9854990

---

Generated with mux • Model: anthropic:claude-opus-4-7 • Thinking: xhigh

[mux-bot]

⚠️ Auto-fixup could not resolve CI failure: flaky tests, not caused by the cleanup commit.

Failed job: Test / Integration (run 25244368462)

Failures (both in tests/ui/compaction/compaction.test.ts):

1. force compaction triggers during streaming — timed out at 120000 ms waiting for "Compaction boundary" transcript text.
2. /compact with Ctrl+Enter (turn-end) does NOT auto-background foreground bash — waitFor in tests/ui/helpers.ts:144 could not locate the workspace in the sidebar.

What I checked:

• The PR diff is type-only refactors (extract ResolvedWorkspaceAiSettings / ServiceTier aliases, drop unused GuardAnchors / appendSpace literals, route ThemeContext color-scheme through isLightThemeMode). None of it touches compaction, streaming, the workspace consumer manager, or the test harness.
• Ran both failing tests locally on 1312f5a68:
  • force compaction triggers during streaming → PASS in 6.7s.
  • /compact with Ctrl+Enter ... → PASS in 4.7s.
• The streaming-compaction test has a 120s budget and clearly hit a slow/contended runner; the workspace-not-found timeout looks like the usual sidebar race in the harness.

No fix pushed. A re-run of Test / Integration should clear it; if it recurs on this PR or others, the test harness itself likely needs a stability pass (separate from this cleanup PR).

[mux-bot]

⚠️ Auto-fixup: CI failure looks like a flaky integration test, not caused by the cleanup commits.

Failed job: Test / Integration (run 25400653486)

Failing test: tests/ui/compaction/compaction.test.ts › force compaction triggers during streaming — timed out at 120s waiting for "Compaction boundary" to appear in the transcript. The next test in the same file cascaded into "Project not found in sidebar" (broken state from the prior timeout).

Why this looks flaky, not regression-caused:

• None of the files touched by this PR are in the compaction code path. The diff is pure type/dead-code cleanup in ThemeContext, useAutoScroll, StreamingMessageAggregator (stream-error row helper extract — no behavioral change), streamContextBuilder (drop unused isPlanHandoffAgent bool), taskService, file_edit_insert, provider config, slash command suggestions, chatCommands, and cli/run.ts.
• This branch's prior PR-workflow runs (May 1, May 2, May 3) all passed with the same nine refactor commits already on the branch. The only commit added since the last green run is 5f8c84bab refactor: drop dead length guard in parseBedrockModelName secondPart, which is unrelated to compaction.
• tests/ui/compaction/compaction.test.ts has a documented history of stabilization patches (88c99f08c, 5dc32b039, 355e0007c).
• The failure mode is a 120s timeout, typical of CI runner contention rather than a logic regression.

No fix pushed — recommend re-running the failed job (gh run rerun 25400653486 --failed). If it reproduces deterministically, it's worth investigating whether one of today's main-branch merges (DeepSeek V4 support, the perf revert, or the Harbor CI change) destabilized this scenario, since the auto-cleanup branch picked those up via rebase.