Commit 6ea6964
🤖 refactor: auto-cleanup (#3169)
## Summary
Long-lived auto-cleanup PR that accumulates low-risk,
behavior-preserving refactors picked from recent `main` commits.
## Changes
### Use shared `isAbortError` utility in AuthTokenModal
Replace two inline `error instanceof DOMException && error.name ===
"AbortError"`
checks in `AuthTokenModal.tsx` with the existing shared `isAbortError()`
utility
from `@/browser/utils/isAbortError`, deduplicating the abort detection
logic.
### Extract `extractChunkDeltaText` helper to deduplicate advisor chunk
parsing
Pull the repeated `switch` over `chunk.type` (extracting
`chunk.textDelta` or
`chunk.argsTextDelta`) into a single `extractChunkDeltaText()` helper in
`advisorService.ts`, then call it from both `executeAdvisorStream` and
`executeAdvisorStreamWithRetry`.
### Remove unnecessary exports from `skillFileUtils`
Un-export `parseSkillFile`, `serializeSkillFile`, and `SKILL_FILENAME`
from
`src/node/services/agentSkills/skillFileUtils.ts` — all three are only
used
within the same file, so the `export` keyword was unnecessary.
### Remove dead `getCancelledCompactionKey` storage helper
Remove the `getCancelledCompactionKey` function and its entry in the
`EPHEMERAL_WORKSPACE_KEY_FUNCTIONS` array from `storage.ts` — the only
consumer
(`useResumeManager.ts`) was deleted, leaving this as dead code.
### Remove dead `quickReviewNotes` module
Remove `src/browser/utils/review/quickReviewNotes.ts` and its test file
(482 lines). The `buildQuickLineReviewNote` and
`buildQuickHunkReviewNote`
functions were never imported by any production code since their
introduction
in PR #2448.
### Un-export `isBashOutputTool` in messageUtils
Remove the `export` keyword from `isBashOutputTool` in
`src/browser/utils/messages/messageUtils.ts` — the type guard is only
used within the same file by `computeBashOutputGroupInfos`, so the
export was unnecessary.
### Deduplicate `hasErrorCode` in submoduleSync
Replace inline `NodeJS.ErrnoException`-like error-code checks in
`submoduleSync.ts` with calls to the existing `hasErrorCode` helper,
keeping a single canonical place where error-code narrowing lives.
### Simplify `hasCompletedDescendants` to reuse
`listCompletedDescendantAgentTaskIds`
Rewrite `hasCompletedDescendants` to delegate to the existing
`listCompletedDescendantAgentTaskIds` helper instead of re-implementing
the
traversal, collapsing the two code paths into one.
### Reuse `anthropicSupportsNativeXhigh` in Anthropic fetch wrapper
Replace the duplicated Opus 4.7+ regex inside
`wrapFetchWithAnthropicCacheControl`
(src/node/services/providerModelFactory.ts)
with a call to the existing `anthropicSupportsNativeXhigh` helper from
`src/common/types/thinking.ts`. The helper already performs the same
regex
check plus provider-prefix normalization (e.g.,
`anthropic/claude-opus-4-7` via the `ai-model-id` gateway header),
keeping the
wire-level detection and the policy-level detection in one place.
### Extract `getFetchInputUrl` helper to deduplicate URL extraction
The OpenAI/Codex and Copilot fetch wrappers in `providerModelFactory.ts`
each
contained an identical 15-line IIFE that extracted a URL string from the
`fetch` `input` argument (handling string, `URL`, and `Request`-like
shapes).
Extract the logic into a single `getFetchInputUrl` helper so both
wrappers
share one implementation. Behavior-preserving: the helper returns the
same
empty-string fallback on unrecognized inputs, so callers continue to
fall
through to normal fetch behavior without throwing.
### Extract `clonePersistedToolModelUsage` helper in streamManager
The deep-clone pattern for `PersistedToolModelUsage` (spread event,
fresh
`usage` object, conditional `providerMetadata`) was duplicated between
`recordToolModelUsage` and the stream-end tool-usage snapshot in
`streamManager.ts`. Extract a single file-local helper so both sites
share
the same implementation. Behavior-preserving: both callsites continue to
produce structurally identical clones.
### Reuse `getClosestTranscriptAncestor` in
`getTranscriptContextMenuLink`
The new `getTranscriptContextMenuLink` helper (added in #3188) inlined
the same "resolve event target → `element.closest(selector)` → require
both to stay within the transcript root" pattern that
`getClosestTranscriptAncestor` — defined a few lines above in the same
file — already implements. Delegate to the shared helper so the
null/contains guards live in one place. Behavior-preserving: the
helper returns null for a null/outside-root target, then
`element.closest("a[href]")`, then null again if the anchor is outside
the transcript root — identical to the previous inline checks. All 22
`transcriptContextMenu` tests continue to pass.
### Remove duplicate `gpt-5.5-pro` thinking-policy test
When #3192 renamed `gpt-5.4-pro` → `gpt-5.5-pro` across
`src/common/utils/thinking/policy.test.ts`, it accidentally introduced a
third `returns medium/high/xhigh for gpt-5.5-pro` test that is
byte-identical to the renamed first occurrence (the two remaining tests
are the bare-prefix and `with version suffix` variants; the deleted
block
had no version suffix and no gateway prefix). Drop the duplicate so the
suite has one canonical no-suffix test, one mux-gateway test, and one
version-suffix test. Behavior-preserving — `getThinkingPolicyForModel`
coverage for `gpt-5.5-pro` is unchanged; 63 / 63 tests in
`policy.test.ts`
continue to pass.
### Extract `getAppProxyBasePathFromRequestValue` helper in orpc server
The orpc server's public-base-path detection in
`src/node/orpc/server.ts`
repeated the pattern `parsePathnameFromRequestValue(value) →
getAppProxyBasePathFromPathname(...)` across four callsites (forwarded
headers, the `originalUrl` / `url` loop, the referer header, and the
direct app-proxy handler-prefix calculator). Extract a single
`getAppProxyBasePathFromRequestValue` helper that performs the two-step
normalize-then-classify operation, then call it from every site.
Behavior-preserving: each callsite still produces `null` when the value
is absent or yields an invalid pathname, and otherwise returns the same
parsed app-proxy base path. All 52 tests in
`src/node/orpc/server.test.ts` continue to pass.
### Inline `getRoutePathnameForBaseHref` wrapper in orpc server
The new helper added in #3195 was a one-line shim that simply renamed
`getPathnameFromRequestUrl(req.url)` to fit the surrounding "for base
href" naming theme. It was used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`), and the
existing `getPathnameFromRequestUrl` already conveys the intent at the
callsite. Inline both calls so the request-URL → pathname conversion
lives at the points of use, removing one layer of indirection without
changing behavior. All 52 tests in `src/node/orpc/server.test.ts`
continue to pass.
### Remove dead `AdvisorToolResultSchema` definitions
`AdvisorToolResultSchema` and its three constituent schemas
(`AdvisorToolAdviceResultSchema`, `AdvisorToolLimitResultSchema`,
`AdvisorToolErrorResultSchema`) in
`src/common/utils/tools/toolDefinitions.ts`
were introduced alongside the experimental advisor tool in #3157 but
were never imported anywhere — neither by `src/common/types/tools.ts`
(which derives the public advisor result shape from a different type
local to `AdvisorToolCall.tsx`), nor by the advisor tool implementation
itself, nor by any test. Unlike the analogous `TaskToolResultSchema` /
`TaskAwaitToolResultSchema` / `TaskApplyGitPatchToolResultSchema` /
`TaskTerminateToolResultSchema` (all of which are imported via
`z.infer` in `src/common/types/tools.ts`), the advisor variant had no
consumer. Drop the four dead schemas; the file shrinks by ~32 lines and
keeps `AdvisorToolInputSchema` (which is imported by `advisor.ts`)
intact. Behavior-preserving.
### Reuse `getProviderPolicy()` in custom-provider `getConfig()` loop
`ProviderService.getConfig()`'s custom-provider branch inlined the same
"if enforced, look up `providerAccess` entry → narrow to
`{ forcedBaseUrl, allowedModels }`" lookup that the existing private
`getProviderPolicy()` helper already implements (and that other
callsites such as `addCustomOpenAICompatibleProvider` use). Replace the
inline lookup with a call to `getProviderPolicy(providerId)` so the
small policy-shape projection lives in one place. Behavior-preserving:
the only structural difference is that, when policy is not enforced,
`getProviderPolicy()` returns `{}` while the inline form passed
`{ forcedBaseUrl: undefined, allowedModels: null }`, but
`buildCustomProviderConfigInfo` normalizes both via
`policy?.forcedBaseUrl ?? resolveConfigBaseUrl(...)` and
`policy?.allowedModels ?? null`, so the resulting `ProviderConfigInfo`
is byte-identical. All 74 tests in `providerService.test.ts` continue
to pass.
### Collapse task-group parent rail offset into shared helper
After #3199 introduced `getTaskGroupMemberDepth` and set
`TASK_GROUP_MEMBER_PARENT_RAIL_OFFSET_PX =
SIDEBAR_LEADING_SLOT_CENTER_OFFSET_PX`,
the `task-group-member` branch of `getSubAgentParentRailX` in
`src/browser/components/sidebarItemLayout.ts` reduced to
`getSidebarLeadingSlotCenterX(depth)`. Replace the inline
`getSidebarItemPaddingLeft(depth) +
TASK_GROUP_MEMBER_PARENT_RAIL_OFFSET_PX`
arithmetic with a call to the existing helper and drop the now-redundant
constant, leaving the leading-slot center offset defined exactly once.
Behavior-preserving: `getSubAgentParentRailX` still returns `38` at
`memberDepth = 2.5`, matching the pinned values in
`sidebarItemLayout.test.ts` (and the equivalent
`getSubAgentChildStatusCenterX` result). All 40 tests in
`sidebarItemLayout.test.ts`, `AgentListItem.test.tsx`, and
`ProjectSidebar.test.tsx` continue to pass.
### Remove unnecessary exports from inline-skill utilities
Un-export four interfaces in the new inline-skill helper files added in
#3204 — `InlineSkillSuggestionContext` and
`InlineSkillSuggestionRefreshContext` in
`src/browser/utils/agentSkills/inlineSkillSuggestions.ts`, plus
`InlineSkillCursorMatch` and `InlineSkillResolveOptions` in
`src/browser/utils/agentSkills/inlineSkillReferences.ts`. All four are
only used as parameter types within their defining files: the test
files import the value functions and pass object-literal arguments,
and the consumer call-sites in `ChatInput/index.tsx` only import the
exported functions, never the parameter type names. So the `export`
keyword was unnecessary. Behavior-preserving and type-only — TypeScript
compile passes for both browser and main configs, and the 49 tests in
`inlineSkillSuggestions.test.ts` and `inlineSkillReferences.test.ts`
continue to pass.
> The earlier "sync thinking-policy doc comments with gpt-5.5 regex"
> cleanup was dropped during rebase: #3192 superseded it by retiring
> `gpt-5.4` from those comments entirely, so the comment-only diff
> became redundant.
> The earlier "reuse `hasNonEmptyString` helper for apiKey checks"
> cleanup was dropped during rebase: #3202 restructured
> `resolveProviderCredentials` to delegate to a new
> `resolveApiKeyCandidate` helper (subsuming the inline check) and
> already updated `hasAnyConfiguredProvider` to use `hasNonEmptyString`
> directly, so the cleanup diff no longer applied cleanly and was no
> longer needed.
### Replace stale `system-1` reference in telemetry comment
The `ExperimentOverriddenPayload.experimentId` JSDoc in
`src/common/telemetry/payload.ts` used `'system-1'` as an example
experiment ID, but the System 1 feature was removed wholesale in
#3207 and that experiment ID no longer exists. Swap the example for
a current entry from `EXPERIMENT_IDS` (`'agent-browser'`) so the
JSDoc points readers at a real experiment. Behavior-preserving —
comment-only change.
### Extract `isLightThemeMode` helper for Shiki theme detection
Three callsites independently encoded
`themeMode === "light" || themeMode.endsWith("-light")` to map a
theme-mode string (including namespaced variants like `flexoki-light`)
to the light Shiki theme:
- `highlightDiffChunk.ts` had a private `isLightTheme(theme: ThemeMode)`
helper.
- `HighlightedCode.tsx` and `MarkdownComponents.tsx` had it inline (the
latter with an intermediate `isLight` local).
Promote the predicate to `isLightThemeMode` in
`src/browser/utils/highlighting/shiki-shared.ts` (next to
`SHIKI_DARK_THEME` / `SHIKI_LIGHT_THEME` and `mapToShikiLang`) and route
all three callsites through it. The suffix convention now has a single
source of truth for the light/dark mapping. Behavior-preserving.
### Remove unnecessary exports from `fileRead`
After #3208 removed the file explorer / file viewer flow, the only
external consumers of `src/browser/utils/fileRead.ts` are
`ImmersiveReviewView` (`buildReadFileScript`, `processFileContents`)
and the colocated test (`buildReadFileScript`,
`EXIT_CODE_TOO_LARGE`, `processFileContents`).
Un-export the helpers that are now only used inside the module itself
(`MAX_FILE_SIZE`, `shellEscape`, `base64ToUint8Array`,
`detectImageType`, `detectSvg`, `detectBinary`, `parseReadFileOutput`)
so the module surface accurately reflects its public API.
Behavior-preserving.
Auto-cleanup checkpoint: d1c0109
---
_Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking:
`xhigh`_
<!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=xhigh -->
---------
Co-authored-by: mux-bot[bot] <264182336+mux-bot[bot]@users.noreply.github.com>
Co-authored-by: Mux <noreply@coder.com>
Co-authored-by: mux-bot <mux-bot@coder.com>
Co-authored-by: ammar-agent <ammar+ai@ammar.io>1 parent d1c0109 commit 6ea6964
25 files changed
Lines changed: 152 additions & 685 deletions
File tree
- src
- browser
- components
- AuthTokenModal
- features
- Messages
- utils
- agentSkills
- highlighting
- messages
- review
- common
- constants
- telemetry
- utils
- thinking
- tools
- node
- orpc
- runtime
- services
- tools
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| |||
136 | 137 | | |
137 | 138 | | |
138 | 139 | | |
139 | | - | |
140 | | - | |
| 140 | + | |
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
| |||
218 | 218 | | |
219 | 219 | | |
220 | 220 | | |
221 | | - | |
222 | | - | |
| 221 | + | |
223 | 222 | | |
224 | 223 | | |
225 | 224 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
9 | 8 | | |
10 | 9 | | |
11 | 10 | | |
| |||
28 | 27 | | |
29 | 28 | | |
30 | 29 | | |
31 | | - | |
32 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| |||
163 | 163 | | |
164 | 164 | | |
165 | 165 | | |
166 | | - | |
167 | | - | |
| 166 | + | |
168 | 167 | | |
169 | 168 | | |
170 | 169 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
36 | | - | |
| 36 | + | |
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
| 17 | + | |
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| |||
335 | 335 | | |
336 | 336 | | |
337 | 337 | | |
338 | | - | |
| 338 | + | |
339 | 339 | | |
340 | 340 | | |
341 | 341 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
| 4 | + | |
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
| 27 | + | |
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
37 | | - | |
| 37 | + | |
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| |||
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
71 | | - | |
| 71 | + | |
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
| |||
80 | 80 | | |
81 | 81 | | |
82 | 82 | | |
83 | | - | |
| 83 | + | |
84 | 84 | | |
85 | 85 | | |
86 | 86 | | |
| |||
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
109 | | - | |
| 109 | + | |
110 | 110 | | |
111 | 111 | | |
112 | 112 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
| 2 | + | |
2 | 3 | | |
3 | 4 | | |
4 | 5 | | |
| |||
29 | 30 | | |
30 | 31 | | |
31 | 32 | | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | 33 | | |
38 | 34 | | |
39 | 35 | | |
| |||
72 | 68 | | |
73 | 69 | | |
74 | 70 | | |
75 | | - | |
| 71 | + | |
76 | 72 | | |
77 | 73 | | |
78 | 74 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
10 | 21 | | |
11 | 22 | | |
12 | 23 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
0 commit comments