Improved MCP and global rules instructions#705
Conversation
WalkthroughThe PR updates instructional content and rules text in the UI and server, changes RAG tool parameters from source_domain to source_id, adjusts default task pagination from 50 to 10, and tweaks task creation defaults and documentation. No runtime control flow or APIs changed beyond the two RAG tool function signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant MCP Server
participant RAG Service as RAG API
rect rgb(245,248,255)
note over Client,MCP Server: Targeted RAG search using source_id
Client->>MCP Server: rag_get_available_sources()
MCP Server-->>Client: List of sources (id, name, type)
Client->>MCP Server: rag_search_knowledge_base(query, source_id)
MCP Server->>RAG Service: POST /api/rag/query { query, source: source_id }
RAG Service-->>MCP Server: { success, results, reranked, error? }
MCP Server-->>Client: Results
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
python/src/mcp_server/features/tasks/task_tools.py (1)
271-287: Treat any 2xx as success (handle 201/204).Create/Update/Delete only accept 200; APIs often return 201 Created or 204 No Content. Use
response.is_success.Apply:
- if response.status_code == 200: + if response.is_success: result = response.json()- if response.status_code == 200: + if response.is_success: result = response.json()- if response.status_code == 200: + if response.is_success: result = response.json()Also applies to: 323-338, 351-359
python/src/mcp_server/mcp_server.py (1)
223-241: Instructions mismatch: use find_tasks, not list_tasks.The tool is registered as
find_tasks; the instructions referencelist_tasks, causing client calls to fail.Apply:
-1. **Get current task**: `list_tasks(task_id="...")` -2. **Search/List tasks**: `list_tasks(query="auth", filter_by="status", filter_value="todo")` +1. **Get current task**: `find_tasks(task_id="...")` +2. **Search/List tasks**: `find_tasks(query="auth", filter_by="status", filter_value="todo")` @@ -### Consolidated Task Tools (Optimized ~2 tools from 5) -- `list_tasks(query=None, task_id=None, filter_by=None, filter_value=None, per_page=10)` +### Consolidated Task Tools (Optimized ~2 tools from 5) +- `find_tasks(query=None, task_id=None, filter_by=None, filter_value=None, per_page=10)` - - task_id parameter for getting single task (full details) + - task_id parameter for getting single task (full details)Also applies to: 234-242
🧹 Nitpick comments (10)
python/src/mcp_server/features/tasks/task_tools.py (1)
111-121: Validate inputs: clamp pagination and reject invalid filters.Prevent bad requests and noisy errors.
Apply:
- # List mode with search and filters + # List mode with search and filters + # Guardrails + page = 1 if page is None or page < 1 else page + per_page = 10 if per_page is None else max(1, min(per_page, 100)) + if filter_by and filter_by not in {"status", "project", "assignee"}: + return MCPErrorFormatter.format_error( + error_type="validation_error", + message=f"Unsupported filter_by: {filter_by}", + suggestion="Use one of: status | project | assignee", + ) params: dict[str, Any] = { "page": page, "per_page": per_page, "exclude_large_fields": True, # Always exclude large fields in MCP responses }Also applies to: 122-149
python/src/mcp_server/features/rag/rag_tools.py (3)
21-23: Use centralized timeout config for consistency.Align with server-wide env-driven timeouts.
Apply:
from src.server.config.service_discovery import get_api_url +from src.mcp_server.utils.timeout_config import get_default_timeout @@ - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() @@ - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() @@ - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout()Also applies to: 55-56, 103-106, 163-166
74-76: Preserve stack traces in logs.Add
exc_info=Trueper logging guidelines.Apply:
- except Exception as e: - logger.error(f"Error getting sources: {e}") + except Exception as e: + logger.error(f"Error getting sources: {e}", exc_info=True) return json.dumps({"success": False, "error": str(e)}, indent=2)- except Exception as e: - logger.error(f"Error performing RAG query: {e}") + except Exception as e: + logger.error(f"Error performing RAG query: {e}", exc_info=True) return json.dumps({"success": False, "results": [], "error": str(e)}, indent=2)- except Exception as e: - logger.error(f"Error searching code examples: {e}") + except Exception as e: + logger.error(f"Error searching code examples: {e}", exc_info=True) return json.dumps({"success": False, "results": [], "error": str(e)}, indent=2)Also applies to: 134-136, 197-199
101-110: Basic input guardrails: empty query and match_count clamp.Avoid wasteful calls and extreme result sizes.
Apply:
try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() + if not query or not query.strip(): + return json.dumps({"success": False, "results": [], "error": "query must be non-empty"}, indent=2) + match_count = max(1, min(match_count, 20))try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() + if not query or not query.strip(): + return json.dumps({"success": False, "results": [], "error": "query must be non-empty"}, indent=2) + match_count = max(1, min(match_count, 20))Also applies to: 165-173
python/src/mcp_server/mcp_server.py (2)
375-381: Add exc_info=True to error logs for full traces.Keep stack traces per logging standard.
Apply:
- except Exception as e: - logger.error(f"Health check failed: {e}") + except Exception as e: + logger.error(f"Health check failed: {e}", exc_info=True) return json.dumps({- except Exception as e: - logger.error(f"Session info failed: {e}") + except Exception as e: + logger.error(f"Session info failed: {e}", exc_info=True) return json.dumps({Also applies to: 413-419
72-81: Clarify required port message.Error mentions a “Default value: 8051” but the code fails fast (no default). Remove the misleading note.
Apply:
-raise ValueError( - "ARCHON_MCP_PORT environment variable is required. " - "Please set it in your .env file or environment. " - "Default value: 8051" -) +raise ValueError( + "ARCHON_MCP_PORT environment variable is required. " + "Please set it in your .env file or environment (e.g., 8051)." +)Also applies to: 75-79
archon-ui-main/src/components/settings/IDEGlobalRules.tsx (4)
2-2: Remove unused import.
FileCodeis unused; triggers ESLint warning.Apply:
-import { FileCode, Copy, Check } from 'lucide-react'; +import { Copy, Check } from 'lucide-react';
200-203: Drop unused variables.
codeBlockLangandlistStackaren’t used.Apply:
- let codeBlockLang = ''; - const listStack: string[] = []; + // no-op @@ - codeBlockLang = line.slice(3).trim(); + // language marker ignoredAlso applies to: 207-211
251-262: Avoid dynamic Tailwind classes; rely on inline style.
ml-${marginLeft}won’t be generated by Tailwind and is redundant with inline style.Apply:
- <li key={index} className={`ml-${marginLeft} list-disc text-gray-600 dark:text-gray-400 my-0.5`} - style={{ marginLeft: `${marginLeft * 4}px` }} + <li key={index} className="list-disc text-gray-600 dark:text-gray-400 my-0.5" + style={{ marginLeft: `${marginLeft * 4}px` }} dangerouslySetInnerHTML={{ __html: processedContent }} />Also applies to: 257-261
239-249: List semantics:outside /
.
Rendering bare
- elements is invalid HTML; consider wrapping contiguous items in
<ul>/<ol>for a11y.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
archon-ui-main/src/components/settings/IDEGlobalRules.tsx(1 hunks)python/src/mcp_server/features/rag/rag_tools.py(4 hunks)python/src/mcp_server/features/tasks/task_tools.py(13 hunks)python/src/mcp_server/mcp_server.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
python/src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
python/src/**/*.py: Fail fast on critical conditions: service startup failures, missing configuration/env vars, database connection/auth failures, critical dependencies unavailable
Never accept or store corrupted data (e.g., zero embeddings, null foreign keys, malformed JSON); skip failed items entirely and continue processing
For batch/background operations, continue processing but log detailed per-item failures; for external APIs use retries with exponential backoff and then fail clearly
Error messages must include context, use specific exception types, preserve full stack traces (logging with exc_info=True), include relevant IDs/URLs, and never return None to indicate failure—raise instead; for batch ops report success counts and detailed failures
Backend uses Python 3.12 with a 120-character line length
Avoid introducing WebSocket support in the backend; updates are handled via HTTP polling
Adhere to Ruff lint rules (e.g., no unused imports) and provide type hints to satisfy MyPy
python/src/**/*.py: Fail fast on service startup failures (credentials, DB, service init); crash with clear errors
Treat missing configuration (env vars/invalid settings) as fatal; stop the system
Do not hide database connection failures; bubble up and surface clearly
Authentication/authorization failures must halt the operation and be visible
Never silently accept bad data; let Pydantic validation errors raise
If critical dependencies are unavailable, fail immediately
Reject invalid data that could corrupt state (e.g., zero embeddings, null FKs, malformed JSON)
Batch processing should complete remaining items but log detailed per-item failures
Background tasks (e.g., embedding generation) should finish queues while logging failures
Treat optional features as skippable: log and skip when disabled rather than crashing
External API calls: use retry with exponential backoff; on final failure, raise with clear service/context info
Never accept corrupted data during partial-failure work...
Files:
python/src/mcp_server/features/rag/rag_tools.pypython/src/mcp_server/mcp_server.pypython/src/mcp_server/features/tasks/task_tools.py
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never return None/null to indicate failure; raise an exception with details instead
Files:
python/src/mcp_server/features/rag/rag_tools.pyarchon-ui-main/src/components/settings/IDEGlobalRules.tsxpython/src/mcp_server/mcp_server.pypython/src/mcp_server/features/tasks/task_tools.py
python/src/mcp_server/features/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place MCP tools under python/src/mcp_server/features/[feature]/ named find_[resource] and manage_[resource] patterns
Files:
python/src/mcp_server/features/rag/rag_tools.pypython/src/mcp_server/features/tasks/task_tools.py
archon-ui-main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
archon-ui-main/src/**/*.{ts,tsx}: Use TanStack Query for all data fetching; avoid prop drilling
TypeScript: strict mode with no implicit any in frontend code
State naming: is[Action]ing for loading flags, [resource]Error for errors, selected[Resource] for current selection
Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend
archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures (if any) should be logged and not crash the client; continue serving others
Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback
Use vertical slice architecture: place UI under src/features/[feature]/(components|hooks|services|types)
State naming: use is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Service method names: get[Resource]sByProject(projectId), getResource, create/update/delete patterns
Frontend TypeScript should be strict (no implicit any)
Files:
archon-ui-main/src/components/settings/IDEGlobalRules.tsx
archon-ui-main/src/components/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Legacy UI code should adhere to standard React ESLint rules
Files:
archon-ui-main/src/components/settings/IDEGlobalRules.tsx
archon-ui-main/src/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Legacy UI under /components should follow ESLint standard React rules
Files:
archon-ui-main/src/components/settings/IDEGlobalRules.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/components/**/*.{ts,tsx,js,jsx} : Legacy UI under /components should follow ESLint standard React rules
Applied to files:
archon-ui-main/src/components/settings/IDEGlobalRules.tsx
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/components/**/*.{js,jsx,ts,tsx} : Legacy UI code should adhere to standard React ESLint rules
Applied to files:
archon-ui-main/src/components/settings/IDEGlobalRules.tsx
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Use Biome formatting/conventions in /src/features: 120-char lines, double quotes, trailing commas
Applied to files:
archon-ui-main/src/components/settings/IDEGlobalRules.tsx
🧬 Code graph analysis (1)
python/src/mcp_server/features/tasks/task_tools.py (3)
python/src/mcp_server/features/projects/project_tools.py (1)
truncate_text(30-34)python/src/mcp_server/utils/timeout_config.py (1)
get_default_timeout(12-30)python/src/mcp_server/utils/error_handling.py (3)
MCPErrorFormatter(16-148)format_error(20-57)from_http_error(60-100)
🔇 Additional comments (3)
python/src/mcp_server/features/tasks/task_tools.py (1)
23-24: Good defaults and response slimming.Reducing default page size to 10, defaulting empty descriptions, and replacing large arrays with counts are sensible changes for MCP traffic.
Also applies to: 260-268, 275-289, 327-335
archon-ui-main/src/components/settings/IDEGlobalRules.tsx (1)
28-103: Rules content aligns with backend changes.Updated guidance (Task cycle, RAG with source_id) matches the server tools.
Also applies to: 116-191
python/src/mcp_server/features/rag/rag_tools.py (1)
78-92: API change: source_domain → source_id — verified updated.
No occurrences ofsource_domainremain; callers, UI and docs usesource_id.
| ### Project Tools | ||
| - `list_projects(project_id=None, query=None, page=1, per_page=10)` | ||
| - List all projects, search by query, or get specific project by ID | ||
| - `manage_project(action, project_id=None, title=None, description=None, github_repo=None)` | ||
| - Actions: "create", "update", "delete" | ||
|
|
||
| ### Document Tools (Consolidated) | ||
| ### Document Tools | ||
| - `list_documents(project_id, document_id=None, query=None, document_type=None, page=1, per_page=10)` | ||
| - List project documents, search, filter by type, or get specific document | ||
| - `manage_document(action, project_id, document_id=None, title=None, document_type=None, content=None, ...)` | ||
| - Actions: "create", "update", "delete" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Project/Document tool names may also be out of sync. Verify.
Docs reference list_projects/list_documents; code elsewhere uses find_*.
Run:
Also applies to: 258-263
🏁 Script executed:
#!/bin/bash
# Show all registered MCP tool names to validate against instructions
rg -nP -C1 '^\s*@mcp\.tool\(\)' python/src | rg -nP 'def\s+([a-zA-Z_][a-zA-Z0-9_]*)\s*\('
# Check instructions for list_* vs find_* occurrences
rg -nP 'list_(tasks|projects|documents)' python/src/mcp_server/mcp_server.py -n -C1Length of output: 2357
Mismatch: docs reference list_projects/list_documents but actual tools are named find_projects/find_documents — fix required.
- Docs: python/src/mcp_server/mcp_server.py (docs block ~lines 252–263) — replace list_projects → find_projects and list_documents → find_documents.
- Code: python/src/mcp_server/features/projects/project_tools.py (async def find_projects, ~line 57) and python/src/mcp_server/features/documents/document_tools.py (async def find_documents, ~line 39).
- Recommendation: follow existing find_/manage_ convention — update the docs; if you prefer list_ names, rename functions and all call sites.
🤖 Prompt for AI Agents
In python/src/mcp_server/mcp_server.py around lines 252–263, the docs reference
list_projects/list_documents but the actual tool functions are named
find_projects/find_documents (see
python/src/mcp_server/features/projects/project_tools.py ~line 57 and
python/src/mcp_server/features/documents/document_tools.py ~line 39); update the
documentation block to replace list_projects → find_projects and list_documents
→ find_documents to match the existing find_/manage_ convention (or
alternatively rename the functions and all call sites if you choose to adopt
list_ names project-wide).
…705) * Fix: conversation list status dots always gray for active workflows (#696) Worker conversations (hidden) have a different ID than the visible parent conversation shown in the sidebar. The status map was keyed on the worker conversation_id, so conv.id lookups always missed. Changes: - Use `parent_conversation_id ?? conversation_id` as the map key in ProjectDetail.tsx and AllConversationsView.tsx so web-run status resolves against the visible parent conversation - Add `parent_conversation_id` to WorkflowRunResponse interface in api.ts (field is already returned by the list endpoint) Fixes #696 * Fix: invalidate sidebar queries immediately when workflow starts running Add 'running' to the status check that triggers queryClient.invalidateQueries, so sidebar status dots update instantly via SSE instead of waiting for the 10-second poll interval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…705) * Fix: conversation list status dots always gray for active workflows (#696) Worker conversations (hidden) have a different ID than the visible parent conversation shown in the sidebar. The status map was keyed on the worker conversation_id, so conv.id lookups always missed. Changes: - Use `parent_conversation_id ?? conversation_id` as the map key in ProjectDetail.tsx and AllConversationsView.tsx so web-run status resolves against the visible parent conversation - Add `parent_conversation_id` to WorkflowRunResponse interface in api.ts (field is already returned by the list endpoint) Fixes #696 * Fix: invalidate sidebar queries immediately when workflow starts running Add 'running' to the status check that triggers queryClient.invalidateQueries, so sidebar status dots update instantly via SSE instead of waiting for the 10-second poll interval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nning ChatPage.tsx was keying the conversationStatusMap on conversation_id (the worker ID), so lookups by parent conv.id always missed. Use parent_conversation_id ?? conversation_id to match the sidebar fix from #705. workflow-store.ts only invalidated React Query caches on terminal statuses, causing a ~10s delay before the blue dot appeared. Add 'running' to the invalidation check, restoring the behavior lost during the Zustand migration (#693). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…oleam00#705) * Fix: conversation list status dots always gray for active workflows (coleam00#696) Worker conversations (hidden) have a different ID than the visible parent conversation shown in the sidebar. The status map was keyed on the worker conversation_id, so conv.id lookups always missed. Changes: - Use `parent_conversation_id ?? conversation_id` as the map key in ProjectDetail.tsx and AllConversationsView.tsx so web-run status resolves against the visible parent conversation - Add `parent_conversation_id` to WorkflowRunResponse interface in api.ts (field is already returned by the list endpoint) Fixes coleam00#696 * Fix: invalidate sidebar queries immediately when workflow starts running Add 'running' to the status check that triggers queryClient.invalidateQueries, so sidebar status dots update instantly via SSE instead of waiting for the 10-second poll interval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nning ChatPage.tsx was keying the conversationStatusMap on conversation_id (the worker ID), so lookups by parent conv.id always missed. Use parent_conversation_id ?? conversation_id to match the sidebar fix from coleam00#705. workflow-store.ts only invalidated React Query caches on terminal statuses, causing a ~10s delay before the blue dot appeared. Add 'running' to the invalidation check, restoring the behavior lost during the Zustand migration (coleam00#693). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…oleam00#705) * Fix: conversation list status dots always gray for active workflows (coleam00#696) Worker conversations (hidden) have a different ID than the visible parent conversation shown in the sidebar. The status map was keyed on the worker conversation_id, so conv.id lookups always missed. Changes: - Use `parent_conversation_id ?? conversation_id` as the map key in ProjectDetail.tsx and AllConversationsView.tsx so web-run status resolves against the visible parent conversation - Add `parent_conversation_id` to WorkflowRunResponse interface in api.ts (field is already returned by the list endpoint) Fixes coleam00#696 * Fix: invalidate sidebar queries immediately when workflow starts running Add 'running' to the status check that triggers queryClient.invalidateQueries, so sidebar status dots update instantly via SSE instead of waiting for the 10-second poll interval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nning ChatPage.tsx was keying the conversationStatusMap on conversation_id (the worker ID), so lookups by parent conv.id always missed. Use parent_conversation_id ?? conversation_id to match the sidebar fix from coleam00#705. workflow-store.ts only invalidated React Query caches on terminal statuses, causing a ~10s delay before the blue dot appeared. Add 'running' to the invalidation check, restoring the behavior lost during the Zustand migration (coleam00#693). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Pull Request
Summary
Improved MCP and global rules instructions
Changes Made
Type of Change
Affected Services
Testing
Test Evidence
Lot of manual testing for this PR to make sure that the full development flow on a real project actually follows the Archon principles laid out in the tool descriptions, MCP instructions, and global rules.
For most of my testing, I used Archon (with Graphiti docs) to build a knowledge graph implementation into an AI agent that previously was just semantic search. Claude Code (and Codex!) successfully searched the Graphiti docs with a source filter, managed tasks in Archon throughout the whole process, and the tasks were appropriate in number and scope.
Checklist
Additional Notes
I would highly recommend using the new and updated global rules for Archon that are found in the settings page of the UI!
Summary by CodeRabbit
Documentation
Refactor