Feat task tracking UI 959ce #466
Conversation
Built-in tools are now always enabled and cannot be disabled via configuration or UI. The related config fields and UI toggles have been deprecated or removed for clarity and backward compatibility. MCP tool configuration remains unchanged.
Remove deprecated built-in tools configuration and related UI/status logic, as built-in tools are now always enabled and no longer require tracking or display. Cleans up code and simplifies tool status handling.
Trigger AnalysisAgent to summarize glob results when file count or character count exceeds thresholds. Update documentation with smart file search and agent collaboration guidelines. Add explicit handling for ask-agent and task-boundary tools in orchestrator.
…eList. Implement task tracking in ComposeRenderer, allowing dynamic updates to task status and management of active tasks. This improves user experience by displaying relevant tasks in the chat interface.
WalkthroughThe system transitions built-in tools from configuration-driven conditional enablement to always-enabled registration. Three sub-agents are unconditionally registered as tools. Built-in tool filtering and toggling are removed from configuration and UI. Task-boundary tool support and AnalysisAgent integration for large-result summarization are added. UI now displays active tasks in a separate panel and removes configurable built-in tool controls. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as AgentChatInterface
participant Renderer as ComposeRenderer
participant Orchestrator as ToolOrchestrator
participant Tool as Task-Boundary Tool
participant TaskPanel as TaskPanel
User->>UI: Agent processes task
UI->>Renderer: Render tool results
Renderer->>Orchestrator: Execute tool (task-boundary)
Orchestrator->>Tool: executeTaskBoundaryTool()
Tool->>Renderer: Tool result (TaskBoundaryParams)
Renderer->>Renderer: updateTaskFromToolCall()
Renderer->>Renderer: Update _tasks state
Renderer->>UI: tasks property updated
UI->>TaskPanel: activeTasks computed (filter non-completed)
TaskPanel->>UI: Render two-pane layout
Note over UI,TaskPanel: AgentMessageList (0.65) + TaskPanel (0.35)
sequenceDiagram
participant User
participant ToolRegistry as ToolRegistry
participant GlobTool as GlobTool
participant AnalysisAgent as AnalysisAgent
participant Result as Result
User->>ToolRegistry: Register built-in tools
ToolRegistry->>GlobTool: Create with analysisAgent
Note over ToolRegistry: All built-in tools registered unconditionally
User->>GlobTool: Execute glob() with large result set
GlobTool->>GlobTool: Check result size vs. thresholds
alt Result exceeds thresholds
GlobTool->>AnalysisAgent: Trigger summarization
AnalysisAgent->>Result: Return summarized result
else Result within thresholds
GlobTool->>Result: Return standard result
end
Result->>User: Tool result with metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a task tracking UI feature and changes built-in tools to be always enabled, removing the ability to configure them. The key changes involve:
- Adding a new TaskPanel component for displaying active tasks from the task-boundary tool
- Removing UI configuration options for built-in tools (they are now always enabled)
- Enhancing GlobTool with AnalysisAgent integration to auto-summarize large file lists
- Removing the
enabledBuiltinToolsfield fromToolConfigFile(breaking change)
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| index.tsx | Updated console logs to reflect built-in tools are always enabled |
| ToolConfigDialog.kt | Removed built-in tools configuration UI, added info banner explaining they're always enabled |
| TaskPanel.kt | New component for displaying task tracking UI with TaskInfo/TaskStatus data structures |
| ComposeRenderer.kt | Added task tracking state and updateTaskFromToolCall method |
| CodingAgentViewModel.kt | Removed built-in tools tracking from ToolLoadingStatus |
| AgentChatInterface.kt | Integrated TaskPanel alongside message list with conditional layout |
| AutoDevApp.kt | Updated console message to reflect built-in tools change |
| ToolConfigExports.kt | Deprecated enabledBuiltinTools parameter, updated getConfigSummary |
| McpToolExecutionTest.kt | Updated tests to use empty enabledBuiltinTools list |
| ToolRegistry.kt | Removed filtering logic, always registers all built-in tools |
| BuiltinToolsProvider.kt | Added AnalysisAgent support to GlobTool |
| GlobTool.kt | Added auto-summarization for large result sets with AnalysisAgent |
| ToolOrchestrator.kt | Added specialized execution methods for task-boundary and ask-agent tools |
| ToolConfigManager.kt | Deleted entire file (functionality moved/removed) |
| ToolConfig.kt | Removed enabledBuiltinTools field from ToolConfigFile data class |
| McpToolConfigService.kt | Deprecated methods related to built-in tool filtering |
| CodingAgentTemplate.kt | Updated documentation with smart file search and agent collaboration guidelines |
| CodingAgent.kt | Removed conditional registration logic for sub-agents |
Comments suppressed due to low confidence (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/config/ToolConfig.kt:20
- The removal of the
enabledBuiltinToolsfield is a breaking change that will cause deserialization errors for existing configuration files that contain this field. Consider keeping the field with@Deprecatedand marking it as ignored during serialization/deserialization using@kotlinx.serialization.Transientor a custom serializer to maintain backward compatibility with existing config files.
data class ToolConfigFile(
/**
* List of enabled MCP tool names (tool names, not server names)
*/
val enabledMcpTools: List<String> = emptyList(),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val activeTasks = remember(viewModel.renderer.tasks) { | ||
| viewModel.renderer.tasks.filter { | ||
| it.status != TaskStatus.COMPLETED && it.status != TaskStatus.CANCELLED | ||
| } | ||
| } |
There was a problem hiding this comment.
The activeTasks filtering is computed twice identically (lines 123-127 and lines 280-284). Consider extracting this into a computed property in the ViewModel or ComposeRenderer to avoid duplicate logic and ensure consistency. For example, add val activeTasks: List<TaskInfo> get() = _tasks.filter { it.status != TaskStatus.COMPLETED && it.status != TaskStatus.CANCELLED } to ComposeRenderer.
| val metadata = mutableMapOf( | ||
| "pattern" to params.pattern, | ||
| "search_path" to searchPath, | ||
| "total_matches" to matches.size.toString(), |
There was a problem hiding this comment.
[nitpick] The metadata map is changed from immutable (mapOf) to mutable (mutableMapOf) only to support adding fields conditionally later. However, this change happens before the conditional logic that needs it. Consider keeping it as mapOf initially and only converting to mutable when needed (line 140+), or use buildMap {} for cleaner code.
| return@safeExecute ToolResult.Success(summarizedText, metadata.toMap()) | ||
| } | ||
| } | ||
|
|
||
| ToolResult.Success(resultText, metadata.toMap()) |
There was a problem hiding this comment.
The call to .toMap() on line 178 is unnecessary since metadata is already a MutableMap which is a Map. The conversion creates an unnecessary copy. Simply pass metadata directly.
| return@safeExecute ToolResult.Success(summarizedText, metadata.toMap()) | |
| } | |
| } | |
| ToolResult.Success(resultText, metadata.toMap()) | |
| return@safeExecute ToolResult.Success(summarizedText, metadata) | |
| } | |
| } | |
| ToolResult.Success(resultText, metadata) |
| } | ||
| } | ||
|
|
||
| ToolResult.Success(resultText, metadata.toMap()) |
There was a problem hiding this comment.
Same issue as Comment 4: the call to .toMap() on line 182 is unnecessary. Since metadata is already a MutableMap, the conversion creates an unnecessary copy. Simply pass metadata directly.
| ToolResult.Success(resultText, metadata.toMap()) | |
| ToolResult.Success(resultText, metadata) |
| ## Task Execution Guidelines | ||
| 1. **Gather Context First**: Before making changes, use /read-file and /glob to understand the codebase | ||
| 1. **Gather Context First**: Before making changes understand the codebase |
There was a problem hiding this comment.
Missing comma after 'changes'. Should be: 'Before making changes, understand the codebase'.
| 1. **Gather Context First**: Before making changes understand the codebase | |
| 1. **Gather Context First**: Before making changes, understand the codebase |
| ## 任务执行指南 | ||
| 1. **先获取上下文**: 在进行更改之前，使用 /read-file 和 /glob 来了解代码库 | ||
| 1. **先获取上下文**: 在进行更改之前，先来了解代码库 |
There was a problem hiding this comment.
The Chinese text is awkward. '先来了解代码库' should be '先了解代码库' (remove '来' which is redundant).
| 1. **先获取上下文**: 在进行更改之前，先来了解代码库 | |
| 1. **先获取上下文**: 在进行更改之前，先了解代码库 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentViewModel.kt (1)
420-430: Fix dead code that always returns 0.Line 428 uses
sumOf { _ -> 0 }which always returns 0 regardless of the number of preloaded servers. This appears to be placeholder code or a logic error.If this should count the total tools across all preloaded servers, consider:
} else { - mcpPreloadingStatus.preloadedServers.sumOf { _ -> 0 } + mcpPreloadingStatus.preloadedServers.sumOf { server -> + McpToolConfigManager.getToolsForServer(server) + } }If it's intentional to return 0 when
enabledMcpToolsCountis 0, the entire else branch can be removed for clarity:val enabledMcpToolsCount = toolConfig?.enabledMcpTools?.size ?: 0 - if (enabledMcpToolsCount > 0) { - enabledMcpToolsCount - } else { - mcpPreloadingStatus.preloadedServers.sumOf { _ -> 0 } - } + enabledMcpToolsCountmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/GlobTool.kt (1)
129-183: Guard AnalysisAgent failures and reconsider storing full result in metadataThe new summarization path is useful, but two concerns:
Reliability:
analysisAgent.execute(...)is insidesafeExecute(ToolErrorType.INVALID_PATTERN). If AnalysisAgent throws or returns a failure for non‑pattern reasons (network/LLM issues), the whole glob call can fail even though file matching itself succeeded. Since summarization is an enhancement, it would be safer to wrap this call in its owntry/catchand silently fall back to the plainToolResult.Success(resultText, metadata)on any analysis error.Metadata size: When summarization succeeds you add
full_result(the entire file list text) into metadata. If downstream consumers surface metadata into model context, this may partially defeat the purpose of summarizing for large outputs. Consider either:
- Omitting
full_resultfrom metadata, or- Truncating it / providing an external reference instead, controlled by a separate flag.
These adjustments would keep glob stable and avoid unexpected large payloads while still enabling the new summary behavior.
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/config/ToolConfigDialog.kt (1)
291-327: AssigndiscoverMcpToolsresult tomcpToolsstate in the reload handler (line 311)The
discoverMcpToolsmethod returnsMap<String, List<ToolItem>>which should update themcpToolsstate variable. Currently the return value is discarded. This causes:
- Newly added/removed MCP servers not reflected in UI after reload
- Line 319 print statement reads stale
mcpToolsdata instead of freshly discovered toolsCapture the result:
mcpTools = McpToolConfigManager.discoverMcpTools(newMcpServers, toolConfig.enabledMcpTools.toSet())This pattern is already used in
CodingAgent.kt:232andToolOrchestrator.kt:312—both capture and use the return value.
🧹 Nitpick comments (10)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/AutoDevApp.kt (1)
615-626: Log message accurately reflects new always-enabled tool behavior.The updated log message at line 620 correctly communicates that built-in tools are now unconditionally enabled, aligning with the PR's transition away from configuration-driven tool filtering. The message is clear and informative.
Optional: Consider a logging framework for debug output.
The console prints via
printlnwork for now, but migrating to a proper logging framework (e.g., SLF4J or a Kotlin logger) would improve observability and allow filtering by level. This is a nice-to-have for future refactoring.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentViewModel.kt (2)
201-237: Consider improving error handling and logging.The method prints errors to console (line 234) rather than using a proper logging framework. Consider using structured logging for better observability in production.
} catch (e: Exception) { - println("[ERROR] Failed to save conversation history: ${e.message}") + logger.error("Failed to save conversation history", e) }
413-448: Consider simplifying the nested conditionals.The method has high cyclomatic complexity due to repeated
isPreloading()checks. Consider extracting helper functions or early returns to improve readability.Example refactor:
fun getToolLoadingStatus(): ToolLoadingStatus { val toolConfig = cachedToolConfig val subAgentTools = ToolType.byCategory(ToolCategory.SubAgent) val mcpServersTotal = toolConfig?.mcpServers?.filter { !it.value.disabled }?.size ?: 0 val mcpServersLoaded = mcpPreloadingStatus.preloadedServers.size val (mcpToolsEnabled, mcpToolsTotal) = if (McpToolConfigManager.isPreloading()) { 0 to 0 } else { val enabled = toolConfig?.enabledMcpTools?.size ?: 0 val total = McpToolConfigManager.getTotalDiscoveredTools() enabled to total } return ToolLoadingStatus( subAgentsEnabled = subAgentTools.size, subAgentsTotal = subAgentTools.size, mcpServersLoaded = mcpServersLoaded, mcpServersTotal = mcpServersTotal, mcpToolsEnabled = mcpToolsEnabled, mcpToolsTotal = mcpToolsTotal, isLoading = McpToolConfigManager.isPreloading() ) }mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (1)
68-71: Consider clearing_taskswhen resetting conversation stateThe task-boundary integration and
_tasksstate wiring look solid, but_tasksis only ever appended/updated inupdateTaskFromToolCalland never cleared. SinceAgentChatInterfacefilters out COMPLETED/CANCELLED tasks, this is mostly invisible in the UI, but:
- Stale tasks from previous sessions may linger in memory if the same
ComposeRendererinstance is reused.- A “Clear history/New chat” action probably expects both timeline and task context to reset.
You can keep the current behavior and just clear tasks when resetting:
fun clearMessages() { _timeline.clear() _currentStreamingOutput = "" _errorMessage = null _taskCompleted = false _isProcessing = false _executionStartTime = 0L _currentExecutionTime = 0L _totalTokenInfo = TokenInfo() _lastMessageTokenInfo = null + _tasks.clear() }Also applies to: 230-234, 267-303, 462-472
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.kt (1)
122-166: Deduplicate task-aware layout and use a derived state foractiveTasksThe two blocks that compute
activeTasksand renderAgentMessageList+TaskPanelare duplicated in the tree-view and non-tree-view branches. The filtering logic is also derived directly from aSnapshotStateList.To improve maintainability and Compose-idiomatic state handling:
- Extract this into a small helper composable (or function) that takes
rendererand renders either the single-pane or two-pane layout.- Prefer deriving
activeTasksviaderivedStateOf { … }instead ofremember(viewModel.renderer.tasks) { … }, so it reacts naturally toSnapshotStateListmutations without relying on equality semantics.For example:
@Composable private fun ChatAndTaskPane(renderer: ComposeRenderer, modifier: Modifier = Modifier) { val activeTasks by remember { derivedStateOf { renderer.tasks.filter { it.status != TaskStatus.COMPLETED && it.status != TaskStatus.CANCELLED } } } if (activeTasks.isNotEmpty()) { Row( modifier = modifier, horizontalArrangement = Arrangement.spacedBy(8.dp) ) { AgentMessageList( renderer = renderer, modifier = Modifier .weight(0.65f) .fillMaxHeight(), onOpenFileViewer = { renderer.openFileViewer(it) } ) TaskPanel( tasks = activeTasks, modifier = Modifier .weight(0.35f) .fillMaxHeight() .padding(end = 12.dp), onClose = { /* see next comment */ } ) } } else { AgentMessageList( renderer = renderer, modifier = modifier, onOpenFileViewer = { renderer.openFileViewer(it) } ) } }Then both branches can just call
ChatAndTaskPane(renderer = viewModel.renderer, modifier = Modifier.fillMaxWidth().weight(1f)).Also applies to: 279-323
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt (1)
27-47: Task data model and panel UI look consistent; consider simplifying list keys
TaskInfo/TaskStatusalign well with howComposeRendererupdates tasks, and the panel/ card UI logic is sound (status display, elapsed time, summary, empty state).One small optional tweak: in the
LazyColumnyou currently use:items(tasks, key = { "${it.taskName}_${it.timestamp}" }) { task -> … }Since
ComposeRendererenforces a singleTaskInfopertaskName(updates in-place),taskNameby itself is already a stable, unique key and won’t change across status/summary updates. You could simplify tokey = { it.taskName }to avoid key churn on each timestamp update, though this is a minor optimization and not required.Also applies to: 49-154, 156-263
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgent.kt (1)
110-121: Add clarifying comments explaining the purpose of each sub-agent registration.All three registrations are actively used and necessary, each serving distinct purposes:
registerTool()adds to MainAgent's tool list (accessed viagetAllTools()for tool filtering/listing)toolRegistry.registerTool()adds to ToolRegistry (accessed viagetAllTools()for tool discovery and formatting)subAgentManager.registerSubAgent()adds to SubAgentManager's internal map (accessed viaaskSubAgent()for sub-agent execution)Add a comment above this block explaining these distinct purposes to clarify the pattern for future maintainers:
// Register Sub-Agents in three registries for different access patterns: // - MainAgent.tools: for getAllTools/getSubAgents filtering // - ToolRegistry: for tool discovery and AI formatting // - SubAgentManager: for runtime execution via askSubAgent() registerTool(errorRecoveryAgent) toolRegistry.registerTool(errorRecoveryAgent) subAgentManager.registerSubAgent(errorRecoveryAgent) // ... etcmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/config/ToolConfigDialog.kt (1)
39-39: Built‑ins‑always‑enabled messaging is consistent and clearThe inline comments, footer summary (
"Built-in tools: Always enabled"), and the new banner inToolSelectionTaball consistently communicate that built‑in tools are no longer configurable and are always available. This aligns well with the PR’s direction.If you plan to evolve the list of “file operations, search, shell, ...” over time, consider centralizing that text (or at least the examples) to avoid future drift between banner and actual built‑in set.
Also applies to: 346-349, 430-459
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/GlobTool.kt (1)
93-98: Threshold constants are reasonable but currently hard‑codedThe 100‑file and 10k‑character thresholds are sensible defaults for auto‑summarization. If you expect different repos or environments to have very different needs, consider making these thresholds configurable via tool metadata or config in the future.
mpp-core/src/jsMain/kotlin/cc/unitmesh/agent/config/ToolConfigExports.kt (1)
78-86: JS config API cleanly deprecates built‑in toggles while preserving shapeThe JS‑facing API keeps
enabledBuiltinToolsfor backward compatibility but:
- Clearly documents it as deprecated/ignored.
- Ensures
updateToolConfigonly updatesenabledMcpTools.- Maps only MCP fields in
toCommon/fromCommon, withenabledBuiltinToolsalways coming back asemptyArray().This matches the “built‑ins always enabled” direction without breaking existing call signatures. If you have public TS typings, consider mirroring the deprecation note there as well so JS/TS consumers see it at compile time.
Also applies to: 92-96, 101-104, 107-107, 130-133, 151-152
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgent.kt(1 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgentTemplate.kt(2 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/config/McpToolConfigService.kt(2 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/config/ToolConfig.kt(2 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/config/ToolConfigManager.kt(0 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/orchestrator/ToolOrchestrator.kt(2 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/GlobTool.kt(5 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/registry/BuiltinToolsProvider.kt(1 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/registry/ToolRegistry.kt(1 hunks)mpp-core/src/commonTest/kotlin/cc/unitmesh/agent/orchestrator/McpToolExecutionTest.kt(2 hunks)mpp-core/src/jsMain/kotlin/cc/unitmesh/agent/config/ToolConfigExports.kt(3 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/AutoDevApp.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.kt(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentViewModel.kt(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt(3 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/config/ToolConfigDialog.kt(6 hunks)mpp-ui/src/jsMain/typescript/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/config/ToolConfigManager.kt
🧰 Additional context used
🧬 Code graph analysis (3)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgent.kt (2)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/registry/ToolRegistry.kt (1)
registerTool(26-30)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/core/MainAgent.kt (1)
registerTool(12-15)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/registry/ToolRegistry.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/core/MainAgent.kt (1)
registerTool(12-15)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.kt (2)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentMessageList.kt (1)
AgentMessageList(29-146)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt (1)
TaskPanel(52-154)
⏰ 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). (4)
- GitHub Check: Agent
- GitHub Check: Test mpp-core (KMP)
- GitHub Check: Build (223)
- GitHub Check: Build (241)
🔇 Additional comments (17)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentViewModel.kt (2)
27-27: LGTM: Backward-compatible constructor parameter addition.The optional
chatHistoryManagerparameter is properly defaulted tonulland correctly integrated with null-safe operators throughout the class.
316-336: LGTM: Session management methods are well-implemented.The
newSessionandswitchSessionmethods correctly handle state transitions by clearing the renderer before loading new session data. Null-safety is properly enforced with safe call operators.mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgentTemplate.kt (3)
41-41: LGTM - Simplified guidance.The simplified wording maintains the intent while being more concise and tool-agnostic.
221-285: LGTM - Chinese translations are consistent.The Chinese translations properly mirror the English content structure and correctly preserve technical identifiers (agent names, commands, JSON parameters) untranslated, which is the right approach for a technical prompt.
66-106: Fix documentation to match actual/ask-agentcommand syntax.The agent names and command structure are correct, but the template documentation uses an inconsistent format. The examples show JSON blocks with nested triple backticks, while the actual tool interface (AskAgentTool.kt:40) uses named parameters.
Update the examples to use the correct named parameter format instead of JSON blocks:
- Change from:
/ask-agentfollowed by```jsonblock- Change to:
/ask-agent agentName="analysis-agent" question="What are the main patterns in the file structure you analyzed?"This applies to all three
/ask-agentexamples in lines 77-98 (and their Chinese equivalents in lines 256-277).mpp-ui/src/jsMain/typescript/index.tsx (1)
89-89: LGTM! Logging messages updated to reflect always-enabled built-in tools.The informational messages now correctly communicate that built-in tools are always enabled, aligning with the broader architectural changes in this PR.
Also applies to: 116-116
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/config/McpToolConfigService.kt (1)
25-54: LGTM! Clean deprecation strategy for built-in tool filtering.The deprecated methods now correctly implement always-enabled behavior with clear
ReplaceWithhints for callers to update their code. The backward-compatible approach allows existing code to continue functioning while guiding developers toward the new semantics.mpp-core/src/commonTest/kotlin/cc/unitmesh/agent/orchestrator/McpToolExecutionTest.kt (1)
26-29: LGTM! Test setup properly updated for deprecated field.The tests now initialize
enabledBuiltinToolswith an empty list and include clear comments explaining that the field is deprecated. This maintains backward compatibility while documenting the architectural change for future maintainers.Also applies to: 52-55
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/registry/ToolRegistry.kt (1)
172-183: LGTM! Built-in tool registration simplified to always-enabled behavior.The removal of config-based filtering streamlines the registration logic and ensures all built-in tools are consistently available. The updated logging clearly communicates the always-enabled status.
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/orchestrator/ToolOrchestrator.kt (3)
256-266: LGTM! Clean addition of new tool type handlers.The orchestrator now properly routes
AskAgenttool type andtask-boundarytools to their specialized execution functions, maintaining consistency with the existing tool handling patterns.
521-565: LGTM! Well-structured specialized tool executors.Both
executeAskAgentTool()andexecuteTaskBoundaryTool()follow the established pattern:
- Proper parameter extraction with fallback names
- Clear error messages for missing required parameters
- Type-safe parameter construction
- Consistent invocation flow
567-581: LGTM! Result handling extended to support AgentResult.The integration of
ToolResult.AgentResultinto success/error checking logic is clean and maintains backward compatibility with existing result types.mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/registry/BuiltinToolsProvider.kt (1)
35-38: No issues found. The GlobTool constructor properly accepts the optional AnalysisAgent parameter.The verification confirms that
GlobToolat line 336 ofmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/GlobTool.kthas the correct constructor signature withanalysisAgent: cc.unitmesh.agent.subagent.AnalysisAgent? = nullas an optional parameter. The code inBuiltinToolsProvider.ktpasses the arguments correctly to match this signature.mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/config/ToolConfig.kt (1)
16-42: No breaking change - backward compatibility is already implemented.Verification confirms that all three concerns from the review comment are already addressed:
kotlinx.serialization is configured with
ignoreUnknownKeys = truein bothMcpToolConfigManager.ktandConfigManager.js.kt, which means existing config files containing theenabledBuiltinToolsfield will deserialize successfully with the field gracefully ignored.Error handling includes fallback to
ToolConfigFile.default(), providing migration resilience if deserialization encounters issues.Deprecation documentation is already present: the field is marked
@deprecatedin the JS exports layer (JsToolConfigFile) with a note that it's "kept for backward compatibility," andToolConfig.ktincludes a comment that "Built-in tools are always enabled and don't need to be listed."Existing
~/.autodev/mcp.jsonfiles with the removed field will load without errors. The change is backward compatible.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/config/ToolConfigDialog.kt (1)
53-84: Auto‑save now correctly targets only MCP tool configAuto‑save now derives
enabledMcpToolssolely frommcpToolsand persists that plusmcpServers, with built‑ins intentionally excluded (always enabled). This is aligned with the new config model and looks correct.Also applies to: 90-90
mpp-core/src/jsMain/kotlin/cc/unitmesh/agent/config/ToolConfigExports.kt (2)
55-75: Config summary now clearly reflects MCP‑centric model
getConfigSummarynow produces a concise, multi‑section summary emphasizing that built‑in tools are always enabled and detailing enabled MCP tools and servers. This is aligned with the new configuration semantics and should be helpful for CLI/JS callers.
236-238: Log message correctly reflects built‑ins being always enabledThe load‑time logging now prints a fixed “Built-in tools: Always enabled (all)” line instead of relying on config state, matching the new behavior.
| ## Smart File Search Guidelines | ||
| When searching for files, use **specific and targeted patterns** to avoid overwhelming context: | ||
| **DO:** | ||
| - ✅ Use specific patterns: `src/**/*.kt`, `**/test/**/*.java`, `**/config/*.yml` | ||
| - ✅ Target specific directories: `/glob pattern="*.ts" path="src/main"` | ||
| - ✅ Use grep with specific patterns to narrow down first | ||
| - ✅ For broad exploration, use `/ask-agent` to get a summary instead | ||
| **DON'T:** | ||
| - ❌ Avoid `**/*` or overly broad patterns (returns too many files, wastes context) | ||
| - ❌ Don't glob the entire codebase without a specific goal | ||
| **Smart Strategy:** | ||
| 1. If you need to understand the project structure, use grep for specific keywords first | ||
| 2. Use targeted glob patterns based on what you found | ||
| 3. For very large result sets (100+ files), the system will automatically invoke a SummaryAgent to provide a concise overview | ||
There was a problem hiding this comment.
🧩 Analysis chain
Verify implementation details match the documented behavior.
The new guidance references specific thresholds and automatic behaviors:
- Line 64: Claims "100+ files" automatically invokes SummaryAgent (note: text says "SummaryAgent" but line 71 says "analysis-agent")
- Line 53: Shows glob syntax
/glob pattern="*.ts" path="src/main" - Line 64: Promises automatic agent invocation for large result sets
Please verify:
- The actual threshold for auto-summarization (is it exactly 100 files?)
- The correct glob tool syntax matches the schema
- The automatic AnalysisAgent invocation is implemented as described
- Consistent agent naming (SummaryAgent vs analysis-agent)
🏁 Script executed:
#!/bin/bash
# Verify the implementation of glob tool and auto-summarization threshold
echo "=== Searching for glob tool implementation and threshold ==="
rg -n "100|threshold" --type kotlin -g "**/tool/**" -g "**/*Glob*" -C 3
echo -e "\n=== Searching for AnalysisAgent/SummaryAgent references ==="
rg -n "AnalysisAgent|SummaryAgent|analysis-agent" --type kotlin -C 3
echo -e "\n=== Searching for glob tool schema/parameters ==="
ast-grep --pattern 'class GlobTool {
$$$
}'Length of output: 58005
Update line 64 to use "AnalysisAgent" instead of "SummaryAgent".
The documentation in line 64 incorrectly references "SummaryAgent" when the actual implementation uses "AnalysisAgent". The codebase correctly implements a 100-file threshold that automatically triggers the AnalysisAgent for large file lists. Update the reference to match the correct agent name.
Line 64 should read:
- Current: "For very large result sets (100+ files), the system will automatically invoke a SummaryAgent to provide a concise overview"
- Corrected: "For very large result sets (100+ files), the system will automatically invoke an AnalysisAgent to provide a concise overview"
🤖 Prompt for AI Agents
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgentTemplate.kt around
lines 47 to 65: the documentation on line 64 references "SummaryAgent" but the
implementation uses "AnalysisAgent", so update the text to replace
"SummaryAgent" with "AnalysisAgent" so the doc matches the code; ensure the
sentence reads that for very large result sets (100+ files) the system will
automatically invoke an AnalysisAgent to provide a concise overview.
| private val fileSystem: ToolFileSystem, | ||
| private val analysisAgent: cc.unitmesh.agent.subagent.AnalysisAgent? = null | ||
| ) : BaseExecutableTool<GlobParams, ToolResult>() { | ||
|
|
||
| override val name: String = "glob" | ||
| override val description: String = | ||
| """Efficiently finds files matching specific glob patterns (e.g., `src/**/*.ts`, `**/*.md`), returning absolute paths sorted by modification time (newest first). Ideal for quickly locating files based on their name or path structure, especially in large codebases.""".trimIndent() | ||
| """Efficiently finds files matching specific glob patterns (e.g., `src/**/*.ts`, `**/*.md`), returning absolute paths sorted by modification time (newest first). Ideal for quickly locating files based on their name or path structure, especially in large codebases. | ||
| ⚠️ IMPORTANT: Avoid overly broad patterns like `**/*` as they can return too many files and waste context. Use specific patterns instead. When results are too large (100+ files), the system automatically triggers AnalysisAgent to provide a concise summary.""".trimIndent() |
There was a problem hiding this comment.
AnalysisAgent wiring looks correct; update description to match actual sort behavior
Plumbing the optional AnalysisAgent through GlobTool’s constructor into GlobInvocation looks correct and keeps behavior unchanged when the agent is null.
One minor mismatch: the description claims “returning absolute paths sorted by modification time (newest first)”, but actual behavior sorts by modification time only when params.sortByTime is true, and otherwise sorts lexicographically by path. It may be worth updating the description to mention the sortByTime option explicitly (and that the default is path order) to avoid confusion.
Also applies to: 377-378
🤖 Prompt for AI Agents
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/GlobTool.kt around
lines 337-345 (and also update similar text at 377-378): the tool description
incorrectly states that results are "sorted by modification time (newest first)"
unconditionally; change the description to mention that sorting is conditional
based on params.sortByTime (when true results are sorted by modification time
newest-first, otherwise results are sorted lexicographically by path) and note
the default behavior (path order) so the docs match the actual implementation.
| TaskPanel( | ||
| tasks = activeTasks, | ||
| modifier = Modifier | ||
| .weight(0.35f) | ||
| .fillMaxHeight() | ||
| .padding(end = 12.dp), | ||
| onClose = { /* Tasks auto-hide when completed */ } | ||
| ) |
There was a problem hiding this comment.
Wire TaskPanel.onClose to some behavior or remove the close icon
Right now TaskPanel is given onClose = { /* Tasks auto-hide when completed */ }, so clicking the close button in the panel header has no visible effect. That’s likely confusing to users.
Consider either:
- Implementing
onCloseto hide/suppress the panel independently of task completion (e.g., via aremember { mutableStateOf(true) }flag or a UI-wide “show task panel” flag), or - Removing/hiding the close icon if manual closing isn’t supported and auto-hide on completion is the only intended behavior.
Also applies to: 303-310
🤖 Prompt for AI Agents
In
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.kt
around lines 146-153 (also applies to lines 303-310), the TaskPanel is passed a
no-op onClose lambda so the header close button does nothing; either wire
onClose to a visible state flag that hides the TaskPanel (e.g. add a remember {
mutableStateOf(true) } or use the existing UI-wide flag and set it to false on
close) or remove/hide the close icon from TaskPanel when manual closing isn’t
supported; update the call sites to toggle that state (or remove the close
parameter) so clicking the close button produces the expected behavior.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.