Merged
Conversation
Remove the System 1 experiment, settings UI, send-option plumbing, internal bash compaction agent/tooling, and generated docs references.\n\n---\n\n_Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `high` • Cost: `$24.72`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=high costs=24.72 -->
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
mux-bot Bot
added a commit
that referenced
this pull request
Apr 30, 2026
The ExperimentOverriddenPayload comment used 'system-1' as an example experiment ID, but the System 1 feature was removed in #3207. Replace it with a current experiment ID ('agent-browser') so the example is not stale.
mux-bot Bot
added a commit
that referenced
this pull request
Apr 30, 2026
The ExperimentOverriddenPayload comment used 'system-1' as an example experiment ID, but the System 1 feature was removed in #3207. Replace it with a current experiment ID ('agent-browser') so the example is not stale.
mux-bot Bot
added a commit
that referenced
this pull request
Apr 30, 2026
The ExperimentOverriddenPayload comment used 'system-1' as an example experiment ID, but the System 1 feature was removed in #3207. Replace it with a current experiment ID ('agent-browser') so the example is not stale.
ammario
pushed a commit
that referenced
this pull request
Apr 30, 2026
## 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the System 1 feature entirely: the experiment flag, Settings UI, send-option/config plumbing, internal bash compaction agent/tooling, generated docs, and feature-specific tests/scripts.
Implementation
Validation
make static-checkafter rebasing ontoorigin/main.Risks
Low-to-moderate risk due to broad deletion across settings, tool registration, and streaming UI. Remaining bash live-output behavior is still covered by targeted WorkspaceStore/live-buffer tests and static checks.
Generated with
mux• Model:openai:gpt-5.5• Thinking:high• Cost:$24.72