feat: large tool call offload #1276
Conversation
📝 WalkthroughWalkthroughImplements tool-output guardrails: directory_tree depth limits, per-conversation session-scoped file access, offloading long tool outputs (>3000 chars) to per-session files with model-facing stubs, permission_request propagation with new permission-related tool_call variants, and improved error text propagation. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent Presenter
participant Processor as ToolCallProcessor
participant FS as Session FS
participant LLM as LLM / Model
Agent->>Processor: Tool completes (toolCallId, conversationId, output)
Processor->>Processor: Measure output length (>3000?)
alt Exceeds threshold
Processor->>FS: Write full output to ~/.deepchat/sessions/<conversationId>/tool_<toolCallId>.offload
Processor->>Processor: Build stub (total chars, preview, file path)
Processor->>Agent: Insert stub into tool_call_response (tool_call_response_raw unchanged)
Agent->>LLM: Model receives stub with offload path
else Within threshold
Processor->>Agent: Insert full content into tool_call_response
Agent->>LLM: Model receives full content
end
Note right of LLM: Model may request file via session-scoped file tool if permitted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used📓 Path-based instructions (19)**/*.{js,ts,tsx,jsx,vue,mjs,cjs}📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Files:
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Files:
src/renderer/src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Files:
src/renderer/src/**/*.vue📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Files:
src/renderer/src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Files:
src/renderer/src/components/**/*.vue📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Files:
**/*.vue📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/renderer/**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{js,ts,tsx,jsx,mjs,cjs}📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Files:
src/main/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/main/presenter/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{src/main,src/renderer,test}/**/*.{ts,tsx,js}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/renderer/src/stores/**/*.ts📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Files:
src/renderer/src/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Files:
src/renderer/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Files:
src/shared/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-01-13T09:23:07.415ZApplied to files:
📚 Learning: 2025-06-21T15:49:17.044ZApplied to files:
🧬 Code graph analysis (2)src/renderer/src/stores/chat.ts (2)
src/main/presenter/agentPresenter/streaming/streamUpdateScheduler.ts (1)
🔇 Additional comments (17)
✏️ Tip: You can disable this entire section by setting 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/presenter/agentPresenter/loop/errorClassification.ts (1)
10-14: Docstring is inconsistent with the new behavior.The function docstring still lists "Invalid input format (invalid URL, malformed JSON, etc.)" and "Schema validation failures" as non-retryable errors, but the implementation now returns
false(retryable) for JSON and schema validation errors at lines 44 and 58. Update the documentation to reflect the actual behavior.📝 Suggested docstring update
/** * Checks if an error is non-retryable (should stop the agent loop). * * Non-retryable errors are those that won't be resolved by retrying: - * - Invalid input format (invalid URL, malformed JSON, etc.) + * - Invalid URL format * - Explicit permission denied - * - Schema validation failures * - Authentication errors that can't be resolved by retry * - Malformed requests that won't work on retry + * - Tool definition not found + * + * Retryable errors (LLM may correct on retry): + * - Invalid/malformed JSON (LLM can fix argument formatting) + * - Schema validation failures (LLM can correct parameters) *src/main/presenter/agentPresenter/index.ts (1)
235-242: Error messages should be in English per coding guidelines.Lines 236 and 241 contain Chinese error messages. As per coding guidelines, all logs and comments (including error messages) must be in English for TypeScript files.
📝 Suggested fix
if (message.role !== 'assistant') { - throw new Error('只能重试助手消息') + throw new Error('Can only retry assistant messages') } const userMessage = await this.messageManager.getMessage(message.parentId || '') if (!userMessage) { - throw new Error('找不到对应的用户消息') + throw new Error('Could not find corresponding user message') }
🤖 Fix all issues with AI agents
In `@docs/specs/tool-output-guardrails/plan.md`:
- Line 29: The document has an internal inconsistency: the file name is listed
as `tool_<toolCallId>.offload` on line 29 but as a `.txt` file on line 69;
standardize the extension to match spec.md by choosing one extension (either
`.offload` or `.txt`), update every occurrence of `tool_<toolCallId>.*` in this
document to that extension, and ensure the chosen extension aligns with the
filename and examples used in spec.md.
In `@docs/specs/tool-output-guardrails/spec.md`:
- Around line 51-52: Standardize the offload file extension between the two
docs: update the example in spec.md that currently shows `tool_<toolCallId>.txt`
to use the same extension used in plan.md (change to `tool_<toolCallId>.offload`
or vice versa), and ensure both spec.md and plan.md consistently reference that
single extension everywhere.
In `@src/main/presenter/agentPresenter/streaming/streamUpdateScheduler.ts`:
- Line 257: The flushAll method is missing the permission_request field so
pending permission-request deltas get dropped; update the eventData assembly in
flushAll (mirror how flushRender constructs eventData) to include
permission_request: delta.permission_request (or the same source used in
flushRender), ensuring the permission_request from the current delta is passed
through when emitting or sending the event.
🧹 Nitpick comments (3)
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts (1)
1-95: Well-structured test for tool output offloading.The test properly validates the offload behavior:
- Creates a temporary directory and mocks
app.getPath- Uses 3001 characters to exceed the 3000-char threshold
- Verifies stub content, file path, raw data preservation, and conversation message updates
- Cleans up resources in
afterEachOne minor suggestion: consider using a typed array instead of
any[]on line 57 for better type safety.🔧 Optional type improvement
- const events: any[] = [] + const events: Array<{ type: string; data?: Record<string, unknown> }> = []test/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts (1)
134-136: Validates max depth enforcement.The test correctly verifies that requesting depth beyond the maximum (3) results in rejection. No shared constant currently exists for the max depth value in agentFileSystemHandler—it's hardcoded in the DirectoryTreeArgsSchema. Extracting this to a named constant would improve maintainability and allow the test to reference it directly instead of hardcoding
depth: 4.src/main/presenter/agentPresenter/loop/toolCallProcessor.ts (1)
375-399: Solid implementation with graceful error handling.The offload logic is well-structured with proper fallback behavior. One minor consideration:
The
toolCallIdsanitization (line 386) only removes path separators (/,\). While tool call IDs are typically safe UUIDs, for defense-in-depth, you could use a more comprehensive sanitization to handle any unexpected characters that might be problematic on different file systems.🔧 Optional: More robust filename sanitization
- const safeToolCallId = toolCallId.replace(/[\\/]/g, '_') + const safeToolCallId = toolCallId.replace(/[<>:"/\\|?*\x00-\x1f]/g, '_')
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
docs/specs/tool-output-guardrails/plan.mddocs/specs/tool-output-guardrails/spec.mddocs/specs/tool-output-guardrails/tasks.mdsrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/index.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tssrc/main/presenter/toolPresenter/index.tssrc/renderer/src/stores/chat.tssrc/shared/types/core/agent-events.tssrc/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tstest/main/presenter/sessionPresenter/permissionHandler.test.ts
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
All logs and comments must be in English
Files:
src/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/shared/types/core/agent-events.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/renderer/src/stores/chat.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use OxLint as the linter
Files:
src/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/shared/types/core/agent-events.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/renderer/src/stores/chat.ts
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use Prettier as the code formatter
Files:
src/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/shared/types/core/agent-events.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/renderer/src/stores/chat.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Shared TypeScript types and utilities should be placed in
src/shared/
Files:
src/shared/types/presenters/tool.presenter.d.tssrc/shared/types/core/agent-events.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use camelCase for variable and function names; use PascalCase for types and classes; use SCREAMING_SNAKE_CASE for constants
Configure Prettier with single quotes, no semicolons, and line width of 100 characters. Runpnpm run formatafter completing features
Files:
src/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/shared/types/core/agent-events.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/renderer/src/stores/chat.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use OxLint for linting JavaScript and TypeScript files; ensure lint-staged hooks and typecheck pass before commits
Enable strict TypeScript type checking
Files:
src/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/shared/types/core/agent-events.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/renderer/src/stores/chat.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/shared/types/core/agent-events.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/renderer/src/stores/chat.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Vitest test suites should be organized in
test/main/**andtest/renderer/**mirroring source structure, with file names following*.test.tsor*.spec.tspattern
Files:
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.tstest/main/presenter/sessionPresenter/permissionHandler.test.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts
{src/main,src/renderer,test}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use IPC communication: Renderer calls main process via
usePresentercomposable, Main sends to Renderer via EventBus
Files:
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/renderer/src/stores/chat.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write tests using Vitest with separate test suites for main and renderer processes
Files:
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.tstest/main/presenter/sessionPresenter/permissionHandler.test.tstest/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Electron main process code should reside in
src/main/, with presenters organized inpresenter/subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed viaeventbus.ts
Files:
src/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement all system capabilities as main-process presenters following the Presenter Pattern
Files:
src/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/agentPresenter/index.tssrc/main/presenter/agentPresenter/streaming/streamUpdateScheduler.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/loop/errorClassification.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts
src/renderer/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*.{vue,ts,tsx}: Use vue-i18n framework for internationalization located at src/renderer/src/i18n/
All user-facing strings must use i18n keys, not hardcoded text
src/renderer/src/**/*.{vue,ts,tsx}: Usereffor primitives and references,reactivefor objects in Vue 3 Composition API
Prefercomputedproperties over methods for derived state in Vue components
Import Shadcn Vue components from@/shadcn/components/ui/path alias
Use thecn()utility function combining clsx and tailwind-merge for dynamic Tailwind classes
UsedefineAsyncComponent()for lazy loading heavy Vue components
Use TypeScript for all Vue components and composables with explicit type annotations
Define TypeScript interfaces for Vue component props and data structures
UseusePresentercomposable for main process communication instead of direct IPC calls
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Ensure all code comments are in English and all log messages are in English, with no non-English text in code comments or console statements
Use VueUse composables for common utilities like
useLocalStorage,useClipboard,useDebounceFnVue 3 renderer app code should be organized in
src/renderer/srcwith subdirectories forcomponents/,stores/,views/,i18n/, andlib/
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/stores/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
src/renderer/src/stores/**/*.ts: Use Setup Store syntax withdefineStorefunction pattern in Pinia stores
Use getters (computed properties) for derived state in Pinia stores
Keep Pinia store actions focused on state mutations and async operations
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Use class-variance-authority (CVA) for defining component variants with Tailwind classes
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
src/renderer/src/**/*.{ts,tsx}: UseshallowRefandshallowReactivefor optimizing reactivity with large objects
Prefertypeoverinterfacein TypeScript unless using inheritance withextends
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
All user-facing strings must use i18n keys (supports 12 languages)
Files:
src/renderer/src/stores/chat.ts
🧠 Learnings (6)
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/**/*.ts : Implement all system capabilities as main-process presenters following the Presenter Pattern
Applied to files:
src/shared/types/presenters/tool.presenter.d.tssrc/main/presenter/toolPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tssrc/main/presenter/agentPresenter/message/messageBuilder.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/mcpPresenter/inMemoryServers/**/*.ts : Implement new MCP tools in the inMemoryServers directory and register in mcpPresenter/index.ts
Applied to files:
src/shared/types/presenters/tool.presenter.d.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/toolPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.tssrc/main/presenter/agentPresenter/tool/toolCallCenter.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/acp/agentToolManager.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to **/*.test.ts : Write tests using Vitest with separate test suites for main and renderer processes
Applied to files:
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Implement `coreStream` method following standardized event interface for LLM providers
Applied to files:
src/main/presenter/agentPresenter/streaming/llmEventHandler.tssrc/main/presenter/agentPresenter/index.tstest/main/presenter/sessionPresenter/permissionHandler.test.ts
📚 Learning: 2026-01-05T02:41:45.219Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Project uses Node.js ≥ 20.19 and pnpm ≥ 10.11 (pnpm only); Windows developers must enable Developer Mode for symlinks
Applied to files:
src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts
📚 Learning: 2025-06-21T15:49:17.044Z
Learnt from: neoragex2002
Repo: ThinkInAIXYZ/deepchat PR: 550
File: src/renderer/src/stores/chat.ts:1011-1035
Timestamp: 2025-06-21T15:49:17.044Z
Learning: In src/renderer/src/stores/chat.ts, the user prefers to keep both `text` and `content` properties in the `handleMeetingInstruction` function's `sendMessage` call, even though they are redundant, rather than removing the `content` property.
Applied to files:
src/renderer/src/stores/chat.ts
🧬 Code graph analysis (8)
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts (1)
src/main/presenter/agentPresenter/loop/toolCallProcessor.ts (1)
ToolCallProcessor(46-502)
src/main/presenter/toolPresenter/index.ts (1)
test/mocks/electron.ts (1)
app(2-10)
src/main/presenter/agentPresenter/index.ts (1)
src/main/eventbus.ts (1)
eventBus(151-151)
src/main/presenter/agentPresenter/streaming/streamUpdateScheduler.ts (1)
src/shared/types/core/agent-events.ts (1)
LLMAgentEventData(4-60)
src/main/presenter/agentPresenter/message/messageBuilder.ts (2)
src/main/presenter/agentPresenter/tool/toolCallCenter.ts (1)
ToolCallCenter(15-29)src/main/presenter/index.ts (1)
presenter(334-334)
src/main/presenter/agentPresenter/acp/agentToolManager.ts (1)
src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts (1)
AgentFileSystemHandler(162-1199)
src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts (1)
test/mocks/electron.ts (1)
app(2-10)
src/renderer/src/stores/chat.ts (2)
src/shared/chat/messageBlocks.ts (1)
finalizeAssistantMessageBlocks(3-25)src/shared/chat.d.ts (2)
AssistantMessage(39-42)UserMessage(34-37)
⏰ 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 (39)
src/main/presenter/agentPresenter/loop/errorClassification.ts (1)
38-59: LGTM! Making JSON and schema errors retryable is a reasonable design choice.Allowing the LLM to retry when it produces malformed JSON or incorrect parameters gives it an opportunity to self-correct. This aligns well with the agent loop's error recovery strategy.
docs/specs/tool-output-guardrails/tasks.md (1)
1-12: LGTM!The task breakdown is well-structured and aligns with the spec and plan documents.
docs/specs/tool-output-guardrails/spec.md (1)
1-59: LGTM!The spec clearly defines acceptance criteria for error presentation, directory_tree depth control, and tool output offload. The depth semantics (root=0) and offload threshold (>3000 chars) are well documented.
docs/specs/tool-output-guardrails/plan.md (1)
81-87: Good risk mitigation coverage.The fallback behavior for missing
conversationId(truncate without offload) is a sensible degradation strategy. Consider documenting the truncation length for this fallback case.src/main/presenter/agentPresenter/index.ts (3)
173-182: LGTM! Consistent error propagation pattern.The error extraction and propagation to the renderer is well implemented. The pattern
error instanceof Error ? error.message : String(error)handles both Error instances and string errors gracefully.
203-212: LGTM!Consistent error handling pattern with
sendMessage.
253-262: LGTM!Consistent error handling pattern with
sendMessageandcontinueLoop.src/main/presenter/agentPresenter/streaming/llmEventHandler.ts (1)
264-266: Correctly propagates permission_request to the stream delta.The condition
msg.permission_request !== undefinedproperly handles cases where permission_request might be a falsy but defined value (e.g., empty object), ensuring it's included in the delta for downstream handling.test/main/presenter/sessionPresenter/permissionHandler.test.ts (2)
112-114: Mock correctly updated to include new interface method.The
buildToolSystemPromptmock is properly added to align with the updatedIToolPresenterinterface.
228-230: Consistent mock update.Same mock addition as above, maintaining consistency across test helpers.
src/shared/types/presenters/tool.presenter.d.ts (1)
29-33: Clean interface extension for tool system prompt construction.The new method follows the established pattern in the interface with proper JSDoc documentation. The optional
conversationIdin the context object provides flexibility for different use cases.test/main/presenter/agentPresenter/acp/agentFileSystemHandler.test.ts (1)
94-132: Thorough depth-limiting test coverage.The test comprehensively validates depth behavior at levels 0, 1, and 2, correctly asserting:
- At depth 0: directories exist but have no children populated
- At depth 1: immediate children are populated, nested directories have no children
- At depth 2: two levels of children are populated
The nested directory structure setup and assertions are well-organized.
src/main/presenter/agentPresenter/tool/toolCallCenter.ts (1)
25-28: LGTM!The new method follows the established delegation pattern in this class, cleanly forwarding to
toolPresenter.buildToolSystemPrompt. Consistent with the existing architecture.src/main/presenter/agentPresenter/streaming/streamUpdateScheduler.ts (2)
20-20: LGTM!Type reference to
LLMAgentEventData['permission_request']ensures consistency with the shared type definition.
171-173: LGTM!Follows the established pattern for handling delta fields.
src/main/presenter/agentPresenter/message/messageBuilder.ts (3)
109-109: LGTM!The
isToolPromptModevariable correctly captures both 'agent' and 'acp agent' modes for determining when tool system prompts should be included.
121-121: LGTM!ToolCallCenter instantiation is appropriate here as it's reused for both
getAllToolDefinitionsand the newbuildToolSystemPromptcall.
162-167: LGTM!The guard condition correctly ensures the tool system prompt is only added when tools are available and the mode supports tool usage. The prompt is properly appended to the system prompt.
src/shared/types/core/agent-events.ts (1)
22-31: LGTM!The expanded
tool_callunion type correctly adds the permission flow states ('permission-granted', 'permission-denied') and 'continue' state. This is an additive, backward-compatible change that enables the new permission handling workflow.src/main/presenter/toolPresenter/index.ts (2)
9-10: LGTM!Necessary imports for path construction and accessing the home directory.
24-24: LGTM!Interface properly declares the new method signature.
src/main/presenter/agentPresenter/acp/agentToolManager.ts (3)
168-178: LGTM!The depth parameter schema is well-defined with appropriate constraints (integer, 0-3 range, default 1) to prevent excessive recursion while allowing useful directory tree exploration.
475-488: LGTM!The updated description correctly documents the new depth parameter capability, keeping user-facing documentation in sync with the schema changes.
620-623: LGTM! Session-scoped handler correctly instantiated.The
conversationIdis properly passed to enable per-conversation session file access restrictions during tool execution.Note: The instance-level
this.fileSystemHandlercreated ingetAllToolDefinitions(line 243) doesn't receiveconversationId, but this appears intentional since that handler is only used for workspace path management, not actual file operations with session-scoped access requirements.src/main/presenter/agentPresenter/loop/toolCallProcessor.ts (5)
9-11: LGTM!The new imports are appropriate for the file system offload functionality.
43-44: LGTM!The threshold (3000 chars) and preview length (1024 chars) are reasonable defaults for the offload mechanism.
401-412: LGTM! Path traversal protection is correctly implemented.The validation logic properly handles various traversal attempts:
..paths are correctly rejected since the resolved path won't start withsessionsRoot/.is rejected by the equality check- Embedded
../in paths likefoo/../barresolve safely within the sessions directoryGood defensive coding.
414-422: LGTM!The stub format is clear, informative, and provides sufficient context (total size, file path, preview) for both the LLM and users to understand the offloaded content.
186-233: LGTM!The offload integration is well-implemented:
- Both native and legacy function call paths consistently use
toolContentForModel- The original
toolResponse.rawDatais preserved intool_call_response_rawfor debugging/UI purposes- The flow correctly handles the async offload operation before proceeding
src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts (7)
92-95: LGTM!The
DirectoryTreeArgsSchemacorrectly mirrors the schema defined inagentToolManager.ts, ensuring consistency between validation and tool definition.
162-178: LGTM!The constructor properly initializes session-scoped access control with:
- Optional
conversationIdfor per-conversation restrictions- Computed
sessionsRootpath for session boundary enforcement
216-263: LGTM! Thorough security validation.The
validatePathmethod correctly performs session read checks both:
- On the initially resolved path (line 224)
- After symlink resolution (line 238)
This double-check prevents symlink-based bypass attacks where a link could point outside the allowed session directory.
265-286: LGTM! Session isolation is correctly enforced.The session read access control logic:
- Allows unrestricted access outside the sessions directory
- Requires an active conversation for any session file access
- Restricts access to only the current conversation's session directory
The path comparison correctly handles both exact directory matches and subdirectory access.
1046-1082: LGTM! Depth-controlled recursion correctly implemented.The depth parameter works as documented:
depth=0: Root level only (no directory recursion)depth=1: Root + one level of subdirectoriesdepth=3(max): Up to 3 levels deepThe
accessType: 'read'validation ensures session-scoped restrictions apply to directory tree operations.
548-551: LGTM!Search operations correctly apply
accessType: 'read'validation to enforce session-scoped access restrictions.
831-852: LGTM!Read operations (
readFile,listDirectory,grepSearch,getFileInfo,globSearch) consistently applyaccessType: 'read'validation, ensuring session-scoped restrictions are enforced across all file reading paths.src/renderer/src/stores/chat.ts (3)
892-928: LGTM!The extended
tool_callunion andpermission_requesttype definition comprehensively support the new permission workflow. The structure accommodates various permission scenarios (read/write/command) with appropriate metadata fields.
1033-1125: LGTM! Permission flow handling is well-structured.The implementation correctly:
- Creates permission request blocks with comprehensive metadata
- Uses reverse search to find the most recent matching permission block (important for nested tool calls)
- Updates block status appropriately for granted/denied/continue states
- Clears
needsUserActionflag when permission decisions are madeThe
JSON.stringifyfor nested objects inpermissionExtra(lines 1042-1044) ensures serialization compatibility with the message block system.
1688-1729: LGTM! Thread-aware message editing prevents cross-conversation pollution.The updated
handleMessageEditedcorrectly:
- Guards against missing context (lines 1691-1692, 1704)
- Filters out edits from other conversations (lines 1707-1709)
- Properly handles variant messages by updating the parent main message (lines 1712-1723)
- Updates view cache only for the active thread's messages
This prevents stale or incorrect message updates when multiple conversations are open.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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/main/presenter/agentPresenter/loop/toolCallProcessor.ts (1)
287-327: Double stringification bug in appendNativeFunctionCallMessages.The method receives
toolResponse: { content: unknown }wherecontentis already a string (fromtoolContentForModelwhich comes fromstringifyToolContent). However, line 321 callsstringifyToolContent(toolResponse.content)again. This will double-stringify content that's already a string, which is redundant but harmless for strings. However, if the intent is to use the offloaded stub directly, this re-stringification is unnecessary.🐛 Suggested fix
Since
toolResponse.contentis already stringified when passed from lines 194-196, you can use it directly:- const toolContent = this.stringifyToolContent(toolResponse.content) conversationMessages.push({ role: 'tool', - content: toolContent, + content: toolResponse.content as string, tool_call_id: toolCall.id })Alternatively, rename the parameter to clarify that it's already a string:
private appendNativeFunctionCallMessages( conversationMessages: ChatMessage[], toolCall: { id: string; name: string; arguments: string }, - toolResponse: { content: unknown } + toolResponse: { content: string } ): void {
🤖 Fix all issues with AI agents
In `@docs/specs/tool-output-guardrails/spec.md`:
- Around line 3-4: Replace the incorrect date string "2025-03-08" in the spec
header with the correct date "2026-01-15" so the document status header shows
the accurate creation/update date (update the line containing 日期: 2025-03-08 to
日期: 2026-01-15).
♻️ Duplicate comments (1)
src/main/presenter/toolPresenter/index.ts (1)
164-175: Consider platform-agnostic fallback path.The fallback path
'~/.deepchat/sessions/<conversationId>/tool_<toolCallId>.offload'uses Unix-style~which may not be accurate on Windows. However, sinceconversationIdis always provided by the caller (permessageBuilder.ts), this fallback is defensive and unlikely to be used in practice.The existing past review comment about prompt clarity is still valid - the LLM receives the actual path in the stub message itself, so the system prompt could be more explicit about using that concrete path.
♻️ Optional: Platform-agnostic fallback
buildToolSystemPrompt(context: { conversationId?: string }): string { const conversationId = context.conversationId || '<conversationId>' const offloadPath = resolveToolOffloadTemplatePath(conversationId) ?? - '~/.deepchat/sessions/<conversationId>/tool_<toolCallId>.offload' + '<session_directory>/tool_<toolCallId>.offload' return [ 'Tool outputs may be offloaded when large.', - `When you see an offload stub, read the full output from: ${offloadPath}`, + `When you see "[Tool output offloaded]" with "Full output saved to: <path>", use that exact path to read the full output.`, 'Use file tools to read that path. Access is limited to the current conversation session.' ].join('\n') }
🧹 Nitpick comments (4)
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts (1)
45-95: LGTM - comprehensive offload test!The test thoroughly validates the offload behavior:
- Uses 3001 characters (just above the 3000 threshold)
- Verifies the stub contains the offload indicator, character count, and file path
- Confirms
tool_call_response_rawpreserves the original data- Validates the offloaded content is correctly written to disk
- Checks that the tool message in conversation history contains the offload indicator
This aligns with the spec requirements and tests the key integration points.
Consider adding edge case test.
An additional test for exactly 3000 characters (which should NOT trigger offload) would strengthen the boundary condition coverage.
♻️ Optional: Boundary condition test
it('does not offload tool responses at exactly 3000 characters', async () => { const exactOutput = 'x'.repeat(3000) const rawData = { content: exactOutput } as MCPToolResponse const processor = new ToolCallProcessor({ getAllToolDefinitions: async () => [toolDefinition], callTool: async () => ({ content: exactOutput, rawData }) }) const conversationMessages: ChatMessage[] = [{ role: 'assistant', content: 'hello' }] const conversationId = 'conv-456' const modelConfig = { functionCall: true } as ModelConfig const events: any[] = [] for await (const event of processor.process({ eventId: 'event-2', toolCalls: [{ id: 'tool-2', name: 'mock_tool', arguments: '{}' }], enabledMcpTools: [], conversationMessages, modelConfig, abortSignal: new AbortController().signal, currentToolCallCount: 0, maxToolCalls: 5, conversationId })) { events.push(event) } const endEvent = events.find( (event) => event.type === 'response' && event.data?.tool_call === 'end' ) expect(endEvent).toBeDefined() expect(endEvent.data.tool_call_response).not.toContain('[Tool output offloaded]') })src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts (2)
221-223: Consider validating the parent path for read access in the catch block.When the target path doesn't exist and we fall back to validating the parent directory (lines 245-259), the session read check is only performed on the original
normalizedRequestedpath. If the parent directory resolution succeeds, we returnnormalizedRequestedwithout a session read check on the resolved parent.This is likely fine since we're creating new files (write operations), but for consistency with read operations, you may want to ensure the catch block doesn't apply session restrictions for paths that don't exist yet.
Also applies to: 235-237
1134-1144: Unused filtering logic - consider simplification.The
validMatchesmapping currently just returns thefilePathunchanged without any filtering. This appears to be leftover from removed allowlist filtering logic. Consider simplifying:♻️ Suggested simplification
- // Preserve matches without allowlist filtering for read operations. - const validMatches = await Promise.all( - matches.map(async (filePath) => { - return filePath - }) - ) - - const filteredMatches = validMatches.filter((match): match is string => match !== null) + const filteredMatches = matchessrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts (1)
329-373: Same pattern issue in appendLegacyFunctionCallMessages.The parameter
toolResponse: { content: unknown; rawData?: unknown }receives already-stringified content but the type suggests it could be anything. Consider updating the type to{ content: string }for clarity, since callers (lines 214-216) always pass stringified content.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/specs/tool-output-guardrails/plan.mddocs/specs/tool-output-guardrails/spec.mddocs/specs/tool-output-guardrails/tasks.mdsrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/toolPresenter/index.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/specs/tool-output-guardrails/tasks.md
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
All logs and comments must be in English
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use OxLint as the linter
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use Prettier as the code formatter
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Electron main process code should reside in
src/main/, with presenters organized inpresenter/subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed viaeventbus.ts
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use camelCase for variable and function names; use PascalCase for types and classes; use SCREAMING_SNAKE_CASE for constants
Configure Prettier with single quotes, no semicolons, and line width of 100 characters. Runpnpm run formatafter completing features
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use OxLint for linting JavaScript and TypeScript files; ensure lint-staged hooks and typecheck pass before commits
Enable strict TypeScript type checking
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement all system capabilities as main-process presenters following the Presenter Pattern
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
{src/main,src/renderer,test}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use IPC communication: Renderer calls main process via
usePresentercomposable, Main sends to Renderer via EventBus
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/agentPresenter/acp/agentFileSystemHandler.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Vitest test suites should be organized in
test/main/**andtest/renderer/**mirroring source structure, with file names following*.test.tsor*.spec.tspattern
Files:
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write tests using Vitest with separate test suites for main and renderer processes
Files:
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts
🧠 Learnings (6)
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/**/*.ts : Implement all system capabilities as main-process presenters following the Presenter Pattern
Applied to files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/sessionPresenter/index.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/mcpPresenter/inMemoryServers/**/*.ts : Implement new MCP tools in the inMemoryServers directory and register in mcpPresenter/index.ts
Applied to files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/sessionPaths.tstest/main/presenter/agentPresenter/loop/toolCallProcessor.test.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/loop/toolCallProcessor.ts
📚 Learning: 2026-01-05T02:41:45.219Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Applies to src/main/**/*.ts : Electron main process code should reside in `src/main/`, with presenters organized in `presenter/` subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed via `eventbus.ts`
Applied to files:
src/main/presenter/sessionPresenter/sessionPaths.tssrc/main/presenter/sessionPresenter/index.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to **/*.test.ts : Write tests using Vitest with separate test suites for main and renderer processes
Applied to files:
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to {src/main,src/renderer,test}/**/*.{ts,tsx,js} : Use IPC communication: Renderer calls main process via `usePresenter` composable, Main sends to Renderer via EventBus
Applied to files:
src/main/presenter/sessionPresenter/index.ts
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Use `usePresenter` composable for main process communication instead of direct IPC calls
Applied to files:
src/main/presenter/sessionPresenter/index.ts
🧬 Code graph analysis (6)
src/main/presenter/toolPresenter/index.ts (1)
src/main/presenter/sessionPresenter/sessionPaths.ts (1)
resolveToolOffloadTemplatePath(28-32)
src/main/presenter/sessionPresenter/sessionPaths.ts (1)
test/mocks/electron.ts (1)
app(2-10)
src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts (1)
src/main/presenter/sessionPresenter/sessionPaths.ts (1)
getSessionsRoot(4-6)
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts (3)
test/mocks/electron.ts (1)
app(2-10)src/main/presenter/agentPresenter/loop/toolCallProcessor.ts (1)
ToolCallProcessor(46-487)src/shared/types/presenters/legacy.presenters.d.ts (1)
ModelConfig(148-168)
src/main/presenter/sessionPresenter/index.ts (1)
src/main/presenter/sessionPresenter/sessionPaths.ts (1)
resolveSessionDir(8-19)
src/main/presenter/agentPresenter/loop/toolCallProcessor.ts (1)
src/main/presenter/sessionPresenter/sessionPaths.ts (1)
resolveToolOffloadPath(21-26)
⏰ 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 (21)
src/main/presenter/sessionPresenter/index.ts (3)
18-28: LGTM!The imports are correctly added for the new offload file cleanup functionality. The
fspromises API andresolveSessionDirutility are appropriately imported to support the session cleanup logic.
434-439: LGTM!The deletion order is correct: clear permissions first, then delete offload files, and finally delete the conversation from the database. This ensures proper cleanup of all session-related artifacts.
496-508: LGTM!The
deleteSessionOffloadFilesmethod correctly:
- Returns early if
resolveSessionDirreturns null (invalid/malicious conversationId)- Uses
fs.rmwithrecursive: true, force: truefor safe directory removal- Swallows errors with a warning log, which is appropriate since offload file cleanup failure shouldn't block conversation deletion
Note: The AI summary mentioned a "duplicate insertion" of this method, but I only see one definition here, which appears to be a summary inaccuracy.
src/main/presenter/toolPresenter/index.ts (1)
9-9: LGTM!The import is correctly added to support the new
buildToolSystemPromptmethod.docs/specs/tool-output-guardrails/spec.md (1)
39-59: LGTM!The specification is well-structured and clearly documents:
directory_treedepth semantics (root=0, max=3, default=1)- Offload threshold (3000 characters)
- File extension (
.offload) - now consistent with the implementation- Session-scoped file access restrictions
The acceptance criteria align with the code implementation.
test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts (1)
35-43: LGTM!The test setup correctly:
- Creates an isolated temp directory for each test
- Mocks
app.getPathto return the temp directory- Cleans up the temp directory after each test
This ensures test isolation and prevents file system pollution.
src/main/presenter/sessionPresenter/sessionPaths.ts (4)
4-6: LGTM!The
getSessionsRootfunction correctly usesapp.getPath('home')for cross-platform home directory resolution, combined withpath.resolvefor proper path construction.
8-19: LGTM - robust path traversal protection!The
resolveSessionDirfunction correctly implements security measures:
- Rejects empty/whitespace-only conversation IDs
- Uses
path.resolveto normalize the path and collapse..sequences- Validates the resolved path stays within the sessions root
- Handles the edge case where
resolvedSessionDir === sessionsRoot(e.g., whenconversationIdis ".")This effectively prevents directory traversal attacks.
21-26: LGTM!The
resolveToolOffloadPathfunction correctly:
- Delegates base path validation to
resolveSessionDir- Sanitizes
toolCallIdby replacing both forward and back slashes with underscores, preventing path injectionThe regex
[\\/]handles both Unix (/) and Windows (\) path separators.
28-32: LGTM!The template path function correctly returns a placeholder pattern for use in system prompts, leveraging the same secure
resolveSessionDirvalidation.src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts (5)
92-95: LGTM! Depth parameter properly constrained.The schema correctly limits depth to integers between 0 and 3 with a sensible default of 1. This aligns with the plan to prevent unbounded recursion in directory tree operations.
157-160: LGTM! PathValidationOptions properly extended.The optional
accessTypefield cleanly extends the existing interface for read/write differentiation without breaking existing callers.
162-176: LGTM! Constructor and session scoping look correct.The constructor properly initializes the conversation context and caches the normalized sessions root path. Good defensive handling with optional
conversationId.
263-284: Session read access control is well implemented.The logic correctly:
- Allows access outside sessions root without restriction
- Requires an active conversation for any session file access
- Restricts reads to only the current conversation's directory
One minor observation: the error messages are clear and will help with debugging access issues.
1044-1079: LGTM! Directory tree depth control correctly implemented.The depth limiting logic is sound:
currentDepthstarts at 0 for the root- Recursion only proceeds when
currentDepth < depth- This means
depth: 1allows one level of children (root + 1), which matches the planThis effectively prevents unbounded recursion that could cause the 10MB failure mentioned in the plan.
src/main/presenter/agentPresenter/loop/toolCallProcessor.ts (4)
43-44: LGTM! Offload thresholds align with the plan.The threshold (3000) and preview length (1024) match the specification in plan.md. These are reasonable values for balancing context window usage while preserving enough preview for the model.
375-397: LGTM! Offload logic is robust with good fallback behavior.The implementation correctly:
- Returns original content if under threshold or no conversationId
- Gracefully falls back to original content on file I/O errors
- Creates the session directory recursively before writing
This matches the plan's risk mitigation for missing
conversationIdscenarios.
399-407: LGTM! Stub format is clear and informative.The stub includes all necessary information for the model to understand the offload and retrieve the full content via file tools.
110-123: LGTM! Tool call finished notification is well-implemented.Good defensive programming with the try-catch wrapper to prevent callback errors from breaking the main flow.
docs/specs/tool-output-guardrails/plan.md (2)
1-87: Plan document is well-structured and aligns with implementation.The plan accurately describes the implemented features:
- Directory tree depth (0-3, default 1) ✓
- Offload threshold (3000 chars) ✓
- Preview length (1024 chars) ✓
- File path format (.offload extension) ✓
- Session-scoped read access ✓
The extension inconsistency noted in past review comments has been resolved - both lines 29 and 69 now correctly use
.offload.
85-87: Good call-out on conversationId fallback behavior.The plan correctly notes the need to define fallback behavior when
conversationIdis missing. The implementation intoolCallProcessor.tshandles this by simply returning the original content without offloading (line 381), which matches the suggested "truncate only, don't offload" approach mentioned here.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Raise offload threshold from 3000 to 5000 characters - Add whitelist for tools that require offload (execute_command, directory_tree, etc.) - Add offset/limit params to read_file for pagination support - Auto-truncate large files at 4500 chars with metadata hint - Prevents infinite offload loop when reading offloaded files
* docs(spec): add tool output guardrails * fix(agent): tool output guardrails * fix(agent): standardize tool offload extension * feat: extract path to session * fix: review issue * fix: error response on renderer * feat: add read_file pagination and whitelist-based tool offload - Raise offload threshold from 3000 to 5000 characters - Add whitelist for tools that require offload (execute_command, directory_tree, etc.) - Add offset/limit params to read_file for pagination support - Auto-truncate large files at 4500 chars with metadata hint - Prevents infinite offload loop when reading offloaded files * fix: independent reasoning time for each thinking block in agent loop
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.