Skip to content

feat: 继续进行内存泄露修复及垃圾代码清理#413

Merged
claude-code-best merged 5 commits intomainfrom
performance/20260505/memory-leak-fix
May 5, 2026
Merged

feat: 继续进行内存泄露修复及垃圾代码清理#413
claude-code-best merged 5 commits intomainfrom
performance/20260505/memory-leak-fix

Conversation

@claude-code-best
Copy link
Copy Markdown
Owner

@claude-code-best claude-code-best commented May 5, 2026


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

Summary by CodeRabbit

  • Bug Fixes

    • Fixed unbounded memory growth in long-running sessions by implementing performance buffer management and reducing file history snapshots.
  • Chores

    • Removed deprecated placeholder message components.
    • Removed legacy Claude model version from model selection.
    • Removed Windows clipboard-based text input method.
    • Cleaned up unused imports and code across the codebase.
    • Modernized React component imports.

claude-code-best and others added 5 commits May 5, 2026 19:36
删除未使用的文件(BuiltinStatusLine.tsx、4 个重复的 .ts stub)、
移除约 55 个文件中未使用的 React 导入、
清理约 50 处未使用的导入/变量/参数。
净减少 ~296 行代码,precheck 4077 测试全部通过。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
移除 autonomyStatus/betas/claudemd/executorCrossPlatform 中
未使用的导入和参数。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
移除未使用的导入(getSubscriptionType、isEnvDefinedFalsy、
getClaudeConfigHomeDir 等)、未使用的常量(ACCENT_COLOR、
NAME_MATCH_BONUS、CLIPBOARD_THRESHOLD)和死函数
(getOpus41Option、pasteViaClipboard),
为未使用参数添加 _ 前缀。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@mintlify
Copy link
Copy Markdown

mintlify Bot commented May 5, 2026

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
ccb-863780bf 🟢 Ready View Preview May 5, 2026, 1:13 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

📝 Walkthrough

Walkthrough

This PR performs large-scale codebase cleanup: enforces process.env.NODE_ENV as "production" in build configs, installs a GC-backed performance shim, removes unused React default imports and unused function parameters across ~80+ files, removes deprecated features (ULTRAPLAN overrides, clipboard text input), adds Langfuse span flush capability, and refactors several targeted behaviors to eliminate dead code and technical debt.

Changes

Build-time Configuration & Performance Infrastructure

Layer / File(s) Summary
Build Configuration
build.ts, scripts/dev.ts, vite.config.ts
process.env.NODE_ENV is now hardcoded to "production" during bundling by merging into the define configuration, replacing reliance on the original build environment.
Performance Buffer Management
src/utils/performanceShim.ts (new), src/entrypoints/cli.tsx
New performance shim module wraps globalThis.performance with a GC-able Map-backed implementation of mark, measure, getEntriesByType, getEntriesByName, clearMarks, and clearMeasures, preventing unbounded native buffer growth in long-running sessions. Shim is installed early in CLI startup before React/OTel capture.
Trace Finalization
src/query.ts
query() now calls await flushLangfuse() after endTrace() to force immediate processing of buffered Langfuse spans, and clears globalThis.performance marks/measures to prevent memory leaks.

React & Import Modernization

Layer / File(s) Summary
React Import Conversion
src/buddy/CompanionCard.tsx, src/cli/handlers/mcp.tsx, src/commands/config/config.tsx, src/commands/diff/diff.tsx, src/commands/doctor/doctor.tsx, src/commands/help/help.tsx, src/commands/hooks/hooks.tsx, src/commands/permissions/permissions.tsx, src/commands/review/ultrareviewCommand.tsx, src/commands/stats/stats.tsx, src/commands/usage/usage.tsx, src/hooks/notifs/useCanSwitchToExistingSubscription.tsx, src/hooks/useChromeExtensionNotification.tsx, src/hooks/useOfficialMarketplaceNotification.tsx, src/services/mcpServerApproval.tsx, src/services/remoteManagedSettings/securityCheck.tsx, src/utils/teleport.tsx, and others
Removes unused default React imports (import React from 'react' / import * as React) across ~25+ files, relying on automatic JSX transform.
Named React Imports
src/commands/assistant/assistant.tsx, src/commands/install-github-app/*, src/hooks/notifs/*, src/hooks/useArrowKeyHistory.tsx, src/hooks/useTypeahead.tsx, src/context/promptOverlayContext.tsx, src/components/LogoV2/CondensedLogo.tsx, src/components/PromptInput/PromptInputFooterSuggestions.tsx, src/components/agents/new-agent-creation/wizard-steps/*, and others
Converts default React imports to named-only imports (import { useEffect, useState } from 'react') or type-only ReactNode imports across ~60+ files to eliminate redundant runtime dependency.
Unused Parameter & Import Cleanup
src/cli/print.ts, src/commands/provider.ts, src/services/compact/cachedMicrocompact.ts, src/services/compact/snipCompact.ts, src/services/skillSearch/prefetch.ts, src/services/tools/toolHooks.ts, src/utils/toolPool.ts, src/screens/REPL.tsx, src/components/tasks/WorkflowDetailDialog.tsx, src/components/teams/TeamsDialog.tsx, and others
Renames unused function parameters with leading underscore (e.g., state_state, mode_mode, onDone_onDone) to satisfy linting and clarify intent across ~40+ locations.
Unused Type & Constant Removal
src/QueryEngine.ts, src/assistant/gate.ts, src/bridge/*, src/commands/login/login.tsx, src/types/message.ts, src/types/permissions.ts, src/utils/settings/types.ts, and others
Removes unused imports (CompactMetadata, getKairosActive, SDKResultSuccess, feature from bun:bundle, NormalizedAssistantMessage, etc.) and constants (NAME_MATCH_BONUS, FINALIZE_SAFETY_TIMEOUT_MS) throughout the codebase.

Feature & Logic Refinements

Layer / File(s) Summary
Deprecated Feature Removal
src/commands/ultraplan.tsx
Removes dev-only ULTRAPLAN prompt file override logic (ULTRAPLAN_PROMPT_FILE env read), deprecated ULTRAPLAN_TIMEOUT_MS constant, and getUltraplanModel() helper, simplifying prompt sourcing to rely on getPromptText() and feature-gated timeout.
Langfuse Trace Flushing
src/services/langfuse/client.ts, src/services/langfuse/index.ts
Adds new flushLangfuse() async function that forces span processor flush when processor is set, ensuring Langfuse payloads are processed immediately rather than batched.
Auto-Updater Integration Removal
src/components/PromptInput/Notifications.tsx
Removes AutoUpdaterWrapper integration and auto-updater-related props (shouldShowAutoUpdater, autoUpdaterResult, isAutoUpdating, onAutoUpdaterResult, onChangeIsUpdating) from notification logic, simplifying component surface.
Component Stub Removal
src/components/messages/SnipBoundaryMessage.ts, src/components/messages/UserCrossSessionMessage.ts, src/components/messages/UserForkBoilerplateMessage.ts, src/components/messages/UserGitHubWebhookMessage.ts
Deletes auto-generated placeholder component stubs that returned null, cleaning up unused message type entries.
Skill Learning & Computer Use Simplifications
src/commands/skill-learning/skill-learning.ts, src/utils/computerUse/win32/windowMessage.ts, src/utils/computerUse/gates.ts
Skill import command now reads/filters JSON directly instead of using importInstincts() helper; Windows text input removes clipboard-based fallback (CLIPBOARD_THRESHOLD, pasteViaClipboard); subscription-tier gating is bypassed unconditionally.
Memory & Retention Tuning
src/utils/fileHistory.ts
Reduces file snapshot retention cap from 100 to 20 to limit unbounded memory growth from file checkpointing.
Console OAuth & Permission Wiring
src/components/ConsoleOAuthFlow.tsx, src/components/Settings/Config.tsx
Removes unused imports (fi locale, plural); updates permission mode imports to drop EXTERNAL_PERMISSION_MODES.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

This PR combines large-scale repetitive import cleanup (which is mechanically straightforward) with multiple independent logic changes, new infrastructure (performance shim), removed features, and wiring adjustments spread across many files. While individual hunks are often simple, the variety of change types (imports, parameters, deletions, new module, logic tweaks) and heterogeneity across ~120+ files demand careful per-domain review to catch any missed dependencies or breaking implications.

Possibly related PRs

  • claude-code-best/claude-code#412: Modifies src/query.ts trace finalization and message cleanup logic, directly adjacent to the Langfuse flush and performance-clear changes in this PR.
  • claude-code-best/claude-code#308: Extends Langfuse tracing surface and span export behavior, overlapping with the new flushLangfuse() addition.
  • claude-code-best/claude-code#243: Modifies build tooling configuration (build.ts, vite.config.ts) to inject compile-time defines, sharing the same files and pattern as this PR's NODE_ENV hardcoding.

Suggested labels

v6, cleanup, refactor


🐰 A rabbit hops through React imports clean,
Unused parameters culled from the scene,
Performance buffers now won't runaway,
Langfuse flushes its spans right away,
Clipboards dismissed, old stubs swept away—
The code's much leaner than yesterday! 🌟

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch performance/20260505/memory-leak-fix

@codecov-commenter
Copy link
Copy Markdown

@blacksmith-sh
Copy link
Copy Markdown

blacksmith-sh Bot commented May 5, 2026

Autofix enabled on this PR. I'll automatically:

  • Fix CI failures
  • Address review comments from bots
  • Resolve merge conflicts

To disable, uncheck the autofix checkbox in the PR description or comment @codesmith !autofix off.

View in Codesmith

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/utils/toolPool.ts (1)

52-59: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align JSDoc param name with the renamed argument.

@param mode no longer matches _mode, which can confuse API readers and tooling.

Suggested diff
- * `@param` mode - The permission context mode.
+ * `@param` _mode - The permission context mode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/toolPool.ts` around lines 52 - 59, Update the JSDoc for
mergeAndFilterTools to match the renamed parameter: change the `@param` entry from
"mode" to "_mode" (or rename the function parameter to "mode" if you prefer
consistency) so the docstring and the function signature align; ensure the
description still references ToolPermissionContext['mode'] and that the `@param`
name matches the actual parameter (_mode) used in the mergeAndFilterTools
function.
🧹 Nitpick comments (7)
src/hooks/useSSHSession.ts (1)

24-24: ⚡ Quick win

Use src/* alias for this TypeScript import

Line 24 still uses a relative import path. Please switch this changed import to the src/* alias style to match repo conventions.

As per coding guidelines: "Use src/* path alias in imports; do not use relative paths like ../../../."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useSSHSession.ts` at line 24, The import for SSHSessionManager in
useSSHSession.ts uses a relative path; update the import to use the project's
src path alias (e.g., import type { SSHSessionManager } from
'src/ssh/SSHSessionManager') so it matches repo conventions and other imports;
locate the import statement referencing SSHSessionManager in the
useSSHSession.ts file and replace the relative path with the src/* alias.
src/utils/betas.ts (1)

26-26: ⚡ Quick win

Use src/* alias for this import instead of a relative path

Please switch this changed import to the repository alias style to match TS import conventions.

Suggested diff
-import { isEnvTruthy } from './envUtils.js'
+import { isEnvTruthy } from 'src/utils/envUtils.js'

As per coding guidelines, "Use src/* path alias in imports; do not use relative paths like ../../../."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/betas.ts` at line 26, Update the import in src/utils/betas.ts to
use the repository path alias instead of a relative path: replace the current
relative import of isEnvTruthy with the aliased module import (e.g., import {
isEnvTruthy } from 'src/utils/envUtils.js') so the symbol isEnvTruthy is
imported via the src/* path alias.
src/components/PromptInput/Notifications.tsx (1)

42-50: 🏗️ Heavy lift

Remove obsolete auto-updater props from Props and component API surface

The underscore rename avoids local unused warnings, but these fields are still required by Props, so callers must keep passing dead data/handlers. Since auto-updater UI plumbing is removed, please delete these props from Props and the parent callsites to finish the cleanup.

Also applies to: 57-66

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/PromptInput/Notifications.tsx` around lines 42 - 50, Remove
the obsolete auto-updater API from the Notifications component: delete the
autoUpdaterResult: AutoUpdaterResult | null, isAutoUpdating: boolean,
onAutoUpdaterResult: (result: AutoUpdaterResult) => void, and
onChangeIsUpdating: (isUpdating: boolean) => void fields from the Props type and
remove those same props from the Notifications component parameter list (and any
underscore-prefixed variants). Also remove any remaining references to
AutoUpdaterResult type and any internal use of those props (handlers or flags)
inside Notifications.tsx, and update all parent callsites to stop passing these
props. Ensure imports for AutoUpdaterResult are removed if no longer used.
src/components/Spinner.tsx (1)

245-247: ⚡ Quick win

Comment/API contract is out of sync around TTFT.

Line 245 says apiMetricsRef was removed, but Props still declares it (Line 51-59). Please either remove the stale prop or update the comment and wire it intentionally; keeping both states creates ambiguous component API intent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Spinner.tsx` around lines 245 - 247, Props currently declares
apiMetricsRef but the implementation comments and local variable _ttftText
indicate apiMetricsRef was removed and TTFT support is disabled; pick one:
either remove apiMetricsRef from the Props type (and any related
tests/consumers) and update the comment to remove TTFT gating wording, or
re-thread the prop by restoring usage in the Spinner component (use the
apiMetricsRef to compute/set _ttftText and any metric updates) and update the
comment to reflect active TTFT wiring; specifically edit the Props interface to
remove or keep apiMetricsRef, and update Spinner (the _ttftText initialization
and surrounding comment) to match that choice so the component API and
implementation are consistent.
src/query.ts (1)

71-74: ⚡ Quick win

Dead code: _jobClassifier is loaded for no reason.

The underscore prefix marks the binding as unused, yet require('./jobs/classifier.js') still executes at module-eval time and pulls the classifier module (and its transitive imports) into the bundle on every cold start. If jobs/classifier.js has no module-load side effects you intentionally rely on, drop the entire ternary. Otherwise, leave a comment so a future cleanup pass doesn't repeat this triage.

♻️ Option 1: remove if no longer needed
-const _jobClassifier = feature('TEMPLATES')
-  ? (require('./jobs/classifier.js') as typeof import('./jobs/classifier.js'))
-  : null
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/query.ts` around lines 71 - 74, The binding _jobClassifier is unused but
still causes require('./jobs/classifier.js') to execute at module-eval, pulling
that module into the bundle; either remove the entire ternary and the
_jobClassifier declaration if you don't need the classifier, or make the load
lazy/conditional at use-time (e.g., call require('./jobs/classifier.js') inside
the function that needs it) and keep feature('TEMPLATES') check there; if you
intentionally rely on module-load side effects, leave a brief comment above the
current code mentioning that rationale so future cleanup doesn't remove it.
src/utils/performanceShim.ts (2)

44-66: ⚡ Quick win

measure() returns void but the spec (and TS types) require PerformanceMeasure.

The cast measure as typeof performance.measure (line 130) hides this — any caller that reads the return value will silently get undefined. Native performance.measure(name, ...) has returned a PerformanceMeasure since 2019.

Either return a minimal entry object (mirroring how mark() already does) or, if you're confident no consumer reads it, drop the cast and accept that this shim is intentionally narrower than the spec.

♻️ Return a minimal PerformanceMeasure-shaped object
 function measure(
   name: string,
   startMarkOrOptions?: string | MeasureOptions,
   endMark?: string,
-): void {
+): PerformanceMeasure {
   let startTime: number
   let duration: number
   // ... existing logic ...
   measures.set(name, { name, startTime, duration })
+  return {
+    name,
+    entryType: 'measure',
+    startTime,
+    duration,
+    detail: null,
+    toJSON: () => ({ name, entryType: 'measure', startTime, duration }),
+  } as PerformanceMeasure
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/performanceShim.ts` around lines 44 - 66, The shimbed measure
function currently returns void which breaks callers expecting a
PerformanceMeasure; update the measure function to return a minimal
PerformanceMeasure-shaped object (mirror the mark() behavior) and change its
signature from void to PerformanceMeasure (or the appropriate TS type), ensure
the measures Map stores that object (include name, entryType: 'measure',
startTime, duration) and remove or stop casting `measure` to `typeof
performance.measure` so callers receive the proper value; reference the existing
measure function and the cast `measure as typeof performance.measure` when
locating the changes.

32-42: ⚖️ Poor tradeoff

Behavioral divergence: mark/measure overwrite same-name entries; native preserves history.

marks.set(name, …) and measures.set(name, …) use a Map keyed by name, so the second call to mark('x') discards the first. The native API stores them in a list and getEntriesByName('x', 'mark') returns all of them in order. Anything that uses repeated marks for timeline traces (React Compiler/Profiler, OTel timing export, custom devtools) will silently lose data after this shim is installed.

If keeping memory bounded is the goal, consider a small ring buffer per-name (or capped array) instead of a single-slot map; a single set semantics is a stricter break than the file's intent suggests.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/performanceShim.ts` around lines 32 - 42, The current mark/measure
implementation overwrites same-name entries because marks and measures are
stored as single values in the maps; change this to store an array (or
capped/ring-buffer) per name so repeated calls append entries and preserve
insertion order; update the mark function to push a new timestamp into
marks.get(name) array (creating it if missing) and return a PerformanceMark-like
object with that timestamp, update measure to compute start/end by looking up
the appropriate entries in the marks arrays (and store measures as arrays in
measures map), and adjust getEntriesByName/getEntries to iterate these per-name
arrays in order so consumers like mark, measure, getEntriesByName('x','mark')
receive all historical entries rather than only the last one.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/dev.ts`:
- Around line 17-22: The current defines object hardcodes 'process.env.NODE_ENV'
to 'production' which disables dev-only branches; change this so the production
value is applied only when a new opt-in env var (e.g.
CLAUDE_CODE_FORCE_NODE_ENV) is set: keep the defines and getMacroDefines() usage
but set 'process.env.NODE_ENV' to
JSON.stringify(process.env.CLAUDE_CODE_FORCE_NODE_ENV || process.env.NODE_ENV ||
'development') (or equivalent) so developers default to dev mode while allowing
opt-in production mode for the React memory optimization when
CLAUDE_CODE_FORCE_NODE_ENV=production is provided; update any related comments
to reflect the opt-in behavior and reference the defines symbol and
getMacroDefines().

In `@src/screens/REPL.tsx`:
- Line 4940: Remove the unnecessary cast in the selector: change the useAppState
call that assigns _pipeIpcState to call getPipeIpc with the selector argument
directly (useAppState(s => getPipeIpc(s))) instead of casting s to any; update
the line referencing _pipeIpcState in REPL.tsx to remove "as any" so it matches
other usages like useMasterMonitor and useSlaveNotifications and relies on
getPipeIpc’s accepted any-typed parameter.

In `@src/services/api/gemini/index.ts`:
- Line 9: Replace the deep-relative import of the Tools type in
src/services/api/gemini/index.ts with the project path-alias form: change the
import that references '../../../Tool.js' to use the 'src/...' alias (import {
type Tools } from 'src/...') so the file imports Tools via the src path alias
instead of a '../../../' relative path; keep the imported symbol name (Tools)
unchanged.

In `@src/utils/fileHistory.ts`:
- Around line 54-56: The comment above MAX_SNAPSHOTS is misleading because
snapshots are throttled, not disabled; update the comment to accurately state
that file checkpointing remains enabled but capped to prevent unbounded growth
and refer to the gating via fileHistoryEnabled(). Locate the MAX_SNAPSHOTS
constant and replace the "Disabled" wording with a note like "Throttled: file
checkpointing remains enabled but capped at MAX_SNAPSHOTS (see
fileHistoryEnabled() gating) and re-enable full backups only after switching to
incremental diffs," ensuring the comment mentions the reason (heap snapshot
growth) and the intended future change.

In `@src/utils/performanceShim.ts`:
- Around line 142-160: The code uses unsafe `as any` casts in the performance
shim; replace them with safer double-assertions or proper typings: cast
`original` to the correct Performance-like interface using `as unknown as {
onresourcetimingbufferfull?: EventListener | null; toJSON: () => any; [k:
string]: any }` (or extend the Performance interface) so the getter
`onresourcetimingbufferfull` and the `toJSON()` call use typed access instead of
`(original as any)`, change the setter parameter `_v: any` to a typed parameter
(e.g. `_v: EventListener | null`) or `_v: unknown` then cast only if needed, and
replace `(globalThis as any).__performanceShimInstalled` with `(globalThis as
unknown as { __performanceShimInstalled?: boolean }).__performanceShimInstalled`
(or add an ambient declaration), updating references in `get
onresourcetimingbufferfull`, `set onresourcetimingbufferfull`, `toJSON`, and
`installPerformanceShim` accordingly.
- Around line 81-106: Add the missing no-argument overload and other missing
members to the shim so it fully matches typeof performance: implement
performance.getEntries() to return the union of getEntriesByType('mark') and
getEntriesByType('measure') (reuse getEntriesByType/getEntriesByName logic), and
add stubbed/pass-through implementations for markResourceTiming,
setResourceTimingBufferSize (already stub-typed but ensure the function exists),
eventCounts, and the EventTarget-like methods
addEventListener/removeEventListener/dispatchEvent on the shim object; ensure
their signatures match the standard Performance/EventTarget surface so the final
"as typeof performance" cast is accurate and consumers calling
performance.getEntries() or the other APIs won't throw.

---

Outside diff comments:
In `@src/utils/toolPool.ts`:
- Around line 52-59: Update the JSDoc for mergeAndFilterTools to match the
renamed parameter: change the `@param` entry from "mode" to "_mode" (or rename the
function parameter to "mode" if you prefer consistency) so the docstring and the
function signature align; ensure the description still references
ToolPermissionContext['mode'] and that the `@param` name matches the actual
parameter (_mode) used in the mergeAndFilterTools function.

---

Nitpick comments:
In `@src/components/PromptInput/Notifications.tsx`:
- Around line 42-50: Remove the obsolete auto-updater API from the Notifications
component: delete the autoUpdaterResult: AutoUpdaterResult | null,
isAutoUpdating: boolean, onAutoUpdaterResult: (result: AutoUpdaterResult) =>
void, and onChangeIsUpdating: (isUpdating: boolean) => void fields from the
Props type and remove those same props from the Notifications component
parameter list (and any underscore-prefixed variants). Also remove any remaining
references to AutoUpdaterResult type and any internal use of those props
(handlers or flags) inside Notifications.tsx, and update all parent callsites to
stop passing these props. Ensure imports for AutoUpdaterResult are removed if no
longer used.

In `@src/components/Spinner.tsx`:
- Around line 245-247: Props currently declares apiMetricsRef but the
implementation comments and local variable _ttftText indicate apiMetricsRef was
removed and TTFT support is disabled; pick one: either remove apiMetricsRef from
the Props type (and any related tests/consumers) and update the comment to
remove TTFT gating wording, or re-thread the prop by restoring usage in the
Spinner component (use the apiMetricsRef to compute/set _ttftText and any metric
updates) and update the comment to reflect active TTFT wiring; specifically edit
the Props interface to remove or keep apiMetricsRef, and update Spinner (the
_ttftText initialization and surrounding comment) to match that choice so the
component API and implementation are consistent.

In `@src/hooks/useSSHSession.ts`:
- Line 24: The import for SSHSessionManager in useSSHSession.ts uses a relative
path; update the import to use the project's src path alias (e.g., import type {
SSHSessionManager } from 'src/ssh/SSHSessionManager') so it matches repo
conventions and other imports; locate the import statement referencing
SSHSessionManager in the useSSHSession.ts file and replace the relative path
with the src/* alias.

In `@src/query.ts`:
- Around line 71-74: The binding _jobClassifier is unused but still causes
require('./jobs/classifier.js') to execute at module-eval, pulling that module
into the bundle; either remove the entire ternary and the _jobClassifier
declaration if you don't need the classifier, or make the load lazy/conditional
at use-time (e.g., call require('./jobs/classifier.js') inside the function that
needs it) and keep feature('TEMPLATES') check there; if you intentionally rely
on module-load side effects, leave a brief comment above the current code
mentioning that rationale so future cleanup doesn't remove it.

In `@src/utils/betas.ts`:
- Line 26: Update the import in src/utils/betas.ts to use the repository path
alias instead of a relative path: replace the current relative import of
isEnvTruthy with the aliased module import (e.g., import { isEnvTruthy } from
'src/utils/envUtils.js') so the symbol isEnvTruthy is imported via the src/*
path alias.

In `@src/utils/performanceShim.ts`:
- Around line 44-66: The shimbed measure function currently returns void which
breaks callers expecting a PerformanceMeasure; update the measure function to
return a minimal PerformanceMeasure-shaped object (mirror the mark() behavior)
and change its signature from void to PerformanceMeasure (or the appropriate TS
type), ensure the measures Map stores that object (include name, entryType:
'measure', startTime, duration) and remove or stop casting `measure` to `typeof
performance.measure` so callers receive the proper value; reference the existing
measure function and the cast `measure as typeof performance.measure` when
locating the changes.
- Around line 32-42: The current mark/measure implementation overwrites
same-name entries because marks and measures are stored as single values in the
maps; change this to store an array (or capped/ring-buffer) per name so repeated
calls append entries and preserve insertion order; update the mark function to
push a new timestamp into marks.get(name) array (creating it if missing) and
return a PerformanceMark-like object with that timestamp, update measure to
compute start/end by looking up the appropriate entries in the marks arrays (and
store measures as arrays in measures map), and adjust
getEntriesByName/getEntries to iterate these per-name arrays in order so
consumers like mark, measure, getEntriesByName('x','mark') receive all
historical entries rather than only the last one.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e2d88263-3684-4765-8dee-7a5019fa5477

📥 Commits

Reviewing files that changed from the base of the PR and between 100e9d2 and 3f1c846.

📒 Files selected for processing (142)
  • build.ts
  • scripts/dev.ts
  • src/QueryEngine.ts
  • src/assistant/gate.ts
  • src/bridge/bridgeMain.ts
  • src/bridge/remoteBridgeCore.ts
  • src/bridge/replBridge.ts
  • src/buddy/CompanionCard.tsx
  • src/cli/handlers/mcp.tsx
  • src/cli/print.ts
  • src/commands/assistant/assistant.tsx
  • src/commands/config/config.tsx
  • src/commands/diff/diff.tsx
  • src/commands/doctor/doctor.tsx
  • src/commands/help/help.tsx
  • src/commands/hooks/hooks.tsx
  • src/commands/install-github-app/ApiKeyStep.tsx
  • src/commands/install-github-app/CheckExistingSecretStep.tsx
  • src/commands/install-github-app/CheckGitHubStep.tsx
  • src/commands/install-github-app/ChooseRepoStep.tsx
  • src/commands/install-github-app/CreatingStep.tsx
  • src/commands/install-github-app/ErrorStep.tsx
  • src/commands/install-github-app/ExistingWorkflowStep.tsx
  • src/commands/install-github-app/InstallAppStep.tsx
  • src/commands/install-github-app/WarningsStep.tsx
  • src/commands/login/login.tsx
  • src/commands/permissions/permissions.tsx
  • src/commands/provider.ts
  • src/commands/remoteControlServer/remoteControlServer.tsx
  • src/commands/review/ultrareviewCommand.tsx
  • src/commands/skill-learning/skill-learning.ts
  • src/commands/stats/stats.tsx
  • src/commands/ultraplan.tsx
  • src/commands/usage/usage.tsx
  • src/components/BuiltinStatusLine.tsx
  • src/components/ConsoleOAuthFlow.tsx
  • src/components/FileEditToolUpdatedMessage.tsx
  • src/components/LogoV2/CondensedLogo.tsx
  • src/components/LogoV2/LogoV2.tsx
  • src/components/LogoV2/feedConfigs.tsx
  • src/components/Onboarding.tsx
  • src/components/PromptInput/Notifications.tsx
  • src/components/PromptInput/PromptInput.tsx
  • src/components/PromptInput/PromptInputFooter.tsx
  • src/components/PromptInput/PromptInputFooterSuggestions.tsx
  • src/components/PromptInput/SandboxPromptFooterHint.tsx
  • src/components/SandboxViolationExpandedView.tsx
  • src/components/Settings/Config.tsx
  • src/components/Spinner.tsx
  • src/components/agents/new-agent-creation/CreateAgentWizard.tsx
  • src/components/agents/new-agent-creation/wizard-steps/ColorStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/ConfirmStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/ConfirmStepWrapper.tsx
  • src/components/agents/new-agent-creation/wizard-steps/DescriptionStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/GenerateStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/LocationStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/MemoryStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/MethodStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/ModelStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/PromptStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/ToolsStep.tsx
  • src/components/agents/new-agent-creation/wizard-steps/TypeStep.tsx
  • src/components/messages/SnipBoundaryMessage.ts
  • src/components/messages/UserCrossSessionMessage.ts
  • src/components/messages/UserForkBoilerplateMessage.ts
  • src/components/messages/UserGitHubWebhookMessage.ts
  • src/components/permissions/FilePermissionDialog/permissionOptions.tsx
  • src/components/permissions/shellPermissionHelpers.tsx
  • src/components/tasks/ShellProgress.tsx
  • src/components/tasks/WorkflowDetailDialog.tsx
  • src/components/teams/TeamsDialog.tsx
  • src/components/wizard/WizardDialogLayout.tsx
  • src/components/wizard/WizardNavigationFooter.tsx
  • src/components/wizard/WizardProvider.tsx
  • src/context/promptOverlayContext.tsx
  • src/daemon/main.ts
  • src/entrypoints/cli.tsx
  • src/hooks/notifs/useCanSwitchToExistingSubscription.tsx
  • src/hooks/notifs/useIDEStatusIndicator.tsx
  • src/hooks/notifs/useMcpConnectivityStatus.tsx
  • src/hooks/notifs/usePluginAutoupdateNotification.tsx
  • src/hooks/notifs/usePluginInstallationStatus.tsx
  • src/hooks/notifs/useRateLimitWarningNotification.tsx
  • src/hooks/useArrowKeyHistory.tsx
  • src/hooks/useChromeExtensionNotification.tsx
  • src/hooks/useMergedTools.ts
  • src/hooks/useOfficialMarketplaceNotification.tsx
  • src/hooks/useSSHSession.ts
  • src/hooks/useTypeahead.tsx
  • src/interactiveHelpers.tsx
  • src/main.tsx
  • src/query.ts
  • src/screens/REPL.tsx
  • src/services/acp/permissions.ts
  • src/services/api/claude.ts
  • src/services/api/gemini/index.ts
  • src/services/api/openai/client.ts
  • src/services/api/openai/index.ts
  • src/services/compact/cachedMicrocompact.ts
  • src/services/compact/snipCompact.ts
  • src/services/doubaoSTT.ts
  • src/services/extractMemories/extractMemories.ts
  • src/services/langfuse/client.ts
  • src/services/langfuse/index.ts
  • src/services/langfuse/sanitize.ts
  • src/services/mcp/channelNotification.ts
  • src/services/mcp/client.ts
  • src/services/mcpServerApproval.tsx
  • src/services/remoteManagedSettings/securityCheck.tsx
  • src/services/skillLearning/observationStore.ts
  • src/services/skillLearning/toolEventObserver.ts
  • src/services/skillSearch/localSearch.ts
  • src/services/skillSearch/prefetch.ts
  • src/services/tools/toolHooks.ts
  • src/types/hooks.ts
  • src/types/message.ts
  • src/types/permissions.ts
  • src/utils/attachments.ts
  • src/utils/autonomyStatus.ts
  • src/utils/betas.ts
  • src/utils/claudemd.ts
  • src/utils/computerUse/executorCrossPlatform.ts
  • src/utils/computerUse/gates.ts
  • src/utils/computerUse/win32/inputIndicator.ts
  • src/utils/computerUse/win32/windowMessage.ts
  • src/utils/fileHistory.ts
  • src/utils/lanBeacon.ts
  • src/utils/messages.ts
  • src/utils/model/modelOptions.ts
  • src/utils/performanceShim.ts
  • src/utils/permissions/PermissionMode.ts
  • src/utils/permissions/bypassPermissionsKillswitch.ts
  • src/utils/permissions/getNextPermissionMode.ts
  • src/utils/permissions/permissionSetup.ts
  • src/utils/pipeRegistry.ts
  • src/utils/promptEditor.ts
  • src/utils/queryHelpers.ts
  • src/utils/sentry.ts
  • src/utils/settings/types.ts
  • src/utils/teleport.tsx
  • src/utils/toolPool.ts
  • vite.config.ts
💤 Files with no reviewable changes (73)
  • src/commands/login/login.tsx
  • src/commands/install-github-app/InstallAppStep.tsx
  • src/commands/install-github-app/CreatingStep.tsx
  • src/commands/hooks/hooks.tsx
  • src/hooks/notifs/useCanSwitchToExistingSubscription.tsx
  • src/hooks/useOfficialMarketplaceNotification.tsx
  • src/services/mcpServerApproval.tsx
  • src/utils/queryHelpers.ts
  • src/commands/help/help.tsx
  • src/hooks/notifs/usePluginInstallationStatus.tsx
  • src/utils/permissions/getNextPermissionMode.ts
  • src/components/tasks/ShellProgress.tsx
  • src/commands/install-github-app/CheckGitHubStep.tsx
  • src/commands/install-github-app/ExistingWorkflowStep.tsx
  • src/commands/install-github-app/WarningsStep.tsx
  • src/commands/doctor/doctor.tsx
  • src/commands/install-github-app/ErrorStep.tsx
  • src/services/remoteManagedSettings/securityCheck.tsx
  • src/services/doubaoSTT.ts
  • src/commands/stats/stats.tsx
  • src/hooks/notifs/useMcpConnectivityStatus.tsx
  • src/commands/config/config.tsx
  • src/daemon/main.ts
  • src/assistant/gate.ts
  • src/types/hooks.ts
  • src/hooks/notifs/useRateLimitWarningNotification.tsx
  • src/services/extractMemories/extractMemories.ts
  • src/components/messages/SnipBoundaryMessage.ts
  • src/cli/handlers/mcp.tsx
  • src/commands/review/ultrareviewCommand.tsx
  • src/buddy/CompanionCard.tsx
  • src/components/PromptInput/PromptInputFooterSuggestions.tsx
  • src/components/messages/UserGitHubWebhookMessage.ts
  • src/hooks/useChromeExtensionNotification.tsx
  • src/utils/pipeRegistry.ts
  • src/utils/teleport.tsx
  • src/utils/computerUse/win32/inputIndicator.ts
  • src/hooks/notifs/usePluginAutoupdateNotification.tsx
  • src/services/api/openai/client.ts
  • src/commands/usage/usage.tsx
  • src/hooks/useTypeahead.tsx
  • src/bridge/bridgeMain.ts
  • src/components/LogoV2/CondensedLogo.tsx
  • src/bridge/replBridge.ts
  • src/hooks/useMergedTools.ts
  • src/components/messages/UserCrossSessionMessage.ts
  • src/types/permissions.ts
  • src/components/messages/UserForkBoilerplateMessage.ts
  • src/utils/computerUse/gates.ts
  • src/services/api/openai/index.ts
  • src/services/skillLearning/observationStore.ts
  • src/utils/permissions/PermissionMode.ts
  • src/components/Settings/Config.tsx
  • src/commands/diff/diff.tsx
  • src/commands/permissions/permissions.tsx
  • src/components/LogoV2/LogoV2.tsx
  • src/services/mcp/channelNotification.ts
  • src/components/PromptInput/SandboxPromptFooterHint.tsx
  • src/utils/permissions/bypassPermissionsKillswitch.ts
  • src/utils/autonomyStatus.ts
  • src/services/mcp/client.ts
  • src/components/LogoV2/feedConfigs.tsx
  • src/components/SandboxViolationExpandedView.tsx
  • src/services/api/claude.ts
  • src/types/message.ts
  • src/commands/ultraplan.tsx
  • src/services/skillSearch/localSearch.ts
  • src/bridge/remoteBridgeCore.ts
  • src/utils/claudemd.ts
  • src/utils/computerUse/win32/windowMessage.ts
  • src/components/BuiltinStatusLine.tsx
  • src/commands/skill-learning/skill-learning.ts
  • src/utils/model/modelOptions.ts

Comment thread scripts/dev.ts
Comment on lines +17 to +22
const defines = {
...getMacroDefines(),
// React production mode — prevents 6,889+ _debugStack Error objects
// (12MB) from accumulating during long-running sessions.
'process.env.NODE_ENV': JSON.stringify('production'),
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify how many runtime branches depend on NODE_ENV development.
rg -n --type=ts --type=tsx -C2 "process\.env\.NODE_ENV.*development|NODE_ENV as string.*development"

# Confirm the dev launcher currently forces production.
sed -n '14,24p' scripts/dev.ts

Repository: claude-code-best/claude-code

Length of output: 519


🏁 Script executed:

# Search for NODE_ENV development checks (fixing rg file type filter)
rg -n "NODE_ENV.*development|development.*NODE_ENV" --type ts -A2 -B2

# Check the specific files mentioned in the review
echo "=== desktopDeepLink.ts ===" 
rg -n "NODE_ENV" src/utils/desktopDeepLink.ts -C3

echo "=== doctorDiagnostic.ts ===" 
rg -n "NODE_ENV" src/utils/doctorDiagnostic.ts -C3

Repository: claude-code-best/claude-code

Length of output: 6645


🏁 Script executed:

# Check build/dev script context
echo "=== scripts directory ===" 
ls -la scripts/

echo "=== Check build script NODE_ENV handling ===" 
rg -n "NODE_ENV" scripts/ --type ts

echo "=== Check if there's a separate build launcher ===" 
rg -n "NODE_ENV.*production" scripts/ --type ts -C3

echo "=== Check React-related config for production mode comment context ===" 
rg -n "debugStack|6,889" . --type ts --type tsx

Repository: claude-code-best/claude-code

Length of output: 1791


🏁 Script executed:

# Read dev.ts in full to understand the production mode context
cat -n scripts/dev.ts

echo "=== dev-debug.ts ===" 
cat -n scripts/dev-debug.ts

echo "=== Check package.json for script invocations ===" 
rg -n "dev\.ts|dev-debug" . -t json --type toml -C2

Repository: claude-code-best/claude-code

Length of output: 2872


Hardcoded production NODE_ENV in dev launcher breaks dev-only diagnostics and workflows.

Lines 17–22 force all process.env.NODE_ENV checks to 'production' at compile time. This disables 15+ dev-only branches across the codebase (doctorDiagnostic.ts, desktopDeepLink.ts, DevBar.tsx, warningHandler.ts, AutoUpdater components, and others), breaking developer diagnostic features and install-type detection. While the React memory optimization comment is valid, it should not disable the entire dev experience.

Make the production mode opt-in via environment variable while preserving the React optimization for developers who need it.

🔧 Suggested fix
+const forcedNodeEnv = process.env.CLAUDE_CODE_FORCE_NODE_ENV ?? process.env.NODE_ENV ?? 'development';
 const defines = {
   ...getMacroDefines(),
-  // React production mode — prevents 6,889+ _debugStack Error objects
-  // (12MB) from accumulating during long-running sessions.
-  'process.env.NODE_ENV': JSON.stringify('production'),
+  'process.env.NODE_ENV': JSON.stringify(forcedNodeEnv),
 }

Developers who need the React optimization can opt in: CLAUDE_CODE_FORCE_NODE_ENV=production bun run dev

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const defines = {
...getMacroDefines(),
// React production mode — prevents 6,889+ _debugStack Error objects
// (12MB) from accumulating during long-running sessions.
'process.env.NODE_ENV': JSON.stringify('production'),
}
const forcedNodeEnv = process.env.CLAUDE_CODE_FORCE_NODE_ENV ?? process.env.NODE_ENV ?? 'development';
const defines = {
...getMacroDefines(),
'process.env.NODE_ENV': JSON.stringify(forcedNodeEnv),
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/dev.ts` around lines 17 - 22, The current defines object hardcodes
'process.env.NODE_ENV' to 'production' which disables dev-only branches; change
this so the production value is applied only when a new opt-in env var (e.g.
CLAUDE_CODE_FORCE_NODE_ENV) is set: keep the defines and getMacroDefines() usage
but set 'process.env.NODE_ENV' to
JSON.stringify(process.env.CLAUDE_CODE_FORCE_NODE_ENV || process.env.NODE_ENV ||
'development') (or equivalent) so developers default to dev mode while allowing
opt-in production mode for the React memory optimization when
CLAUDE_CODE_FORCE_NODE_ENV=production is provided; update any related comments
to reflect the opt-in behavior and reference the defines symbol and
getMacroDefines().

Comment thread src/screens/REPL.tsx
useMasterMonitor();
useSlaveNotifications();
const pipeIpcState = useAppState(s => getPipeIpc(s as any));
const _pipeIpcState = useAppState(s => getPipeIpc(s as any));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify no prohibited `as any` assertions remain in TS/TSX production files.
# Expected: zero matches in src/** after applying the fix.
rg -nP --type=ts --type=tsx '\bas\s+any\b' src

Repository: claude-code-best/claude-code

Length of output: 100


🏁 Script executed:

#!/bin/bash
# Search for `as any` patterns in TypeScript/TSX files
# Using fd to find files, then rg to search within them
fd '\.(ts|tsx)$' src --exec rg -n 'as\s+any' {} +

Repository: claude-code-best/claude-code

Length of output: 100714


🏁 Script executed:

cat -n src/screens/REPL.tsx | sed -n '4935,4945p'

Repository: claude-code-best/claude-code

Length of output: 564


🏁 Script executed:

# Find the useAppState hook definition
rg -n "export.*useAppState|function useAppState|const useAppState" --type=ts --type=tsx -A 3 src

Repository: claude-code-best/claude-code

Length of output: 100


🏁 Script executed:

# Find getPipeIpc function definition
rg -n "export.*getPipeIpc|function getPipeIpc|const getPipeIpc" --type=ts --type=tsx -A 3 src

Repository: claude-code-best/claude-code

Length of output: 100


🏁 Script executed:

# Look at the imports and context around line 4940 in REPL.tsx
head -n 4945 src/screens/REPL.tsx | tail -n 20

Repository: claude-code-best/claude-code

Length of output: 694


🏁 Script executed:

# Find useAppState definition
rg -n "useAppState" src --max-count=5 | head -20

Repository: claude-code-best/claude-code

Length of output: 1935


🏁 Script executed:

# Find getPipeIpc function definition
rg -n "getPipeIpc" src --max-count=10 | head -30

Repository: claude-code-best/claude-code

Length of output: 2467


🏁 Script executed:

# Look at imports at the top of REPL.tsx around getPipeIpc
head -n 100 src/screens/REPL.tsx | grep -E "import|getPipeIpc"

Repository: claude-code-best/claude-code

Length of output: 2131


🏁 Script executed:

sed -n '715,730p' src/utils/pipeTransport.ts

Repository: claude-code-best/claude-code

Length of output: 415


🏁 Script executed:

rg -n "useAppState.*getPipeIpc" src

Repository: claude-code-best/claude-code

Length of output: 432


🏁 Script executed:

# Check how useAppState is typed
sed -n '100,120p' src/state/AppState.tsx

Repository: claude-code-best/claude-code

Length of output: 775


Remove the unnecessary as any cast from the AppState selector.

Line 4940 uses s as any, but this cast is redundant. The getPipeIpc function is explicitly designed to accept state: any, as shown in its definition at src/utils/pipeTransport.ts:721. Other usages of this selector throughout the codebase (e.g., in useMasterMonitor.ts and useSlaveNotifications.ts) call getPipeIpc(s) without any cast. Simply remove the as any cast to match the pattern:

Suggested fix
- const _pipeIpcState = useAppState(s => getPipeIpc(s as any));
+ const _pipeIpcState = useAppState(s => getPipeIpc(s));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/screens/REPL.tsx` at line 4940, Remove the unnecessary cast in the
selector: change the useAppState call that assigns _pipeIpcState to call
getPipeIpc with the selector argument directly (useAppState(s => getPipeIpc(s)))
instead of casting s to any; update the line referencing _pipeIpcState in
REPL.tsx to remove "as any" so it matches other usages like useMasterMonitor and
useSlaveNotifications and relies on getPipeIpc’s accepted any-typed parameter.

SystemAPIErrorMessage,
} from '../../../types/message.js'
import { getEmptyToolPermissionContext, type Tools } from '../../../Tool.js'
import { type Tools } from '../../../Tool.js'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use path alias import instead of deep relative path

Line 9 still uses a ../../../ import, which violates the TS import-path guideline. Please switch to the src/* alias form.

Suggested change
-import { type Tools } from '../../../Tool.js'
+import type { Tools } from 'src/Tool.js'

As per coding guidelines: "Use src/* path alias in imports; do not use relative paths like ../../../".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { type Tools } from '../../../Tool.js'
import type { Tools } from 'src/Tool.js'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/api/gemini/index.ts` at line 9, Replace the deep-relative import
of the Tools type in src/services/api/gemini/index.ts with the project
path-alias form: change the import that references '../../../Tool.js' to use the
'src/...' alias (import { type Tools } from 'src/...') so the file imports Tools
via the src path alias instead of a '../../../' relative path; keep the imported
symbol name (Tools) unchanged.

Comment thread src/utils/fileHistory.ts
Comment on lines +54 to +56
// Disabled: file checkpointing causes unbounded memory growth (100 snapshots × full file backups).
// See heap snapshot analysis — re-enable only after switching to incremental diffs.
const MAX_SNAPSHOTS = 20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Misleading comment: feature is not disabled, just throttled.

The comment says "Disabled" but MAX_SNAPSHOTS = 20 keeps file checkpointing enabled with a lower cap. Other call sites still gate via fileHistoryEnabled(). The comment will confuse future readers about whether snapshots are still being taken.

📝 Suggested wording
-// Disabled: file checkpointing causes unbounded memory growth (100 snapshots × full file backups).
-// See heap snapshot analysis — re-enable only after switching to incremental diffs.
+// Reduced from 100 to mitigate unbounded memory growth (full file backups × N snapshots).
+// See heap snapshot analysis — only raise this cap after switching to incremental diffs.
 const MAX_SNAPSHOTS = 20
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Disabled: file checkpointing causes unbounded memory growth (100 snapshots × full file backups).
// See heap snapshot analysis — re-enable only after switching to incremental diffs.
const MAX_SNAPSHOTS = 20
// Reduced from 100 to mitigate unbounded memory growth (full file backups × N snapshots).
// See heap snapshot analysis — only raise this cap after switching to incremental diffs.
const MAX_SNAPSHOTS = 20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/fileHistory.ts` around lines 54 - 56, The comment above
MAX_SNAPSHOTS is misleading because snapshots are throttled, not disabled;
update the comment to accurately state that file checkpointing remains enabled
but capped to prevent unbounded growth and refer to the gating via
fileHistoryEnabled(). Locate the MAX_SNAPSHOTS constant and replace the
"Disabled" wording with a note like "Throttled: file checkpointing remains
enabled but capped at MAX_SNAPSHOTS (see fileHistoryEnabled() gating) and
re-enable full backups only after switching to incremental diffs," ensuring the
comment mentions the reason (heap snapshot growth) and the intended future
change.

Comment on lines +81 to +106
function getEntriesByType(type: string): PerformanceEntryLike[] {
if (type === 'mark') {
return [...marks.entries()].map(([name, startTime]) => ({
name,
entryType: 'mark',
startTime,
duration: 0,
}))
}
if (type === 'measure') {
return [...measures.values()].map(m => ({
name: m.name,
entryType: 'measure',
startTime: m.startTime,
duration: m.duration,
}))
}
return []
}

function getEntriesByName(name: string, type?: string): PerformanceEntryLike[] {
const entries = getEntriesByType(type ?? 'mark').concat(
type === undefined ? getEntriesByType('measure') : [],
)
return entries.filter(e => e.name === name)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing performance.getEntries() (no-arg variant) — as typeof performance cast hides the gap.

The shim implements getEntriesByType and getEntriesByName but not the no-argument getEntries() overload, even though the final cast at line 151 (as typeof performance) advertises it. Any consumer that calls performance.getEntries() will hit a TypeError: performance.getEntries is not a function after the shim is installed. Bun, OTel exporters, and various dev/observability tooling do call this.

The same gap exists for markResourceTiming, setResourceTimingBufferSize (you stub-typed the latter but not the former), eventCounts, and the EventTarget surface (addEventListener/removeEventListener/dispatchEvent). If the shim is meant to replace globalThis.performance wholesale, missing these creates a foot-gun once any new dependency starts using them.

🛡️ Add a no-arg getEntries() at minimum
+function getEntries(): PerformanceEntryLike[] {
+  return [...getEntriesByType('mark'), ...getEntriesByType('measure')]
+}
+
 const shim = {
   now,
   mark,
   measure: measure as typeof performance.measure,
+  getEntries: getEntries as typeof performance.getEntries,
   getEntriesByType: getEntriesByType as typeof performance.getEntriesByType,
#!/bin/bash
# Find callers that would break under the shim.
rg -nP --type=ts -C2 '\bperformance\.(getEntries|markResourceTiming|addEventListener|eventCounts)\b'
rg -nP --type=ts -C2 'globalThis\.performance\.(getEntries|markResourceTiming|addEventListener|eventCounts)\b'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/performanceShim.ts` around lines 81 - 106, Add the missing
no-argument overload and other missing members to the shim so it fully matches
typeof performance: implement performance.getEntries() to return the union of
getEntriesByType('mark') and getEntriesByType('measure') (reuse
getEntriesByType/getEntriesByName logic), and add stubbed/pass-through
implementations for markResourceTiming, setResourceTimingBufferSize (already
stub-typed but ensure the function exists), eventCounts, and the
EventTarget-like methods addEventListener/removeEventListener/dispatchEvent on
the shim object; ensure their signatures match the standard
Performance/EventTarget surface so the final "as typeof performance" cast is
accurate and consumers calling performance.getEntries() or the other APIs won't
throw.

Comment on lines +142 to +160
get onresourcetimingbufferfull() {
return (original as any).onresourcetimingbufferfull
},
set onresourcetimingbufferfull(_v: any) {
// no-op — prevent accumulation
},
toJSON() {
return original.toJSON()
},
} as typeof performance

/**
* Install the shim onto globalThis.performance. Safe to call multiple times.
* Must run before React and OTel import to prevent them from capturing the
* native Performance reference.
*/
export function installPerformanceShim(): void {
if ((globalThis as any).__performanceShimInstalled) return
;(globalThis as any).__performanceShimInstalled = true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

as any violates the project's TypeScript guideline.

Four as any / : any usages remain in production code: line 143 ((original as any).onresourcetimingbufferfull), line 145 (_v: any), and lines 159–160 ((globalThis as any).__performanceShimInstalled). Per the coding guideline, prefer as unknown as SpecificType or extend an interface.

♻️ Replace with safer assertions
   get onresourcetimingbufferfull() {
-    return (original as any).onresourcetimingbufferfull
+    return (original as Performance & { onresourcetimingbufferfull?: unknown })
+      .onresourcetimingbufferfull
   },
-  set onresourcetimingbufferfull(_v: any) {
+  set onresourcetimingbufferfull(_v: unknown) {
     // no-op — prevent accumulation
   },
@@
 export function installPerformanceShim(): void {
-  if ((globalThis as any).__performanceShimInstalled) return
-  ;(globalThis as any).__performanceShimInstalled = true
+  const g = globalThis as typeof globalThis & { __performanceShimInstalled?: boolean }
+  if (g.__performanceShimInstalled) return
+  g.__performanceShimInstalled = true
   globalThis.performance = shim
 }

As per coding guidelines, "Prohibit as any type assertions in production code; ... Use as unknown as SpecificType double assertion or interface supplementation when type mismatches occur".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
get onresourcetimingbufferfull() {
return (original as any).onresourcetimingbufferfull
},
set onresourcetimingbufferfull(_v: any) {
// no-op — prevent accumulation
},
toJSON() {
return original.toJSON()
},
} as typeof performance
/**
* Install the shim onto globalThis.performance. Safe to call multiple times.
* Must run before React and OTel import to prevent them from capturing the
* native Performance reference.
*/
export function installPerformanceShim(): void {
if ((globalThis as any).__performanceShimInstalled) return
;(globalThis as any).__performanceShimInstalled = true
get onresourcetimingbufferfull() {
return (original as Performance & { onresourcetimingbufferfull?: unknown })
.onresourcetimingbufferfull
},
set onresourcetimingbufferfull(_v: unknown) {
// no-op — prevent accumulation
},
toJSON() {
return original.toJSON()
},
} as typeof performance
/**
* Install the shim onto globalThis.performance. Safe to call multiple times.
* Must run before React and OTel import to prevent them from capturing the
* native Performance reference.
*/
export function installPerformanceShim(): void {
const g = globalThis as typeof globalThis & { __performanceShimInstalled?: boolean }
if (g.__performanceShimInstalled) return
g.__performanceShimInstalled = true
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/performanceShim.ts` around lines 142 - 160, The code uses unsafe
`as any` casts in the performance shim; replace them with safer
double-assertions or proper typings: cast `original` to the correct
Performance-like interface using `as unknown as { onresourcetimingbufferfull?:
EventListener | null; toJSON: () => any; [k: string]: any }` (or extend the
Performance interface) so the getter `onresourcetimingbufferfull` and the
`toJSON()` call use typed access instead of `(original as any)`, change the
setter parameter `_v: any` to a typed parameter (e.g. `_v: EventListener |
null`) or `_v: unknown` then cast only if needed, and replace `(globalThis as
any).__performanceShimInstalled` with `(globalThis as unknown as {
__performanceShimInstalled?: boolean }).__performanceShimInstalled` (or add an
ambient declaration), updating references in `get onresourcetimingbufferfull`,
`set onresourcetimingbufferfull`, `toJSON`, and `installPerformanceShim`
accordingly.

@claude-code-best claude-code-best merged commit f5c9880 into main May 5, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants