feat: Enable assistant chat to create and manage features #28
Conversation
Allow users to interact with the project assistant to create features through natural conversation. The assistant can now: - Create single features via `feature_create` tool - Create multiple features via `feature_create_bulk` - Skip features to deprioritize them via `feature_skip` Changes: - Add `feature_create` MCP tool for single-feature creation - Update assistant allowed tools to include feature management - Update system prompt to explain new capabilities - Enhance UI tool call display with friendly messages Security: File system access remains read-only. The assistant cannot modify source code or mark features as passing (requires actual implementation by the coding agent). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new feature creation tool (duplicated declaration) to the MCP server, expands assistant tool permissions to include feature management, and applies stylistic and message-handling updates to the assistant UI hook. Changes include DB-backed feature creation, assistant tool list changes, and UI WebSocket/message refinements. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Client/UI)
participant Assistant as Assistant (LLM)
participant Server as Server (assistant_chat_session)
participant Tool as Feature Tool (mcp_server.feature_create)
participant DB as Database
User->>Assistant: Ask to create feature (category, name, description, steps)
Assistant->>Server: Invoke tool "mcp__features__feature_create" (tool_call)
Server->>Tool: Execute feature_create(category,name,desc,steps)
Tool->>DB: Query max(Feature.priority)
DB-->>Tool: Return current max priority
Tool->>DB: Insert Feature(passes=False, priority=next_priority, ...)
DB-->>Tool: Confirm insert
Tool-->>Server: Return success JSON + feature data
Server-->>Assistant: Tool result (formatted)
Assistant-->>User: Confirm creation / show feature details
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
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: 1
🤖 Fix all issues with AI agents
In @mcp_server/feature_mcp.py:
- Around line 417-465: The tool parameters for feature_create are missing the
Pydantic Field constraints that FeatureCreateItem enforces; update the Annotated
Field() calls in feature_create for category, name, description, and steps to
match FeatureCreateItem by adding category: min_length=1, max_length=100; name:
min_length=1, max_length=255; description: min_length=1; and steps: min_length=1
(on the list) so the tool validates inputs the same way as FeatureCreateItem
(keep using the existing Field import and the same parameter names in the
feature_create definition).
🧹 Nitpick comments (4)
server/services/assistant_chat_session.py (1)
1-8: Module docstring is slightly outdated.The docstring says the assistant "cannot modify any files" which remains true, but it should mention the new feature management capabilities for accuracy.
📝 Suggested update
""" Assistant Chat Session ====================== -Manages read-only conversational assistant sessions for projects. -The assistant can answer questions about the codebase and features -but cannot modify any files. +Manages conversational assistant sessions for projects. +The assistant can answer questions about the codebase, manage the feature +backlog (create/skip features), but cannot modify source code files. """mcp_server/feature_mcp.py (1)
438-442: Potential race condition on priority assignment.If multiple concurrent calls to
feature_createoccur, the samemax_prioritycould be read before either commits, resulting in duplicate priorities. This is also present infeature_skipandfeature_create_bulk.For single-user assistant usage this is likely acceptable, but worth noting if scaling to concurrent access.
ui/src/hooks/useAssistantChat.ts (2)
257-273: Polling loop lacks a maximum retry limit.The
checkAndSendfunction recursively calls itself viasetTimeoutwhile the WebSocket isCONNECTING, but there's no limit on iterations. If the connection gets stuck inCONNECTINGstate (e.g., network issues), this could poll indefinitely.Consider adding a retry counter or timeout.
♻️ Suggested fix with retry limit
// Wait for connection then send start message + let attempts = 0; + const maxAttempts = 50; // 5 seconds max const checkAndSend = () => { - if (wsRef.current?.readyState === WebSocket.OPEN) { + if (wsRef.current?.readyState === WebSocket.OPEN) { setIsLoading(true); const payload: { type: string; conversation_id?: number } = { type: "start", }; if (existingConversationId) { payload.conversation_id = existingConversationId; setConversationId(existingConversationId); } wsRef.current.send(JSON.stringify(payload)); - } else if (wsRef.current?.readyState === WebSocket.CONNECTING) { + } else if (wsRef.current?.readyState === WebSocket.CONNECTING && attempts < maxAttempts) { + attempts++; setTimeout(checkAndSend, 100); + } else if (attempts >= maxAttempts) { + onError?.("Connection timeout"); } };
158-184: Type assertions could be consolidated.The inline type casts like
data.input as { name?: string; category?: string }are repeated with similar patterns. While functional, this could be refactored to a helper or typed more precisely if the input shapes are known.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mcp_server/feature_mcp.pyserver/services/assistant_chat_session.pyui/src/hooks/useAssistantChat.ts
🧰 Additional context used
🧬 Code graph analysis (1)
mcp_server/feature_mcp.py (2)
api/database.py (2)
Feature(19-44)to_dict(33-44)ui/src/lib/types.ts (1)
Feature(60-67)
🔇 Additional comments (4)
server/services/assistant_chat_session.py (3)
32-47: LGTM! Well-organized tool constants.The separation of
READONLY_FEATURE_MCP_TOOLS,FEATURE_MANAGEMENT_TOOLS, and combinedASSISTANT_FEATURE_TOOLSprovides clear organization and makes the permission model explicit.
73-141: Clear and comprehensive system prompt.The prompt effectively documents what the assistant can and cannot do, provides structured guidance for feature creation, and includes helpful examples. The security constraints are well-articulated.
192-199: Permissions correctly updated to include feature management tools.The
ASSISTANT_FEATURE_TOOLSconstant is consistently used in both the permissions list and allowed_tools, maintaining alignment between the security settings file and Claude client configuration.Also applies to: 239-239
ui/src/hooks/useAssistantChat.ts (1)
154-196: Good UX improvement with friendly tool descriptions.The user-friendly messages for tool calls significantly improve the chat experience by showing meaningful descriptions like "Creating feature: X" instead of raw tool names. The handling covers all the new feature management tools.
Match the same validation constraints used in FeatureCreateItem: - category: min_length=1, max_length=100 - name: min_length=1, max_length=255 - description: min_length=1 - steps: min_length=1 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
mcp_server/feature_mcp.py (1)
422-422: Consider validating individual step strings.The
stepsparameter enforcesmin_length=1on the list, ensuring at least one step is provided. However, there's no validation that each individual step string is non-empty. A user could pass["", "valid step"], which would store an empty string in the database.Consider adding validation to ensure each step is a non-empty string, either through Pydantic field validation or runtime checks:
# Option 1: Update the Field annotation to validate each item steps: Annotated[list[str], Field(min_length=1, description="List of implementation/verification steps (each must be non-empty)")]Then add a runtime check:
# Add after line 438 (inside the try block) if any(not step.strip() for step in steps): return json.dumps({"error": "All steps must be non-empty strings"})
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mcp_server/feature_mcp.py
🧰 Additional context used
🧬 Code graph analysis (1)
mcp_server/feature_mcp.py (4)
server/services/assistant_chat_session.py (1)
get_session(351-354)server/services/spec_chat_session.py (1)
get_session(424-427)api/database.py (2)
Feature(19-44)to_dict(33-44)ui/src/lib/types.ts (1)
Feature(60-67)
🔇 Additional comments (2)
mcp_server/feature_mcp.py (2)
18-18: LGTM!Documentation update correctly describes the new tool.
438-460: LGTM!The feature creation logic is correct:
- Priority calculation properly defaults to 1 for the first feature and increments from the max for subsequent features.
- The new feature is correctly initialized with
passes=False.- Database operations (add, commit, refresh) follow the correct sequence.
- The return payload includes the complete feature details, which is helpful for the caller.
|
Thank you @connor-tyndall |
This commit addresses issues found during review of PRs #12 and #28: ## PR #12 (Auth Error Handling) Fixes - Create shared auth.py module with centralized AUTH_ERROR_PATTERNS, is_auth_error(), and print_auth_error_help() functions - Fix start.bat to use directory check instead of outdated .credentials.json file check (matching start.sh behavior) - Update process_manager.py to import from shared auth module - Update start.py to import from shared auth module - Update documentation comments in autonomous_agent_demo.py and client.py to remove references to deprecated .credentials.json ## PR #28 (Feature Management) Improvements - Add _priority_lock threading lock to feature_mcp.py to prevent race conditions when multiple features are created simultaneously - Apply lock to feature_create, feature_create_bulk, and feature_skip - Add checkAndSendTimeoutRef cleanup in useAssistantChat.ts to prevent memory leaks on component unmount - Clear currentAssistantMessageRef on response_done ## Code Quality - All Python files pass ruff linting - All security tests pass (91/91) - UI passes ESLint and TypeScript compilation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 participants
Summary
Allow users to interact with the project assistant to create features through natural conversation. Previously the assistant was read-only; now it can manage the project backlog while keeping file system access read-only.
New capabilities:
Security model preserved:
Changes
mcp_server/feature_mcp.py- Addfeature_createtool for single-feature creationserver/services/assistant_chat_session.py- Update allowed tools and system promptui/src/hooks/useAssistantChat.ts- Enhance tool call display with friendly messagesTest plan
Summary by CodeRabbit
New Features
Style
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.