🤖 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
3242eef to
8df1538
Compare
|
@codex review This run adds cleanup #19: deduped the byte-identical MCP OAuth |
|
To use Codex here, create a Codex account and connect to github. |
8df1538 to
9b34f8a
Compare
|
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.
9b34f8a to
554ded1
Compare
|
@codex review Rebased onto latest |
|
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.This run
Considered
origin/mainfrom the previous checkpointa45851c4athroughafaf1aef5(HEAD), covering the two new commits merged since the last run:fix: keep analytics sidebar controllable on mobile(🤖 fix: keep analytics sidebar controllable on mobile #3740,b4cec0a35)refactor: support incremental subagent reports(🤖 refactor: support incremental subagent reports #3714,afaf1aef5)Cleanup 🤖 fix: resolve lint issues and file lease mismatch bug #21 taken (see above). 🤖 refactor: support incremental subagent reports #3714 added
MessageQueue.removeByDedupeKeyPrefix, which re-spread the entry'sdedupeKeysSeton every message iteration; hoisted a single ordered snapshot and routed both index reads through it.Checkpoint advanced to
afaf1aef5so both commits are marked considered (one cleanup taken this run).Validation
origin/main(afaf1aef5); all prior cleanups replayed without conflict and the branch merges cleanly intomain.make static-checkpasses fully — ESLint, both TypeScript configs, Prettier, shell formatting (shfmt), generated-file/doc-sync checks, code-to-docs links,shellcheck, andhadolint.Risks
Very low. Each cleanup is a behavior-preserving simplification that extracts byte-identical inlined logic into a helper (or, for #15, #17, and #21, hoists a repeated side-effect-free property read / pure spread into a local) — no control flow, arguments, thresholds, error types, emitted events, validation, or rendered text change. Cleanup #16 adds a helper in the shared skill-schema module consumed by both discovery paths; both call sites still validate via
AgentSkillDescriptorSchema.safeParse, so descriptor output is unchanged. The sole comment-only cleanup (#12) touches no executable code. Cleanup #19 touches the MCP OAuth redirect-URI resolution (a security-sensitive path) but preserves the exact header-parsing logic, proto defaulting, andMissing Host headererror; its only structural change — moving theasyncstartServerFlowcall out of the origin-branchtry— is inert because the returned promise is passed through unawaited, so thetrynever guarded anything beyond the synchronousnew URL(...). Cleanup #20 replaces six byte-identical five-component token sums with agetTotalTokenshelper whose iteration order andundefined→0handling match the original inline expressions exactly (addition is commutative, so the differing operand order in thecli/run.tssites is preserved), and it deliberately leaves the four-componentcli/debug/costs.tssum alone so no total changes.Auto-cleanup checkpoint: afaf1ae
Generated with
mux• Model:anthropic:claude-opus-4-8• Thinking:xhigh• Cost:$0.00