🤖 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
|
To use Codex here, create a Codex account and connect to github. |
|
🤖 Auto-fixup diagnosis: flaky test, no code fix pushed. CI run 29448420645 failed only in Test / Integration with a single failing test: Why this is flaky and not caused by the cleanup commit:
No code changes are safe or warranted here. Recommend re-running the failed job; this should pass on retry. Generated with |
cbefdbe to
0fbce7b
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
0fbce7b to
3242eef
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.
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. |
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
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.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.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.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.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.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.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.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.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.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.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.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.)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.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.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.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.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.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.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.This run
Considered
origin/mainfrom the previous checkpoint0890eb050throughbb8c97542(HEAD), covering the one new commit merged since the last run:ci: migrate visual regression testing from Chromatic to Coder Pixel(🤖 ci: migrate visual regression testing from Chromatic to Coder Pixel #3736,bb8c97542)Cleanup 🤖 docs(bash): timeout_secs description—start small, retry backoff; avoid large defaults for UX #19 taken (see above). 🤖 ci: migrate visual regression testing from Chromatic to Coder Pixel #3736 was a CI/Storybook migration (CI/workflow edits are out of scope for cleanups), so this run targets adjacent duplication instead: the two MCP OAuth
startServerFlowhandlers inrouter.tseach inlined the identical redirect-URI resolution; extracted into a sharedresolveMcpOauthRedirectUrihelper.Checkpoint advanced to
bb8c97542so the commit is marked considered (one cleanup taken this run).Validation
origin/main(bb8c97542); 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 and #17, hoists a repeated side-effect-free property read / pure call 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(...).Auto-cleanup checkpoint: bb8c975
Generated with
mux• Model:anthropic:claude-opus-4-8• Thinking:xhigh• Cost:$0.00