🤖 refactor: auto-cleanup#3695
Conversation
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Root cause: The Verification: Ran Recommendation: Re-run the failed CI jobs. No code change needed. |
|
@codex review Latest push rebases onto |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review New in this run: cleanup #3 — deduped the identical blockquote line-prefixing ( |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
78cd7b2 to
a676f79
Compare
|
@codex review Added auto-cleanup #5: |
|
To use Codex here, create a Codex account and connect to github. |
a676f79 to
32928e3
Compare
|
To use Codex here, create a Codex account and connect to github. |
32928e3 to
8afce4c
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
8afce4c to
990576b
Compare
|
To use Codex here, create a Codex account and connect to github. |
cd778d7 to
08734b9
Compare
|
@codex review This run adds one behavior-preserving cleanup (#8 in the branch list): dedupes the two identical |
|
To use Codex here, create a Codex account and connect to github. |
08734b9 to
b42facd
Compare
|
@codex review New auto-cleanup change on this run: extracted a private |
|
To use Codex here, create a Codex account and connect to github. |
Extract the byte-identical consolidation/harvest sweep `recordUsage` callbacks in MemoryConsolidationService into a shared `makeSweepUsageRecorder` helper. Behavior-preserving: same sidecar recording and analyticsIngest emit; only the workspaceId source and analyticsSource literal differ per call site. Auto-cleanup checkpoint: f7f0f02
Both fallback branches in prepareToolSearch inlined the identical
{ [TOOL_SEARCH_TOOL_NAME]: _removed, ...rest } destructure to drop the
built-in tool_search entry from the record. Extract a module-level
withoutToolSearch(tools) helper; output is byte-identical.
accumulateProviderMetadata inlined the same verbose (metadata.anthropic as { cacheCreationInputTokens?: number }).cacheCreationInputTokens ?? 0 cast twice. Extracted a module-private getAnthropicCacheCreateTokens(metadata) helper. Behavior-preserving; the displayUsage.ts site is intentionally left alone because its chain has an extra usage.inputTokenDetails.cacheWriteTokens fallback.
Both getThinkingPolicyForModel and hasExplicitThinkingPolicy inlined the identical getExplicitThinkingPolicy(resolveModelForMetadata(model, providersConfig ?? null)) call after #3708 added alias resolution to each. Extract a private getExplicitThinkingPolicyForModel helper; behavior-preserving.
Both eligibility gates in openaiExplicitPromptCachingAvailable inlined the
identical split(":", 2) + `origin !== "openai" || !modelName` check (once for
the request model, once for the resolved capability target). The destructured
origin/name locals were unused past their guard. Extracted into a module-private
isOpenAIOriginModel(canonical) helper. Behavior-preserving.
renameLegacyToolSearchCallPart and renameLegacyToolSearchResultPart were byte-identical except for their part type. Collapse them into a single generic renameLegacyToolSearchPart<T extends { toolName: string }> and drop the now-unused ToolCallPart/ToolResultPart imports. Behavior-preserving.
The CODEX_OAUTH_CONTEXT_WINDOW_OVERRIDES doc comment already explains that these caps are kept separate from model metadata so API-key requests retain the public window. #3724 rewrote the inline comment to restate the same point, so trim the duplicated sentence and keep only the tier-specific rationale. Comment-only; behavior-preserving.
Dedupe the repeated event.errorType member access and the duplicated event.errorType != null guard introduced by #3729 into a single local const. Pure behavior-preserving simplification; ErrorEvent.errorType is a plain Zod-inferred data property with no side effects.
Extract the byte-identical 'reload under lock and bail unless the record is still the exact one we reconciled (same status AND updatedAt)' guard shared by persistRepairedSettledWorkspaceTurn and reviveRetryingWorkspaceTurn (both added in #3738) into a module-level isReconciledWorkspaceTurnUnchanged type guard. The guard narrows current to non-null for the revive path, and the generic 'compare updatedAt too' rationale now lives in one doc comment while each call site keeps its situational note. Behavior-preserving.
Both the archive keyboard-shortcut path and the context-menu item inlined
the byte-identical props.onArchiveAll(...).catch(() => { /* same comment */ })
fire-and-forget block (introduced by #3741). Extracted a local archiveAll
helper so the swallow-and-surface rationale lives in one place.
…descendant queries
…ions The isKimiK3Model docstring already explains that Kimi K3 always reasons and supports only the max reasoning effort, and that the provider-options branches key off the predicate. #3737 restated that same sentence verbatim in both the Moonshot and OpenRouter branches of buildProviderOptions, so trim the duplicated lead sentence and keep only each branch's site-specific "send it explicitly" rationale. Comment-only; behavior-preserving.
3b7b185 to
efb198e
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
Summary
This is the long-lived auto-cleanup PR. Each run, the auto-cleanup agent reviews new commits merged to
main, rebases onto the latestmain, and applies at most one extremely low-risk, behavior-preserving cleanup. The branch accumulates a small stack of independent cleanups until it is merged.Cleanups in this branch
Dedupe memory sweep
recordUsagecallbacks (MemoryConsolidationService). The consolidation sweep and the harvest sweep each inlined the same 15-line callback that routes billed usage to the headless-usage sidecar and emitsanalyticsIngest. Extracted into a privatemakeSweepUsageRecorder(...)helper.Dedupe the "memory scope is full" cap check (
MemoryService). ThecreateandsaveFile(new-file) paths each inlined a byte-identical block that calledstore.listFiles(), compared the count againstMEMORY_MAX_FILES_PER_SCOPE, and threw aMemoryCommandErrorwith the same message. Extracted into a privateassertScopeHasRoom(store, scope)helper.Dedupe blockquote line formatting in the bash monitor wake prompt (
bashMonitorWakeStore.ts).buildBashMonitorWakePromptrendered both the matched-output lines and the lost-monitor script with the identical.map((line) => \> ${line}`).join("\n")blockquote pattern in two places. Extracted into a module-levelblockquoteLines(lines)` helper.Dedupe the
tool_searchremoval inprepareToolSearch(toolCatalog.ts). Both fallback branches (PTC enabled, and empty deferred catalog) inlined the identical{ [TOOL_SEARCH_TOOL_NAME]: _removed, ...rest }destructure to drop the built-intool_searchentry from the tool record. Extracted into a module-levelwithoutToolSearch(tools)helper.Dedupe the Anthropic cache-create token extraction in
accumulateProviderMetadata(usageHelpers.ts). The function inlined the same verbose(metadata.anthropic as { cacheCreationInputTokens?: number } | undefined)?.cacheCreationInputTokens ?? 0cast twice (once for the accumulated metadata, once for the current step). Extracted into a module-privategetAnthropicCacheCreateTokens(metadata)helper.Dedupe capability-model thinking-policy resolution (
thinking/policy.ts). After 🤖 feat: integrate GPT-5.6 Sol/Terra/Luna with native max effort and pro-mode toggle #3708 taught the thinking policy to resolvemappedToModelaliases, bothgetThinkingPolicyForModelandhasExplicitThinkingPolicyinlined the identicalgetExplicitThinkingPolicy(resolveModelForMetadata(modelString, providersConfig ?? null))call. Extracted into a privategetExplicitThinkingPolicyForModel(modelString, providersConfig)helper.Dedupe queue entry clear-callback projection (
messageQueue.ts). After 🤖 feat: queue messages behind special sends instead of erroring (FIFO message queue) #3696 rewroteMessageQueueinto FIFOQueueEntryitems, bothgetClearCallbacksandremoveWorkspaceTurninlined the identical spread that builds aQueueClearCallbacksobject from an entry's optionalonCanceled/onAcceptedPreStreamFailurefields. Extracted into a privateentryClearCallbacks(entry)helper.Dedupe the OpenAI-origin model check in
openaiExplicitPromptCachingAvailable(cacheStrategy.ts). After 🤖 feat: GPT-5.6 explicit prompt cache breakpoints for direct OpenAI #3712 added the GPT-5.6 explicit-prompt-caching eligibility gate, the function inlined the identicalsplit(":", 2)+origin !== "openai" || !modelNamecheck twice — once for the request model and once for the resolved capability target — and the destructuredorigin/modelNamelocals were unused past their guard in both places. Extracted into a module-privateisOpenAIOriginModel(canonical)helper.Dedupe the
tool-call-execution-startemit inStreamManager(streamManager.ts). 🤖 fix: start tool elapsed timers when execute() actually runs #3716 introduced theToolCallExecutionStartEvent, emitted from two places:applyToolExecutionStart(part already stored) and the"tool-call"case that consumes apendingExecutionStartrecorded before the part landed. Both inlined the byte-identicalthis.emit("tool-call-execution-start", { type, workspaceId, messageId, toolCallId, timestamp } satisfies ToolCallExecutionStartEvent)block, differing only in thetoolCallId/timestampsource. Extracted into a privateemitToolCallExecutionStart(workspaceId, streamInfo, toolCallId, timestamp)helper.Dedupe model-parameter extras merge (
aiService.ts). After 🤖 feat: apply mid-turn thinking-level changes at the next model step #3718 added mid-turn thinking-level rebuilds, the initial-model path and the fallback-model path each inlined a byte-identical closure (mergeModelParameterExtras/mergeNextModelParameterExtras) that folds providers.jsoncproviderExtrasUNDER the Mux-built provider-options namespace (short-circuiting when there are no extras, deep-merging viamergeProviderExtrasUnderMuxwhen the namespace is a plain object). The two differed only in the namespace key and the overrides source. Extracted into a module-levelmakeModelParameterExtrasMerger(namespaceKey, providerExtras)factory that returns the merger closure.Unify the legacy
tool_searchpart-rename helper (toolCatalog.ts). 🤖 fix: avoid OpenAI tool search name collision #3719 renamed the built-in tool-search tool totool_catalog_searchand added request-time rewriting of historicaltool_searchcall/result parts. It introduced two byte-identical helpers —renameLegacyToolSearchCallPart(part: ToolCallPart)andrenameLegacyToolSearchResultPart(part: ToolResultPart)— that differ only in the part type; the rename body is identical. Collapsed both into a single genericrenameLegacyToolSearchPart<T extends { toolName: string }>(part: T)and dropped the now-unusedToolCallPart/ToolResultPartimports.Trim duplicated context-cap rationale comment (
codexOAuth.ts). 🤖 fix: cap GPT-5.6 context over Codex OAuth #3724 added the GPT-5.6 family toCODEX_OAUTH_CONTEXT_WINDOW_OVERRIDESand rewrote the map's inline comment with a sentence that restated the rationale already given in the map's doc comment directly above it. Dropped the duplicated rationale sentence, keeping only the tier-specific explanation. Comment-only; no behavior change. (Re-applied on top of [openai] 🤖 fix: use 372K GPT-5.6 OAuth context #3730, which later rewrote the same inline comment and re-introduced the duplicate.)Dedupe flat-section pinned block resolution (
pinnedReorder.ts). 🤖 feat: project-less scratch chats #3723 added a scratch branch tolocatePinnedBlockthat renders scratch chats as one flat "Chats" section, mirroring the existing multi-project branch. Both branches inlined the byte-identicalcollectFlatSectionRows(...).filter(isWorkspacePinned).map((row) => row.id)projection, theif (!pinnedIds.includes(meta.id)) return nullguard, and thereturn { fullOrder: pinnedIds, blockIds: pinnedIds }shape — differing only in theincludeRowpredicate ((row) => row.kind === "scratch"vsisMultiProject). Extracted into a privatelocateFlatSectionPinnedBlock(meta, sortedWorkspacesByProject, includeRow)helper.Dedupe JSON-wrapped tool-output unwrap (
workflowRunMessages.ts). 🤖 fix: stop terminal workflow await loops #3725 addedisTerminalWorkflowRunToolOutput, which re-inlined the byte-identicaloutput.type === "json" && "value" in outputcontainer check already used bystripWorkflowRunRecordForModelto detect the{ type: "json", value }SDK/UI wrapper before recursing on the inner value. Extracted the check into a module-privateisJsonWrappedOutput(output)helper that both functions call, moving the shared rationale into the helper's doc comment. No control flow or return-shape change.Hoist
errorTypelocal infinalizeWorkspaceTurnFromStreamError(taskService.ts). 🤖 fix: keep workspace-turn handles running through auto-retryable stream errors #3729 reworked workspace-turn stream-error settlement, and the reworked function readevent.errorTypethree times and repeated theevent.errorType != nullguard once for theexplicitRecoverycomputation and once in the recoveryif. Hoisted a singleconst errorType = event.errorTypeand routed all uses through it, deduplicating the repeated member access and null guard.ErrorEventis a Zod-inferred plain data type, so the property read has no side effects; pure behavior-preserving simplification with no control-flow change.Extract
buildSkillDescriptorhelper for skill discovery (common/orpc/schemas/agentSkill.ts+agent_skill_list.ts+agentSkillsService.ts). 🤖 feat: skills refresh — invocation control, $ARGUMENTS, dynamic context, .claude compat #3728 (skills refresh) addeduser-invocable/argument-hint/when_to_usefrontmatter and normalized them viaresolveSkillAdvertise/resolveSkillUserInvocable/resolveSkillWhenToUse. Both descriptor-building sites —readSkillDescriptor(theagent_skill_listtool) andreadSkillDescriptorFromDir(agentSkillsService discovery) — then inlined the byte-identical 7-field object literal mappingparsed.frontmatter+scopeinto anAgentSkillDescriptorbeforeAgentSkillDescriptorSchema.safeParse. Extracted the mapping into a sharedbuildSkillDescriptor(frontmatter, scope)inagentSkill.ts(co-located with theresolveSkill*helpers it calls) and dropped the now-unusedresolveSkill*imports at both call sites. Callers still runsafeParsethemselves since they handle validation failure differently. No behavior change.Hoist duplicated
Date.parse(record.createdAt)in the bash monitor delivery gate (workspaceService.ts). 🤖 fix: defer bash monitor wakes during task_await #3732 (defer bash monitor wakes duringtask_await) reworked the delivery gate indrainBashMonitorWakesso a match is re-checked against the shown frontier while pinned to its originating process instance viaDate.parse(record.createdAt). The new non-blockinggetMonitorWakeDeliveryStatebranch and the fallbackgetSettledShownThroughOffsetbranch each inlined the identicalDate.parse(record.createdAt)call as theoriginNotAfterMsargument. Hoisted a singleconst originNotAfterMs = Date.parse(record.createdAt)before the branches (with a clarifying comment on why the origin timestamp pins the check) and routed both calls through it.Date.parseis pure, so the hoist is behavior-preserving.Dedupe the "wait for any in-flight load" block in
DevToolsService(devToolsService.ts). 🤖 fix: clean up devtools.jsonl on archive/remove and reap orphaned session dirs #3733 addedremoveWorkspaceData(archive/remove DevTools cleanup) directly beside the existingclear; both inlined the byte-identicalconst pendingLoad = this.loadingPromises.get(workspaceId); if (pendingLoad) { await pendingLoad; }guard that drains any in-flightloadFromDiskbefore mutating in-memory state so a late load cannot repopulate stale data after the mutation. Extracted into a privateawaitPendingLoad(workspaceId)helper with the shared rationale in its doc comment; both call sites keep their situational one-line comment. No control-flow change.Dedupe MCP OAuth redirect URI resolution (
router.ts). Both the global (mcpOauth.startServerFlow) and per-project (projects.mcpOauth.startServerFlow) handlers inlined the byte-identical block that derives the OAuth callbackredirectUrifrom request headers — preferring theOriginheader (used verbatim when it parses as a URL), then falling back tox-forwarded-host/hostwith the forwarded proto (defaulting tohttp), and returningErr("Missing Host header")when no usable Host header exists. Extracted into a module-levelresolveMcpOauthRedirectUri(headers)helper that returns the resolved URI orundefined; each handler now mapsundefinedto the sameErr.startServerFlowisasync(its returned promise is passed through unawaited), so moving the call out of the origin-branchtrycannot change behavior — thetryonly ever guarded the synchronousnew URL(...)construction. No header semantics or return-shape change.Extract
getTotalTokenshelper for total-token sums (usageAggregator.ts+ 6 call sites). 🤖 feat: show per-model cost breakdown in workspace Costs tab #3739 (per-model cost breakdown in the Costs tab) added a fifth+ copy of the "sum every usage component" expression —input + cached + cacheCreate + output + reasoning.tokens— already inlined byte-for-byte inCostsTab(session model rows),WorkspaceStore(sessiontotalTokens),tokenMeterUtils(calculateTokenMeterDatatotal),sessionUsageService(per-modeltotalTokensaccumulation), and twice incli/run.ts(budgethasTokensgates). Added agetTotalTokens(usage)helper inusageAggregator.ts, co-located with and mirroring the existinggetTotalCost(same five-component iteration;undefined→0), and routed all six sites through it. The four-component sum incli/debug/costs.ts(which omitscacheCreate) was intentionally left untouched to preserve its existing behavior.Hoist the duplicated
dedupeKeyssnapshot inremoveByDedupeKeyPrefix(messageQueue.ts). 🤖 refactor: support incremental subagent reports #3714 (incremental sub-agent reports) addedMessageQueue.removeByDedupeKeyPrefix, which spread the entry'sdedupeKeysSetinto an array once for thematchingKeysprefix filter and then re-spread the sameSetinside theentry.messages.filter(...)callback — once per message iteration — to map each message index back to its dedupe key. TheSetis not mutated until afterkeptMessagesis computed, so both reads observe the same ordered snapshot. Hoisted a singleconst dedupeKeyList = [...entry.dedupeKeys]before the filter and routed both reads through it, eliminating the per-message re-spread. Pure behavior-preserving simplification with no control-flow change.Dedupe the settled workspace-turn reconciliation guard (
taskService.ts). 🤖 fix: report live workspace-turn state from task_await instead of stale settlements #3738 (report live workspace-turn state fromtask_await) added two read-time reconciliation helpers —persistRepairedSettledWorkspaceTurnandreviveRetryingWorkspaceTurn— that each open their settlement-lock body with the byte-identical guard: reload the handle viagetWorkspaceTurnandreturn currentunless it is still the exact record we reconciled against (current != null && current.status === record.status && current.updatedAt === record.updatedAt). Extracted the condition into a module-levelisReconciledWorkspaceTurnUnchanged(current, record)type guard, co-located with 🤖 fix: report live workspace-turn state from task_await instead of stale settlements #3738's ownisSelfHealEligibleSettledWorkspaceTurn, so the generic "compareupdatedAttoo, not just status" rationale lives in one doc comment while each call site keeps its situational note. Thecurrent is WorkspaceTurnTaskHandleRecordreturn type preserves the non-null narrowing thatreviveRetryingWorkspaceTurnrelies on after the guard. No control-flow or return-shape change.Dedupe the fire-and-forget archive-all catch (
TaskGroupListItem.tsx). 🤖 fix: archive all sidebar variants #3741 (archive all sidebar variants) added anonArchiveAllprop invoked from two places — the archive keyboard-shortcut branch inonKeyDownand theArchive all variantscontext-menu item'sonClick. Both inlined the byte-identical fire-and-forgetprops.onArchiveAll(...).catch(() => { /* the sidebar owner surfaces archive failures through its shared error UI */ })block, differing only in optional-call syntax (inert because the prop is defined in both branches). Extracted a localarchiveAll(buttonElement)helper so the swallow-and-surface rationale lives in one place. No control flow, arguments, or error-handling change.Extract
someDescendantAgentTaskWorkspacehelper for sticky-descendant queries (taskService.ts). [tasks] 🤖 feat: support sticky subagents #3744 (sticky subagents) added two adjacent query methods —hasStickyDescendantsandhasUnarchivedStickyDescendants— that each rebuilt the agent-task index the same way (loadConfigOrDefault()→buildAgentTaskIndex(cfg)→listDescendantAgentTaskIdsFromIndex(index, workspaceId).some(...)) and differed only in the.some()predicate. Extracted a privatesomeDescendantAgentTaskWorkspace(workspaceId, predicate)helper that resolves each descendant entry and threads it through the predicate (keeping.some()short-circuiting); the two public methods now just supply their predicate and keep their ownassert. The helper'sdescendant != null && predicate(descendant)guard is equivalent to the priorindex.byId.get(descendantId)?.taskSticky === trueform, so no behavior changes.Drop the duplicated Kimi K3 max-effort rationale (
providerOptions.ts). 🤖 feat: add native Kimi K3 support via a new Moonshot AI provider #3737 (native Kimi K3 via a new Moonshot AI provider) added theisKimiK3Modelpredicate, whose docstring is the authoritative statement that K3 always reasons and supports only the max reasoning effort and that the provider-options branches key off it. Both the Moonshot and OpenRouter branches ofbuildProviderOptionsthen restated that same lead sentence verbatim, so the duplicated sentence was trimmed from each while keeping only the branch-specific "send it explicitly" rationale (Moonshot: don't rely on the API default; OpenRouter:enabled: truealone falls back to the unsupported default medium effort). Comment-only; behavior-preserving.This run
Considered
origin/mainfrom the previous checkpoint4c1fa27cdthroughd6886e93b(HEAD), covering the one new commit merged since the last run:feat: add native Kimi K3 support via a new Moonshot AI provider(🤖 feat: add native Kimi K3 support via a new Moonshot AI provider #3737,d6886e93b)Cleanup 🤖 Improve plan tool description for better quality plans #25 taken (see above). 🤖 feat: add native Kimi K3 support via a new Moonshot AI provider #3737 introduced
isKimiK3Model, whose docstring already carries K3's fixed "max" reasoning-effort rationale; the twobuildProviderOptionsbranches restated that lead sentence verbatim, so it was trimmed from each while their distinct "send it explicitly" notes were preserved.The remainder of 🤖 feat: add native Kimi K3 support via a new Moonshot AI provider #3737 was additive provider wiring (a new
moonshotaidirect provider, config schema,MOONSHOT_API_KEYenv var, icon, registry/metadata entries, and OpenRouter routing) with no byte-identical code duplication that a safe local extraction would remove.Checkpoint advanced to
d6886e93b.Validation
origin/main(d6886e93b); all prior cleanups replayed without conflict and the branch merges cleanly intomain.make static-checkpasses fully — ESLint, both TypeScript configs, Prettier, shell formatting (shfmt), Python formatting (ruff), shellcheck, hadolint, generated-file/doc-sync checks, and code-to-docs link checks.Risks
Very low. Cleanup #25 is comment-only: it trims a duplicated rationale sentence from two comment blocks in
buildProviderOptions, changing no code, control flow, or provider-options payloads. The authoritative rationale still lives in theisKimiK3Modeldocstring.Auto-cleanup checkpoint: d6886e9
Generated with
mux• Model:anthropic:claude-opus-4-8• Thinking:xhigh• Cost:$0.00