fix: close app kill all acp processes #1175
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds safe provider lookups, an ACP before-quit lifecycle hook that invokes provider cleanup, ACP provider cleanup and shutdown-aware error paths, stronger ACP process-manager shutdown/force-kill behavior, a guard preventing merging of tool messages, new provider-instance getters and related typings, and unit tests for the shutdown/error flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Lifecycle as Lifecycle Presenter
participant LLMPres as LLMProviderPresenter
participant AcpProv as AcpProvider
participant SessionMgr as SessionManager
participant ProcMgr as AcpProcessManager
Lifecycle->>LLMPres: getExistingProviderInstance("acp")
activate LLMPres
LLMPres-->>Lifecycle: AcpProvider | undefined
deactivate LLMPres
alt provider exists
Lifecycle->>AcpProv: cleanup()
activate AcpProv
AcpProv->>SessionMgr: clearAllSessions()
activate SessionMgr
SessionMgr-->>AcpProv: done (errors caught)
deactivate SessionMgr
AcpProv->>ProcMgr: shutdown()
activate ProcMgr
ProcMgr->>ProcMgr: set shuttingDown = true
ProcMgr->>ProcMgr: forceKillAllProcesses("shutdown")
note right of ProcMgr `#ddeeff`: Windows uses taskkill\nUnix uses pkill/SIGTERM
ProcMgr-->>AcpProv: done (errors caught)
deactivate ProcMgr
AcpProv->>AcpProv: resolve pending permission requests as cancelled
AcpProv-->>Lifecycle: cleanup complete
deactivate AcpProv
else provider missing
Lifecycle-->>Lifecycle: early return (no provider)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/types/presenters/legacy.presenters.d.ts (1)
835-847: UnifyILlmProviderPresenter.getProviderInstanceoptionality to avoid type conflicts
ILlmProviderPresentercurrently declaresgetProviderInstanceboth as optional (Line 840) and required (Line 945). TypeScript expects all declarations of the same member to have identical modifiers; mixing optional and required can cause declaration conflicts.Since call sites are already using optional chaining (e.g.
llmProviderPresenter.getProviderInstance?.(...)), it’s more consistent to make the method optional everywhere.Suggested fix (lower block only):
- getProviderInstance(providerId: string): unknown - getExistingProviderInstance?(providerId: string): unknown + getProviderInstance?(providerId: string): unknown + getExistingProviderInstance?(providerId: string): unknownThis keeps the earlier optional declarations consistent and avoids conflicting modifiers.
Also applies to: 945-947
🧹 Nitpick comments (4)
src/main/presenter/threadPresenter/handlers/utilityHandler.ts (1)
371-375: Guarded provider lookup improves robustnessUsing
getProviderInstance?.(...) ?? nullavoids a hard crash if the presenter implementation doesn’t expose that method, while still surfacing a clear"Provider ... not found"error. Behavior for normal implementations remains unchanged.src/main/presenter/configPresenter/index.ts (1)
1193-1207: Defensive ACP provider lookup in config presenterSwitching to
presenter?.llmproviderPresenter?.getProviderInstance?.('acp')avoids crashes when the presenter graph isn’t fully wired (e.g. during tests or early startup), while still refreshing agents when the ACP provider exists.src/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.ts (1)
1-37: ACP cleanup hook correctly avoids new instances during shutdownThe hook cleanly:
- Kills the ACP init terminal in a guarded block, and
- Fetches only an already-instantiated ACP provider via
getExistingProviderInstancebefore calling its optionalcleanup().This prevents creating new ACP processes during shutdown and still tears down existing ones, with failures downgraded to warnings so lifecycle shutdown can proceed.
Consider switching the new
console.log/console.warncalls to the project’s structured logger if/when one is available in this module, to align with logging guidelines.src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (1)
996-999: Centralized, cross‑platform kill logic matches the shutdown goal; consider an optional pkill fallbackRouting both
disposeHandleandshutdown()throughkillChild(viaforceKillAllProcesses) gives a single, auditable place for process termination, and the platform split:
- Windows:
taskkill /PID <pid> /T /Fpluschild.kill()- Others:
pkill -TERM -P <pid>+process.kill(pid, 'SIGTERM')+child.kill()should aggressively clean up ACP trees on app shutdown.
One optional hardening you might consider (non‑blocking):
- If your support matrix includes Unix environments without
pkill, wrap that spawn in a small helper that degrades quietly to justprocess.kill/child.kill()instead of logging repeated warnings whenpkillis missing.Otherwise, this looks consistent with the intent to avoid orphaned ACP processes.
Also applies to: 1035-1070
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/presenter/configPresenter/index.ts(1 hunks)src/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.ts(1 hunks)src/main/presenter/lifecyclePresenter/hooks/index.ts(1 hunks)src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts(5 hunks)src/main/presenter/llmProviderPresenter/index.ts(1 hunks)src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/acpProvider.ts(1 hunks)src/main/presenter/threadPresenter/handlers/utilityHandler.ts(1 hunks)src/shared/types/presenters/legacy.presenters.d.ts(2 hunks)src/shared/types/presenters/llmprovider.presenter.d.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (21)
**/*.{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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use EventBus from
src/main/eventbus.tsfor main-to-renderer communication, broadcasting events viamainWindow.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 insrc/main/with presenters inpresenter/(Window/Tab/Thread/Mcp/Config/LLMProvider) andeventbus.tsfor app events
Use the Presenter pattern in the main process for UI coordination
Files:
src/main/presenter/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.ts
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/main/presenter/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use OxLint for linting JavaScript and TypeScript files
Files:
src/main/presenter/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.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/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use EventBus for inter-process communication events
Files:
src/main/presenter/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/main/presenter/llmProviderPresenter/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
Define the standardized
LLMCoreStreamEventinterface 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,usageobject with token counts,stop_reason(tool_use | max_tokens | stop_sequence | error | complete), andimage_dataobject with Base64-encoded data and mimeType
Files:
src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.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/index.ts
src/main/presenter/llmProviderPresenter/index.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/index.ts: Insrc/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, maintainingconversationMessageshistory, callingprovider.coreStream()on each iteration, and controlling the loop usingneedContinueConversationandtoolCallCount(compared againstMAX_TOOL_CALLS)
Insrc/main/presenter/llmProviderPresenter/index.ts, listen for standardized events yielded bycoreStreamand handle them accordingly: buffer text content (currentContent), handletool_call_start/chunk/endevents by collecting tool details and callingpresenter.mcpPresenter.callTool, send frontend events viaeventBuswith tool call status, format tool results for the next LLM call, and setneedContinueConversation = true
Insrc/main/presenter/llmProviderPresenter/index.ts, handlereasoning,text,image_data, andusageevents by processing and forwarding them throughSTREAM_EVENTS.RESPONSEevents to the frontend
Insrc/main/presenter/llmProviderPresenter/index.ts, handlestopevents by checkingstop_reason: if'tool_use', add the buffered assistant message and prepare for the next loop iteration; otherwise, add the final assistant message and exit the loop
Files:
src/main/presenter/llmProviderPresenter/index.ts
**/*Provider**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/provider-guidelines.mdc)
**/*Provider**/index.ts: Output only discriminated unionLLMCoreStreamEventin Provider implementations, do not use single interface with optional fields
Use factory methodscreateStreamEvent.*to construct events in Provider implementations, avoid direct field pollution
Text events: emit multipletextchunks in arrival order
Reasoning events:reasoningis optional; if provided, ensure it contains the complete chain
Tool call events: strictly follow sequencetool_call_start → tool_call_chunk* → tool_call_end, ensuretool_call_idis required and stable
Stop events: emitstopat stream end withstop_reasonvalue from {tool_use,max_tokens,stop_sequence,error,complete}
Usage events: sendusageonce before or at stream end withprompt_tokens,completion_tokens, andtotal_tokens
Rate limit events: sendrate_limitevent when reaching limit threshold with fields {providerId,qpsLimit,currentQps,queueLength,estimatedWaitTime?}, do not block event channel
Error handling: useerrorevent uniformly to carry error messages, avoid mixing errors into other event fields
Error termination: after fatal error occurs, emitstopevent as needed and terminate the stream
Image events:image_dataevent must providedata(Base64 encoded) andmimeTypefields; control single frame size and frequency to avoid blocking
Do not emitAssistantMessageBlockor any UI types from Provider implementations to UI layer
Do not introduce renderer dependencies inside Provider implementations
Every event construction in Provider implementations must use factory functions
Tool call IDs in Provider implementations must be globally unique and stable, with chunks arriving strictly in order
Error scenarios in Provider implementations must have correspondingstop_reasonanderrormessage
Provider implementations must emit at least oneusageevent if the provider has statistics capability
Providerate_limitevents in Provider implementations...
Files:
src/main/presenter/llmProviderPresenter/index.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main/presenter/llmProviderPresenter/providers/*.ts: Each LLM provider must implement thecoreStreammethod following the standardized event interface for tool calling and response streaming
Convert MCP tools to provider-specific formats and normalize streaming responses to standard events in each provider implementation
src/main/presenter/llmProviderPresenter/providers/*.ts: In Provider implementations (src/main/presenter/llmProviderPresenter/providers/*.ts), thecoreStream(messages, modelId, temperature, maxTokens)method should perform a single-pass streaming API request for each conversation round without containing multi-turn tool call loop logic
In Provider implementations, handle native tool support by converting MCP tools to Provider format usingconvertToProviderToolsand including them in the API request; for Providers without native function call support, prepare messages usingprepareFunctionCallPromptbefore making the API call
In Provider implementations, parse Provider-specific data chunks from the streaming response andyieldstandardizedLLMCoreStreamEventobjects conforming to the standard stream event interface, including text, reasoning, tool calls, usage, errors, stop reasons, and image data
In Provider implementations, include helper methods for Provider-specific operations such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPrompt
Files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
src/shared/**/*.d.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Define type definitions in shared/*.d.ts files for objects exposed by the main process to the renderer process
Files:
src/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/shared/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Shared type definitions and utilities between main and renderer processes should be placed in
src/shared
Files:
src/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Shared types and utilities should be placed in
src/shared/
Files:
src/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
🧠 Learnings (22)
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/presenter/mcpPresenter/**/*.ts : Register new MCP tools in `mcpPresenter/index.ts` after implementing them in `inMemoryServers/`
Applied to files:
src/main/presenter/lifecyclePresenter/hooks/index.tssrc/main/presenter/configPresenter/index.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, include helper methods for Provider-specific operations such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt`
Applied to files:
src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
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/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, handle native tool support by converting MCP tools to Provider format using `convertToProviderTools` and including them in the API request; for Providers without native function call support, prepare messages using `prepareFunctionCallPrompt` before making the API call
Applied to files:
src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:28:20.513Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.513Z
Learning: Use a two-layer LLM provider architecture with Agent Loop and Provider layers
Applied to files:
src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:27:39.200Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/provider-guidelines.mdc:0-0
Timestamp: 2025-11-25T05:27:39.200Z
Learning: Applies to **/*Provider**/index.ts : Do not introduce renderer dependencies inside Provider implementations
Applied to files:
src/main/presenter/configPresenter/index.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
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/main/presenter/configPresenter/index.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.ts
📚 Learning: 2025-11-25T05:28:20.513Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.513Z
Learning: Applies to src/main/**/*.ts : Use the Presenter pattern in the main process for UI coordination
Applied to files:
src/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/presenter/**/*.ts : Organize core business logic into dedicated Presenter classes, with one presenter per functional domain
Applied to files:
src/main/presenter/configPresenter/index.ts
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Use provide/inject for dependency injection when appropriate
Applied to files:
src/main/presenter/configPresenter/index.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each LLM provider must implement the `coreStream` method following the standardized event interface for tool calling and response streaming
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, parse Provider-specific data chunks from the streaming response and `yield` standardized `LLMCoreStreamEvent` objects conforming to the standard stream event interface, including text, reasoning, tool calls, usage, errors, stop reasons, and image data
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/index.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/**/*.ts : 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
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations (`src/main/presenter/llmProviderPresenter/providers/*.ts`), the `coreStream(messages, modelId, temperature, maxTokens)` method should perform a *single-pass* streaming API request for each conversation round without containing multi-turn tool call loop logic
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/threadPresenter/handlers/utilityHandler.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts`, handle `stop` events by checking `stop_reason`: if `'tool_use'`, add the buffered assistant message and prepare for the next loop iteration; otherwise, add the final assistant message and exit the loop
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Implement the two-layer LLM provider architecture with Agent Loop Layer managing conversation flow and Provider Layer handling API interactions
Applied to files:
src/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
📚 Learning: 2025-08-28T12:01:03.917Z
Learnt from: hllshiro
Repo: ThinkInAIXYZ/deepchat PR: 803
File: src/main/presenter/trayPresenter.ts:0-0
Timestamp: 2025-08-28T12:01:03.917Z
Learning: In the DeepChat application, calling app.quit() directly will automatically trigger the LifecycleManager's shutdown handling through Electron's before-quit event interception, so explicit calls to LifecycleManager.requestShutdown() are not needed when simply quitting the application.
Applied to files:
src/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.tssrc/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts
📚 Learning: 2025-11-25T05:26:24.867Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:26:24.867Z
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:39.200Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/provider-guidelines.mdc:0-0
Timestamp: 2025-11-25T05:27:39.200Z
Learning: Applies to **/*Provider**/index.ts : Output only discriminated union `LLMCoreStreamEvent` in Provider implementations, do not use single interface with optional fields
Applied to files:
src/shared/types/presenters/legacy.presenters.d.ts
🧬 Code graph analysis (2)
src/main/presenter/configPresenter/index.ts (1)
src/main/presenter/index.ts (1)
presenter(229-229)
src/main/presenter/lifecyclePresenter/hooks/beforeQuit/acpCleanupHook.ts (3)
src/main/presenter/lifecyclePresenter/hooks/index.ts (1)
acpCleanupHook(18-18)src/shared/types/presenters/legacy.presenters.d.ts (2)
LifecycleHook(2160-2166)LifecycleContext(2151-2155)src/main/presenter/index.ts (1)
presenter(229-229)
⏰ 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 (5)
src/main/presenter/lifecyclePresenter/hooks/index.ts (1)
18-18: Wire ACP cleanup hook into lifecycle exportsExporting
acpCleanupHookalongside otherbeforeQuithooks looks correct and keeps lifecycle registration centralized.src/main/presenter/llmProviderPresenter/index.ts (1)
148-150: Non-creating provider instance getter is well-factored
getExistingProviderInstancecleanly exposes the manager’s cached instance without side effects, which is exactly what the shutdown/cleanup path needs.src/shared/types/presenters/llmprovider.presenter.d.ts (1)
139-146: NewgetExistingProviderInstanceAPI is consistent with implementationAdding
getExistingProviderInstance(providerId: string): unknowntoILlmProviderPresentercleanly reflects the non-creating lookup thatLLMProviderPresenternow exposes, and matches its usage from lifecycle hooks.src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.ts (1)
221-223: Expose cached provider instance without side effects
getExistingProviderInstancecorrectly returns the cached instance fromproviderInstanceswithout instantiating anything new, which is ideal for shutdown and diagnostic paths.src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (1)
1125-1144: ACP provider cleanup correctly tears down sessions, processes, and pending permissionsThe cleanup sequence (clearAllSessions → processManager.shutdown → cancelling pending permissions) is coherent and matches the shutdown goal without obvious race or correctness issues. Map iteration with in-loop deletes is safe for
Map, and resolving permissions as cancelled is a good default at shutdown.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/threadPresenter/utils/promptBuilder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{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/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use EventBus from
src/main/eventbus.tsfor main-to-renderer communication, broadcasting events viamainWindow.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 insrc/main/with presenters inpresenter/(Window/Tab/Thread/Mcp/Config/LLMProvider) andeventbus.tsfor app events
Use the Presenter pattern in the main process for UI coordination
Files:
src/main/presenter/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.ts
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/main/presenter/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use OxLint for linting JavaScript and TypeScript files
Files:
src/main/presenter/threadPresenter/utils/promptBuilder.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/threadPresenter/utils/promptBuilder.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use EventBus for inter-process communication events
Files:
src/main/presenter/threadPresenter/utils/promptBuilder.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: hllshiro
Repo: ThinkInAIXYZ/deepchat PR: 803
File: src/main/presenter/trayPresenter.ts:0-0
Timestamp: 2025-08-28T12:01:03.917Z
Learning: In the DeepChat application, calling app.quit() directly will automatically trigger the LifecycleManager's shutdown handling through Electron's before-quit event interception, so explicit calls to LifecycleManager.requestShutdown() are not needed when simply quitting the application.
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts`, handle `stop` events by checking `stop_reason`: if `'tool_use'`, add the buffered assistant message and prepare for the next loop iteration; otherwise, add the final assistant message and exit the loop
Applied to files:
src/main/presenter/threadPresenter/utils/promptBuilder.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/threadPresenter/utils/promptBuilder.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/threadPresenter/utils/promptBuilder.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, handle native tool support by converting MCP tools to Provider format using `convertToProviderTools` and including them in the API request; for Providers without native function call support, prepare messages using `prepareFunctionCallPrompt` before making the API call
Applied to files:
src/main/presenter/threadPresenter/utils/promptBuilder.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/threadPresenter/utils/promptBuilder.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations (`src/main/presenter/llmProviderPresenter/providers/*.ts`), the `coreStream(messages, modelId, temperature, maxTokens)` method should perform a *single-pass* streaming API request for each conversation round without containing multi-turn tool call loop logic
Applied to files:
src/main/presenter/threadPresenter/utils/promptBuilder.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, include helper methods for Provider-specific operations such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt`
Applied to files:
src/main/presenter/threadPresenter/utils/promptBuilder.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)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| getProviderById(id: string): LLM_PROVIDER | ||
| isAgentProvider(providerId: string): boolean | ||
| getProviderInstance?(providerId: string): unknown | ||
| getExistingProviderInstance?(providerId: string): unknown |
There was a problem hiding this comment.
Remove conflicting getProviderInstance signature
The legacy ILlmProviderPresenter interface now declares getProviderInstance twice—once as optional in this block and again as a required method later in the same interface (around line 945). TypeScript treats this as duplicate property declarations with incompatible optionality, which will surface as a type error (e.g., “Subsequent property declarations must have the same type”) when loading the declaration file, breaking pnpm run typecheck or any build that includes these types.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.ts (1)
165-174: Preserve original error cause + make shutdown detection less brittle
Mapping shutdown errors is good, butmessage.includes('shutting down')is fragile and you’re dropping the original error context. Consider (a) extracting a sharedisAcpShuttingDownError(err)helper (used here + AcpProvider), and (b) rethrowing withcauseso logs keep stack/context.try { handle = await this.processManager.getConnection(agent, workdir) } catch (error) { const message = error instanceof Error ? error.message : String(error) - if (message.includes('shutting down')) { - throw new Error('[ACP] Cannot create session: process manager is shutting down') + if (message.toLowerCase().includes('shutting down')) { + throw new Error('[ACP] Cannot create session: process manager is shutting down', { + cause: error instanceof Error ? error : undefined + }) } throw error }test/main/presenter/acpSessionManager.test.ts (1)
1-38: Good coverage for shutdown-vs-non-shutdown getConnection failures; consider avoiding private-method testing
These assertions match the new mapping behavior. The main nit is reaching intoprivate createSessionviaas any; if feasible, drive this throughgetOrCreateSession()(or make the mapping a small exported helper to test directly). Also consider using a platform-neutral temp path string (oros.tmpdir()), even if it’s “just a string”.test/main/presenter/llmProviderPresenter.test.ts (1)
544-572: Make console spy restoration failure-safe (and optionally assert warn behavior)
If the first test throws before Line 558,warnSpy.mockRestore()won’t run. Prefertry/finally(or anafterEach) so the suite doesn’t leak mocks.it('should swallow ACP warmup shutdown errors', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const mockAcpProvider = { - warmupProcess: vi - .fn() - .mockRejectedValue(new Error('[ACP] Process manager is shutting down, refusing to spawn')) - } - vi.spyOn(llmProviderPresenter as any, 'getAcpProviderInstance').mockReturnValue( - mockAcpProvider as any - ) - - await expect( - llmProviderPresenter.warmupAcpProcess('agent-test', '/tmp') - ).resolves.toBeUndefined() - warnSpy.mockRestore() + try { + const mockAcpProvider = { + warmupProcess: vi + .fn() + .mockRejectedValue(new Error('[ACP] Process manager is shutting down, refusing to spawn')) + } + vi.spyOn(llmProviderPresenter as any, 'getAcpProviderInstance').mockReturnValue( + mockAcpProvider as any + ) + await expect(llmProviderPresenter.warmupAcpProcess('agent-test', '/tmp')).resolves.toBeUndefined() + } finally { + warnSpy.mockRestore() + } })test/main/presenter/acpProvider.test.ts (1)
1-82: Nice targeted tests for shutdown mapping; consider a less “prototype-only” instance setup
Coverage looks solid for the new error-handling split. Optional: if you can cheaply instantiateAcpProviderwith minimal fakes (vsObject.create(...prototype)+as any), it’ll better reflect runtime wiring and reduce “false green” risk. Also, consider replacing'/tmp'with a platform-neutral temp dir string.src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (2)
461-475: De-duplicate “shutting down” detection + make it case-insensitive
This is now the second place matching on'shutting down'(also insrc/main/presenter/llmProviderPresenter/agent/acpSessionManager.ts). Suggest extracting a shared helper (even a small local util module) and usingtoLowerCase()to avoid casing issues.- if (message.includes('shutting down')) { + if (message.toLowerCase().includes('shutting down')) { return { status: 'error', sessionId: undefined, error: 'Process manager is shutting down', events: [] } }
1139-1157: Cleanup() is good; consider snapshot+clear and idempotency guard
Two small robustness tweaks:
- Avoid mutating
pendingPermissionswhile iterating by snapshotting, thenclear().- Consider guarding cleanup from re-entry (shutdown hooks can sometimes fire multiple times).
- for (const [requestId, state] of this.pendingPermissions.entries()) { - state.resolve({ outcome: { outcome: 'cancelled' } }) - this.pendingPermissions.delete(requestId) - } + const pending = Array.from(this.pendingPermissions.values()) + this.pendingPermissions.clear() + for (const state of pending) { + state.resolve({ outcome: { outcome: 'cancelled' } }) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.ts(1 hunks)src/main/presenter/llmProviderPresenter/index.ts(2 hunks)src/main/presenter/llmProviderPresenter/providers/acpProvider.ts(3 hunks)test/main/presenter/acpProvider.test.ts(1 hunks)test/main/presenter/acpSessionManager.test.ts(1 hunks)test/main/presenter/llmProviderPresenter.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/presenter/llmProviderPresenter/index.ts
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{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/acpSessionManager.tstest/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.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/acpSessionManager.tstest/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.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/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use EventBus from
src/main/eventbus.tsfor main-to-renderer communication, broadcasting events viamainWindow.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 insrc/main/with presenters inpresenter/(Window/Tab/Thread/Mcp/Config/LLMProvider) andeventbus.tsfor app events
Use the Presenter pattern in the main process for UI coordination
Files:
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.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/acpSessionManager.tstest/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.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/acpSessionManager.tstest/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.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/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
src/main/presenter/llmProviderPresenter/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
Define the standardized
LLMCoreStreamEventinterface 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,usageobject with token counts,stop_reason(tool_use | max_tokens | stop_sequence | error | complete), andimage_dataobject with Base64-encoded data and mimeType
Files:
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.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/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.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/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.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/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.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/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use EventBus for inter-process communication events
Files:
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
test/**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest framework for unit and integration tests
Files:
test/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tstest/main/presenter/llmProviderPresenter.test.ts
test/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*.{test,spec}.ts: Test files should follow the same directory structure as source code undertest/mainandtest/rendererdirectories
Use Vitest and Vue Test Utils for testing with jsdom configuration
Test files must be named with.test.tsor.spec.tsextension
Files:
test/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tstest/main/presenter/llmProviderPresenter.test.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main/presenter/llmProviderPresenter/providers/*.ts: Each LLM provider must implement thecoreStreammethod following the standardized event interface for tool calling and response streaming
Convert MCP tools to provider-specific formats and normalize streaming responses to standard events in each provider implementation
src/main/presenter/llmProviderPresenter/providers/*.ts: In Provider implementations (src/main/presenter/llmProviderPresenter/providers/*.ts), thecoreStream(messages, modelId, temperature, maxTokens)method should perform a single-pass streaming API request for each conversation round without containing multi-turn tool call loop logic
In Provider implementations, handle native tool support by converting MCP tools to Provider format usingconvertToProviderToolsand including them in the API request; for Providers without native function call support, prepare messages usingprepareFunctionCallPromptbefore making the API call
In Provider implementations, parse Provider-specific data chunks from the streaming response andyieldstandardizedLLMCoreStreamEventobjects conforming to the standard stream event interface, including text, reasoning, tool calls, usage, errors, stop reasons, and image data
In Provider implementations, include helper methods for Provider-specific operations such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPrompt
Files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
🧠 Learnings (20)
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/acpSessionManager.tstest/main/presenter/acpProvider.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : In `src/main/presenter/llmProviderPresenter/index.ts`, handle `stop` events by checking `stop_reason`: if `'tool_use'`, add the buffered assistant message and prepare for the next loop iteration; otherwise, add the final assistant message and exit the loop
Applied to files:
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
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/acpSessionManager.tstest/main/presenter/acpProvider.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:26:24.867Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:26:24.867Z
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:
test/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
📚 Learning: 2025-11-25T05:28:20.513Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.513Z
Learning: Applies to 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
Applied to files:
test/main/presenter/acpProvider.test.tstest/main/presenter/acpSessionManager.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/presenter/mcpPresenter/**/*.ts : Register new MCP tools in `mcpPresenter/index.ts` after implementing them in `inMemoryServers/`
Applied to files:
test/main/presenter/acpProvider.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, include helper methods for Provider-specific operations such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt`
Applied to files:
test/main/presenter/acpProvider.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:28:20.513Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.513Z
Learning: Applies to src/main/**/*.ts : Use the Presenter pattern in the main process for UI coordination
Applied to files:
test/main/presenter/acpProvider.test.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, handle native tool support by converting MCP tools to Provider format using `convertToProviderTools` and including them in the API request; for Providers without native function call support, prepare messages using `prepareFunctionCallPrompt` before making the API call
Applied to files:
test/main/presenter/acpProvider.test.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:27:39.200Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/provider-guidelines.mdc:0-0
Timestamp: 2025-11-25T05:27:39.200Z
Learning: Applies to **/*Provider**/index.ts : Error scenarios in Provider implementations must have corresponding `stop_reason` and `error` message
Applied to files:
test/main/presenter/acpProvider.test.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx,js} : Use Vitest framework for unit and integration tests
Applied to files:
test/main/presenter/acpProvider.test.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
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/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, parse Provider-specific data chunks from the streaming response and `yield` standardized `LLMCoreStreamEvent` objects conforming to the standard stream event interface, including text, reasoning, tool calls, usage, errors, stop reasons, and image data
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.tstest/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:28:20.513Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T05:28:20.513Z
Learning: Applies to src/**/*.ts : Use EventBus for inter-process communication events
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/**/*.ts : Use EventBus from `src/main/eventbus.ts` for main-to-renderer communication, broadcasting events via `mainWindow.webContents.send()`
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
📚 Learning: 2025-11-25T05:26:24.867Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:26:24.867Z
Learning: Applies to src/main/**/*.ts : Use EventBus pattern for inter-process communication within the main process to decouple modules
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
📚 Learning: 2025-11-25T05:26:24.867Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:26:24.867Z
Learning: Implement proper error handling and logging for debugging in Electron applications
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations (`src/main/presenter/llmProviderPresenter/providers/*.ts`), the `coreStream(messages, modelId, temperature, maxTokens)` method should perform a *single-pass* streaming API request for each conversation round without containing multi-turn tool call loop logic
Applied to files:
test/main/presenter/llmProviderPresenter.test.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each LLM provider must implement the `coreStream` method following the standardized event interface for tool calling and response streaming
Applied to files:
test/main/presenter/llmProviderPresenter.test.ts
🧬 Code graph analysis (3)
src/main/presenter/llmProviderPresenter/agent/acpSessionManager.ts (1)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (1)
AcpProcessHandle(24-35)
test/main/presenter/acpProvider.test.ts (1)
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (1)
AcpProvider(58-1158)
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (1)
src/main/presenter/llmProviderPresenter/agent/acpProcessManager.ts (1)
AcpProcessHandle(24-35)
⏰ 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 (1)
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (1)
28-28: Good: typed handle import improves safety/readability
Usingtype AcpProcessHandlehere clarifies whatgetConnection()returns.
* fix: Multiple Issues Affecting Conversation Flow and Model Settings (#1166) * fix: #1164 support maxtoken 2 * fix: mcp tool panel close btn #1163 * fix: #1162 file content in converation * fix(search-assistant): exclude acp models * fix: #1072 thinkcontent respects the global font size set * feat: Hebrew (he-IL) Translation (#1157) * feat: Hebrew (he-IL) Translation * feat: add workspace view to acp agents (#1158) * feat: add workspaceview for acp agent * feat: support workspace dirs * fix: workspace file refresh * fix: tool call status * fix: review refactor * chore: update readme * fix: add file context actions (#1160) * fix: keep last file list * feat(acp-workspace): add file context actions * fix(acp-workspace): move path helpers to preload * fix(preload): allow empty relative path * fix(preload): escape quotes when formatting paths * chore: update docs * fix: Multiple Issues Affecting Conversation Flow and Model Settings (#1166) * fix: #1164 support maxtoken 2 * fix: mcp tool panel close btn #1163 * fix: #1162 file content in converation * fix(search-assistant): exclude acp models * fix: #1072 thinkcontent respects the global font size set * feat: add new i18n translation * feat: add custom font setting (#1167) * feat(settings): add font customization controls * feat: change font with monaco * chore: remove log and remove unuse key * fix: linux font parse * feat: use font-list to get font * fix: font setting not work on settings page (#1169) * style: unify scroll bar style (#1173) * feat: acp init and process manage (#1171) * feat: init acp process on select * feat: warm up acp process on new thread * fix: reuse warmup process * fix: vue warning * chore: add plan for acp debug panel * feat: add debugview for acp * feat: add i18n for debug * fix: code review * fix: ai review * fix: Shutdown may skip releasing warmup‑only processes due to using warmupKey instead of agentId. * chore: update markdown renderer * chore: update mermaid node * Merge commit from fork * chore: update markstream-vue to version 0.0.3-beta.3 fix link renderer feat html_inline render * fix: increase button size for web content limit adjustment * fix: close app kill all acp processes (#1175) * fix: close app kill all acp processes * fix: disable tool call merge * fix: handle uncatch error * fix: remove redundant type * feat: add shell to powerpack (#1178) * chore(powerpack): randomize shell workdir * feat: exclusive inmem server in terminal display * fix: add sandbox in shell script --------- Co-authored-by: xiaomo <wegi866@gmail.com> Co-authored-by: Simon He <674949287@qq.com> Co-authored-by: Simon He <57086651+Simon-He95@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.