Skip to content

Conversation

@zerob13
Copy link
Collaborator

@zerob13 zerob13 commented Nov 28, 2025

Summary by CodeRabbit

  • Chores

    • Simplified built-in agent startup command and added an explicit install step tied to a specific Python runtime.
    • Updated dev tooling dependency (prettier).
  • Bug Fixes / Reliability

    • Improved agent process robustness with health checks, startup timeout, enhanced logging/telemetry, and safer failure handling.
  • Refactor

    • Broadened query hook options surface and exposed additional agent lifecycle info for better observability.
  • Style

    • Minor formatting and expression reflows across UI and test files.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

Walkthrough

Default Kimi CLI invocation and init commands changed; ACP process manager now performs pre-start health checks, wraps connection init in a 5-minute timeout, logs startup/success/failure, attaches stdout/stderr hooks, records lifecycle telemetry, and force-kills on failed startups.

Changes

Cohort / File(s) Summary
ACP Configuration
src/main/presenter/configPresenter/acpConfHelper.ts
Default built-in ACP agent profile now invokes command kimi with args ["--acp"] instead of the previous uv ... kimi --acp. No signature changes.
ACP Initialization
src/main/presenter/configPresenter/acpInitHelper.ts
Replaced direct uv tool run init command with an install-then-run sequence: ["uv tool install --python 3.13 kimi-cli", "kimi"]. No exported API changes.
ACP Process Management
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
Reworked process startup: checks child liveness before init, wraps ClientSideConnection initialization in a 5-minute Promise.race timeout, logs start/success/failure, adds stdout/stderr logging hooks, process error/exit telemetry (including child PID), and forces kill on failed startup. Added fields to AcpProcessHandle: child, connection, agent, readyAt.
Types / Query Options
src/renderer/src/composables/useIpcQuery.ts
Broadened UseIpcQueryOptions to extend full UseQueryOptions (instead of a Pick subset), exposing all query options in the composable's option type.
Minor / Formatting
src/main/presenter/llmProviderPresenter/providers/openAICompatibleProvider.ts, src/renderer/src/components/editor/mention/MentionList.vue, test/main/presenter/FileValidationService.test.ts, package.json, src/main/presenter/configPresenter/acpInitHelper.ts*
Small formatting changes and a devDependency bump (prettier), plus minor test import formatting. (acpInitHelper.ts content change summarized above.)

Sequence Diagram(s)

sequenceDiagram
    participant Spawner as Process Spawner
    participant Child as ACP Child Process
    participant Connector as ClientSideConnection
    participant Logger as Telemetry/Logger

    Spawner->>Child: spawn child (record pid)
    Spawner->>Logger: log "spawn" (pid)
    Note right of Spawner: verify child not killed
    Spawner->>Connector: initialize connection (Promise.race with 5min timeout)
    alt init success
        Connector-->>Spawner: ready
        Spawner->>Logger: log "init success" (readyAt, pid)
        Child->>Logger: stdout/stderr -> hooked logs
    else init failure or timeout
        Connector-->>Spawner: error/timeout
        Spawner->>Logger: log "init failure" (error, pid)
        Spawner->>Child: force kill (if running)
        Spawner->>Logger: log "killed" (pid)
    end
    Child->>Spawner: exit event
    Spawner->>Logger: log "exit" (pid, code)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review guarded init timeout and Promise.race cancellation/cleanup in acpProcessManager.ts.
  • Verify stdout/stderr hooks and that logging/telemetry do not leak file descriptors or timers.
  • Confirm AcpProcessHandle additions are used consistently and won't break callers.
  • Validate the install-and-run command sequence in acpInitHelper.ts (Python version and invocation correctness).

Poem

🐇 I hop to test the new Kimi way,
I swap the uv chain for a lighter play.
With timeouts guarded and logs that sing,
Childs start safe — what joy I bring!
— cheers from a coding rabbit ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the primary changes: updating the kimi initialization script (in acpInitHelper.ts) and the ACP command (in acpConfHelper.ts). It is concise and specific enough for quick understanding.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/kimi-cli-acp

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2fe59f3 and f85e929.

📒 Files selected for processing (5)
  • package.json (1 hunks)
  • src/main/presenter/llmProviderPresenter/providers/openAICompatibleProvider.ts (1 hunks)
  • src/renderer/src/components/editor/mention/MentionList.vue (1 hunks)
  • src/renderer/src/composables/useIpcQuery.ts (1 hunks)
  • test/main/presenter/FileValidationService.test.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • test/main/presenter/FileValidationService.test.ts
  • src/main/presenter/llmProviderPresenter/providers/openAICompatibleProvider.ts
🧰 Additional context used
📓 Path-based instructions (25)
package.json

📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)

package.json: Node.js >= 22 required
pnpm >= 9 required

Files:

  • package.json
**/*.{ts,tsx,js,jsx,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
**/*.vue

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.vue: Use Vue 3 Composition API for all components instead of Options API
Use Tailwind CSS with scoped styles for component styling

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
src/renderer/**/*.vue

📄 CodeRabbit inference engine (CLAUDE.md)

src/renderer/**/*.vue: All user-facing strings must use i18n keys via vue-i18n for internationalization
Ensure proper error handling and loading states in all UI components
Implement responsive design using Tailwind CSS utilities for all UI components

src/renderer/**/*.vue: Use composition API and declarative programming patterns; avoid options API
Structure files: exported component, composables, helpers, static content, types
Use PascalCase for component names (e.g., AuthWizard.vue)
Use Vue 3 with TypeScript, leveraging defineComponent and PropType
Use template syntax for declarative rendering
Use Shadcn Vue, Radix Vue, and Tailwind for components and styling
Implement responsive design with Tailwind CSS; use a mobile-first approach
Use Suspense for asynchronous components
Use <script setup> syntax for concise component definitions
Prefer 'lucide:' icon family as the primary choice for Iconify icons
Import Icon component from '@iconify/vue' and use with lucide icons following pattern '{collection}:{icon-name}'

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
src/renderer/src/**/*.{vue,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)

src/renderer/src/**/*.{vue,ts,tsx}: All user-facing strings must use i18n keys with vue-i18n framework in the renderer
Import and use useI18n() composable with the t() function to access translations in Vue components and TypeScript files
Use the dynamic locale.value property to switch languages at runtime
Avoid hardcoding user-facing text and ensure all user-visible text uses the i18n translation system

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/**/*

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

New features should be developed in the src directory

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/**/*.{vue,js,ts}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Renderer process code should be placed in src/renderer (Vue 3 application)

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)

src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability in Vue.js applications
Implement proper state management with Pinia in Vue.js applications
Utilize Vue Router for navigation and route management in Vue.js applications
Leverage Vue's built-in reactivity system for efficient data handling

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/src/**/*.vue

📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)

Use scoped styles to prevent CSS conflicts between Vue components

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
src/renderer/**/*.{ts,tsx,vue}

📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)

src/renderer/**/*.{ts,tsx,vue}: Write concise, technical TypeScript code with accurate examples
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Avoid enums; use const objects instead
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements

Vue 3 app code in src/renderer/src should be organized into components/, stores/, views/, i18n/, lib/ directories with shell UI in src/renderer/shell/

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/**

📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)

Use lowercase with dashes for directories (e.g., components/auth-wizard)

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/**/*.{ts,vue}

📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)

src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching
Leverage ref, reactive, and computed for reactive state management
Use provide/inject for dependency injection when appropriate
Use Iconify/Vue for icon implementation

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/src/**/*.{ts,tsx,vue}

📄 CodeRabbit inference engine (AGENTS.md)

src/renderer/src/**/*.{ts,tsx,vue}: Use TypeScript with Vue 3 Composition API for the renderer application
All user-facing strings must use vue-i18n keys in src/renderer/src/i18n

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/src/components/**/*.vue

📄 CodeRabbit inference engine (AGENTS.md)

src/renderer/src/components/**/*.vue: Use Tailwind for styles in Vue components
Vue component files must use PascalCase naming (e.g., ChatInput.vue)

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
src/**/*.{ts,tsx,vue,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Prettier with single quotes, no semicolons, and 100 character width

Files:

  • src/renderer/src/components/editor/mention/MentionList.vue
  • src/renderer/src/composables/useIpcQuery.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Enable and maintain strict TypeScript type checking for all files

**/*.{ts,tsx}: Always use try-catch to handle possible errors in TypeScript code
Provide meaningful error messages when catching errors
Log detailed error logs including error details, context, and stack traces
Distinguish and handle different error types (UserError, NetworkError, SystemError, BusinessError) with appropriate handlers in TypeScript
Use structured logging with logger.error(), logger.warn(), logger.info(), logger.debug() methods from logging utilities
Do not suppress errors (avoid empty catch blocks or silently ignoring errors)
Provide user-friendly error messages for user-facing errors in TypeScript components
Implement error retry mechanisms for transient failures in TypeScript
Avoid logging sensitive information (passwords, tokens, PII) in logs

Files:

  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use the usePresenter.ts composable for renderer-to-main IPC communication to call presenter methods directly

Files:

  • src/renderer/src/composables/useIpcQuery.ts
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Do not include AI co-authoring information (e.g., 'Co-Authored-By: Claude') in git commits

Files:

  • src/renderer/src/composables/useIpcQuery.ts
**/*.{js,ts,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)

Write logs and comments in English

Files:

  • src/renderer/src/composables/useIpcQuery.ts
{src/main/presenter/**/*.ts,src/renderer/**/*.ts}

📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)

Implement proper inter-process communication (IPC) patterns using Electron's ipcRenderer and ipcMain APIs

Files:

  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/**/composables/*.ts

📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)

src/renderer/**/composables/*.ts: Use camelCase for composables (e.g., useAuthState.ts)
Use VueUse for common composables and utility functions
Implement custom composables for reusable logic

Files:

  • src/renderer/src/composables/useIpcQuery.ts
src/renderer/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)

Use TypeScript for all code; prefer types over interfaces

Files:

  • src/renderer/src/composables/useIpcQuery.ts
src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use OxLint for linting JavaScript and TypeScript files

Files:

  • src/renderer/src/composables/useIpcQuery.ts
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Use camelCase for variable and function names in TypeScript files
Use PascalCase for type and class names in TypeScript
Use SCREAMING_SNAKE_CASE for constant names

Files:

  • src/renderer/src/composables/useIpcQuery.ts
src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use EventBus for inter-process communication events

Files:

  • src/renderer/src/composables/useIpcQuery.ts
🧠 Learnings (8)
📚 Learning: 2025-11-25T05:26:15.918Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/development-setup.mdc:0-0
Timestamp: 2025-11-25T05:26:15.918Z
Learning: Applies to package.json : Node.js >= 22 required

Applied to files:

  • package.json
📚 Learning: 2025-11-25T05:28:20.500Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.500Z
Learning: Applies to src/**/*.{ts,tsx,vue,js,jsx} : Use Prettier with single quotes, no semicolons, and 100 character width

Applied to files:

  • package.json
📚 Learning: 2025-11-25T05:28:20.500Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.500Z
Learning: Require Node ≥ 20.19 and pnpm ≥ 10.11 (pnpm only, not npm) as the project toolchain

Applied to files:

  • package.json
📚 Learning: 2025-11-25T05:26:15.918Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/development-setup.mdc:0-0
Timestamp: 2025-11-25T05:26:15.918Z
Learning: Applies to package.json : pnpm >= 9 required

Applied to files:

  • package.json
📚 Learning: 2025-11-25T05:28:20.500Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.500Z
Learning: Applies to src/**/*.{ts,tsx,js,jsx} : Use OxLint for linting JavaScript and TypeScript files

Applied to files:

  • package.json
📚 Learning: 2025-11-25T05:28:04.439Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.439Z
Learning: Applies to src/renderer/**/*.vue : Structure files: exported component, composables, helpers, static content, types

Applied to files:

  • src/renderer/src/components/editor/mention/MentionList.vue
📚 Learning: 2025-11-25T05:28:04.439Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.439Z
Learning: Applies to src/renderer/**/*.{ts,tsx,vue} : Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements

Applied to files:

  • src/renderer/src/components/editor/mention/MentionList.vue
📚 Learning: 2025-11-25T05:26:11.297Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.297Z
Learning: Applies to src/renderer/**/*.ts : Use the `usePresenter.ts` composable for renderer-to-main IPC communication to call presenter methods directly

Applied to files:

  • src/renderer/src/composables/useIpcQuery.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-check (x64)
🔇 Additional comments (4)
src/renderer/src/components/editor/mention/MentionList.vue (1)

84-95: LGTM!

The hasFiles function correctly implements a type guard pattern to safely check for the files field on mcpEntry. The reformatted boolean expression maintains the same defensive checks and is appropriately readable.

src/renderer/src/composables/useIpcQuery.ts (2)

18-29: LGTM!

The interface correctly constrains the exposed options to only enabled, staleTime, and gcTime using Pick. This maintains a focused API surface for IPC queries.


21-24: Note: AI summary inconsistency.

The summary claims the interface was broadened to expose all UseQueryOptions fields, but the code correctly constrains options to only enabled, staleTime, and gcTime via Pick. The current implementation is appropriate for limiting the API surface.

package.json (1)

156-156: LGTM!

Routine dev dependency update for Prettier. Version 3.7.1 verified to exist on npm registry.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
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: 2

🧹 Nitpick comments (1)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (1)

184-221: Timeout promise continues running after successful initialization.

The timeout promise created at lines 195-203 will continue its setTimeout even after initPromise resolves successfully. While this is a minor memory concern (the rejection is a no-op after Promise.race settles), it's cleaner to cancel the timer.

     const timeoutMs = 30000 // 30 seconds timeout for initialization
+    let timeoutId: NodeJS.Timeout | undefined
 
     try {
       const initPromise = connection.initialize({
         protocolVersion: PROTOCOL_VERSION,
         clientCapabilities: {},
         clientInfo: { name: 'DeepChat', version: app.getVersion() }
       })
 
       const timeoutPromise = new Promise<never>((_, reject) => {
-        setTimeout(() => {
+        timeoutId = setTimeout(() => {
           reject(
             new Error(
               `[ACP] Connection initialization timeout after ${timeoutMs}ms for agent ${agent.id}`
             )
           )
         }, timeoutMs)
       })
 
       await Promise.race([initPromise, timeoutPromise])
+      clearTimeout(timeoutId)
       console.info(`[ACP] Connection initialization completed successfully for agent ${agent.id}`)
     } catch (error) {
+      clearTimeout(timeoutId)
       console.error(`[ACP] Connection initialization failed for agent ${agent.id}:`, error)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd3ab77 and 88abc0f.

📒 Files selected for processing (3)
  • src/main/presenter/configPresenter/acpConfHelper.ts (1 hunks)
  • src/main/presenter/configPresenter/acpInitHelper.ts (1 hunks)
  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{ts,tsx,js,jsx,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Enable and maintain strict TypeScript type checking for all files

**/*.{ts,tsx}: Always use try-catch to handle possible errors in TypeScript code
Provide meaningful error messages when catching errors
Log detailed error logs including error details, context, and stack traces
Distinguish and handle different error types (UserError, NetworkError, SystemError, BusinessError) with appropriate handlers in TypeScript
Use structured logging with logger.error(), logger.warn(), logger.info(), logger.debug() methods from logging utilities
Do not suppress errors (avoid empty catch blocks or silently ignoring errors)
Provide user-friendly error messages for user-facing errors in TypeScript components
Implement error retry mechanisms for transient failures in TypeScript
Avoid logging sensitive information (passwords, tokens, PII) in logs

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/main/presenter/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Organize core business logic into dedicated Presenter classes, with one presenter per functional domain

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/main/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use EventBus from src/main/eventbus.ts for main-to-renderer communication, broadcasting events via mainWindow.webContents.send()

src/main/**/*.ts: Use EventBus pattern for inter-process communication within the main process to decouple modules
Use Electron's built-in APIs for file system and native dialogs instead of Node.js or custom implementations

src/main/**/*.ts: Electron main process code belongs in src/main/ with presenters in presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider) and eventbus.ts for app events
Use the Presenter pattern in the main process for UI coordination

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Do not include AI co-authoring information (e.g., 'Co-Authored-By: Claude') in git commits

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
**/*.{js,ts,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)

Write logs and comments in English

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
{src/main/presenter/**/*.ts,src/renderer/**/*.ts}

📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)

Implement proper inter-process communication (IPC) patterns using Electron's ipcRenderer and ipcMain APIs

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/main/presenter/llmProviderPresenter/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)

Define the standardized LLMCoreStreamEvent interface with fields: type (text | reasoning | tool_call_start | tool_call_chunk | tool_call_end | error | usage | stop | image_data), content (for text), reasoning_content (for reasoning), tool_call_id, tool_call_name, tool_call_arguments_chunk (for streaming), tool_call_arguments_complete (for complete arguments), error_message, usage object with token counts, stop_reason (tool_use | max_tokens | stop_sequence | error | complete), and image_data object with Base64-encoded data and mimeType

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/**/*

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

New features should be developed in the src directory

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/main/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Main process code for Electron should be placed in src/main

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/**/*.{ts,tsx,vue,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Prettier with single quotes, no semicolons, and 100 character width

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use OxLint for linting JavaScript and TypeScript files

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Use camelCase for variable and function names in TypeScript files
Use PascalCase for type and class names in TypeScript
Use SCREAMING_SNAKE_CASE for constant names

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use EventBus for inter-process communication events

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
src/main/presenter/configPresenter/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Store and retrieve custom prompts via configPresenter.getCustomPrompts() for config-based data source management

Files:

  • src/main/presenter/configPresenter/acpConfHelper.ts
  • src/main/presenter/configPresenter/acpInitHelper.ts
🧠 Learnings (5)
📚 Learning: 2025-11-25T05:27:12.201Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Implement separation of concerns where `src/main/presenter/llmProviderPresenter/index.ts` manages the Agent loop and conversation history, while Provider files handle LLM API interactions, Provider-specific request/response formatting, tool definition conversion, and native vs non-native tool call mechanisms

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:26:11.297Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.297Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Convert MCP tools to provider-specific formats and normalize streaming responses to standard events in each provider implementation

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:27:12.201Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts` (`startStreamCompletion`), implement the Agent loop that manages the overall conversation flow, including multiple rounds of LLM calls and tool usage, maintaining `conversationMessages` history, calling `provider.coreStream()` on each iteration, and controlling the loop using `needContinueConversation` and `toolCallCount` (compared against `MAX_TOOL_CALLS`)

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:26:24.860Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:26:24.860Z
Learning: Applies to {src/main/presenter/**/*.ts,src/renderer/**/*.ts} : Implement proper inter-process communication (IPC) patterns using Electron's ipcRenderer and ipcMain APIs

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:27:12.201Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts`, listen for standardized events yielded by `coreStream` and handle them accordingly: buffer text content (`currentContent`), handle `tool_call_start/chunk/end` events by collecting tool details and calling `presenter.mcpPresenter.callTool`, send frontend events via `eventBus` with tool call status, format tool results for the next LLM call, and set `needContinueConversation = true`

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
🧬 Code graph analysis (1)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (1)
test/mocks/electron.ts (1)
  • app (2-10)
🔇 Additional comments (5)
src/main/presenter/configPresenter/acpConfHelper.ts (1)

27-28: LGTM!

The default profile update aligns with the new initialization flow in acpInitHelper.ts. After uv tool install kimi-cli is executed during initialization, the kimi command becomes available system-wide, making this direct invocation correct.

src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (4)

177-182: LGTM - Good defensive check.

Pre-initialization health check properly guards against the race condition where the process exits before initialization begins. The error message is descriptive with the PID included for debugging.


254-259: LGTM - Improved stderr logging.

The enhanced stderr logging with trimming and conditional output is appropriate for capturing agent errors. Unlike stdout, stderr is not consumed by the protocol stream.


261-266: LGTM - Good process error handling.

Adding an error handler for the child process prevents unhandled error events (e.g., spawn failures, ENOENT). The monitoring log provides useful observability.


238-239: LGTM - Improved exit logging.

Including the PID in the exit log message improves debuggability when multiple agent processes are running.

Copy link
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: 0

♻️ Duplicate comments (1)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (1)

247-252: Remove stdout logging - it corrupts the ACP protocol stream.

This stdout listener conflicts with the protocol stream consumer. At line 173, createAgentStream() converts child.stdout to a Web ReadableStream and passes it to ndJsonStream() (line 546), which the ACP ClientSideConnection immediately begins consuming for JSON-RPC messages (line 175). Adding this separate .on('data') listener at lines 247-252 creates competing consumers that unpredictably split the stream data, corrupting protocol messages and causing parsing failures.

Remove these stdout listeners entirely:

-    child.stdout?.on('data', (chunk: Buffer) => {
-      const output = chunk.toString().trim()
-      if (output) {
-        console.info(`[ACP] ${agent.id} stdout: ${output}`)
-      }
-    })
-

If debugging output is needed, use ACP's diagnostic mechanisms or implement non-invasive logging (e.g., stream duplication via Transform before Web conversion), not competing data listeners.

🧹 Nitpick comments (2)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (2)

186-186: Reduce initialization timeout from 5 minutes to 30-60 seconds.

A 5-minute timeout is excessive for connection initialization. Typical ACP handshakes complete within seconds, and a hang beyond 30-60 seconds usually indicates an unrecoverable failure. The current timeout delays error detection and degrades user experience.

Apply this diff to use a 60-second timeout:

-    const timeoutMs = 60 * 1000 * 5 // 5 minutes timeout for initialization
+    const timeoutMs = 60 * 1000 // 60 seconds timeout for initialization

Or for a 30-second timeout:

-    const timeoutMs = 60 * 1000 * 5 // 5 minutes timeout for initialization
+    const timeoutMs = 30 * 1000 // 30 seconds timeout for initialization

Also applies to: 195-203


186-186: Reorder timeout calculation for clarity.

60 * 1000 * 5 reads as "60 seconds times 5". Reordering to 5 * 60 * 1000 makes the intent clearer: "5 minutes in milliseconds".

-    const timeoutMs = 60 * 1000 * 5 // 5 minutes timeout for initialization
+    const timeoutMs = 5 * 60 * 1000 // 5 minutes timeout for initialization
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88abc0f and 2fe59f3.

📒 Files selected for processing (1)
  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx,js,jsx,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Enable and maintain strict TypeScript type checking for all files

**/*.{ts,tsx}: Always use try-catch to handle possible errors in TypeScript code
Provide meaningful error messages when catching errors
Log detailed error logs including error details, context, and stack traces
Distinguish and handle different error types (UserError, NetworkError, SystemError, BusinessError) with appropriate handlers in TypeScript
Use structured logging with logger.error(), logger.warn(), logger.info(), logger.debug() methods from logging utilities
Do not suppress errors (avoid empty catch blocks or silently ignoring errors)
Provide user-friendly error messages for user-facing errors in TypeScript components
Implement error retry mechanisms for transient failures in TypeScript
Avoid logging sensitive information (passwords, tokens, PII) in logs

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/main/presenter/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Organize core business logic into dedicated Presenter classes, with one presenter per functional domain

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/main/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use EventBus from src/main/eventbus.ts for main-to-renderer communication, broadcasting events via mainWindow.webContents.send()

src/main/**/*.ts: Use EventBus pattern for inter-process communication within the main process to decouple modules
Use Electron's built-in APIs for file system and native dialogs instead of Node.js or custom implementations

src/main/**/*.ts: Electron main process code belongs in src/main/ with presenters in presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider) and eventbus.ts for app events
Use the Presenter pattern in the main process for UI coordination

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Do not include AI co-authoring information (e.g., 'Co-Authored-By: Claude') in git commits

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
**/*.{js,ts,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)

Write logs and comments in English

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
{src/main/presenter/**/*.ts,src/renderer/**/*.ts}

📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)

Implement proper inter-process communication (IPC) patterns using Electron's ipcRenderer and ipcMain APIs

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/main/presenter/llmProviderPresenter/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)

Define the standardized LLMCoreStreamEvent interface with fields: type (text | reasoning | tool_call_start | tool_call_chunk | tool_call_end | error | usage | stop | image_data), content (for text), reasoning_content (for reasoning), tool_call_id, tool_call_name, tool_call_arguments_chunk (for streaming), tool_call_arguments_complete (for complete arguments), error_message, usage object with token counts, stop_reason (tool_use | max_tokens | stop_sequence | error | complete), and image_data object with Base64-encoded data and mimeType

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/**/*

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

New features should be developed in the src directory

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/main/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Main process code for Electron should be placed in src/main

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/**/*.{ts,tsx,vue,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Prettier with single quotes, no semicolons, and 100 character width

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use OxLint for linting JavaScript and TypeScript files

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Use camelCase for variable and function names in TypeScript files
Use PascalCase for type and class names in TypeScript
Use SCREAMING_SNAKE_CASE for constant names

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use EventBus for inter-process communication events

Files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
🧠 Learnings (4)
📚 Learning: 2025-11-25T05:27:12.201Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Implement separation of concerns where `src/main/presenter/llmProviderPresenter/index.ts` manages the Agent loop and conversation history, while Provider files handle LLM API interactions, Provider-specific request/response formatting, tool definition conversion, and native vs non-native tool call mechanisms

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:26:11.297Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.297Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Convert MCP tools to provider-specific formats and normalize streaming responses to standard events in each provider implementation

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:27:12.201Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.201Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts` (`startStreamCompletion`), implement the Agent loop that manages the overall conversation flow, including multiple rounds of LLM calls and tool usage, maintaining `conversationMessages` history, calling `provider.coreStream()` on each iteration, and controlling the loop using `needContinueConversation` and `toolCallCount` (compared against `MAX_TOOL_CALLS`)

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:26:24.860Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:26:24.860Z
Learning: Applies to {src/main/presenter/**/*.ts,src/renderer/**/*.ts} : Implement proper inter-process communication (IPC) patterns using Electron's ipcRenderer and ipcMain APIs

Applied to files:

  • src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
🧬 Code graph analysis (1)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (1)
test/mocks/electron.ts (1)
  • app (2-10)
🔇 Additional comments (4)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (4)

177-182: Good defensive check before initialization.

This health check prevents attempting to initialize a connection to an already-dead process. The error message includes helpful debugging context.


207-221: Well-structured error handling and cleanup.

The error handling properly logs failures, cleans up the child process if still running, and re-throws to propagate the error. The try-catch around child.kill() prevents secondary failures during cleanup.


254-259: Stderr logging is safe and helpful.

Unlike stdout, stderr is not part of the ACP protocol stream (only stdout is converted to Web stream in createAgentStream), so this logging listener doesn't interfere with protocol parsing. The trim and conditional check avoid noise from empty lines.


261-266: Good addition for process observability.

The error event handler captures process-level errors that might not immediately trigger exit events, improving debugging capabilities. The monitoring log confirms setup with helpful context.

@zerob13 zerob13 merged commit 3a89d04 into dev Nov 28, 2025
1 of 2 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