The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

The new helper added in #3195 was a 1-line shim around
`getPathnameFromRequestUrl(req.url)` used in only two adjacent functions
(`shouldInjectSlashlessRootRedirect` and `getPublicBaseHref`). The
existing `getPathnameFromRequestUrl` already conveys the intent, and the
two callsites read clearly without the extra layer.

Behavior-preserving: each callsite continues to compute the same pathname
(or null) it did before. All 52 tests in src/node/orpc/server.test.ts
continue to pass.

## 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>