Improve development environment with Docker Compose profiles#435
Improve development environment with Docker Compose profiles#435
Conversation
…end locally - Created dev.bat script to run backend services in Docker and frontend locally - Added docker-compose.backend.yml for backend-only Docker setup - Updated package.json to run frontend on port 3737 - Fixed api.ts to use default port 8181 instead of throwing error - Script automatically stops production containers to avoid port conflicts - Provides instant HMR for frontend development
… for cross-platform support and enhanced commands
…update test commands for frontend and backend
This commit enhances the development workflow by replacing the separate docker-compose.backend.yml file with Docker Compose profiles, fixing critical service discovery issues, and adding comprehensive developer tooling through an improved Makefile system. Key improvements: - Replace docker-compose.backend.yml with cleaner profile approach - Fix service discovery by maintaining consistent container names - Fix port mappings (3737:3737 instead of 3737:5173) - Add make doctor for environment validation - Fix port configuration and frontend HMR - Improve error handling with .SHELLFLAGS in Makefile - Add comprehensive port configuration via environment variables - Simplify make dev-local to only run essential services - Add logging directory creation for local development - Document profile strategy in docker-compose.yml These changes provide three flexible development modes: - Hybrid mode (default): Backend in Docker, frontend local with HMR - Docker mode: Everything in Docker for production-like testing - Local mode: API server and UI run locally Co-authored-by: Zak Stam <zaksnet@users.noreply.github.com>
WalkthroughAdds a root Makefile and check-env.js, updates docs for Make-driven workflows and installing Make, introduces Docker Compose profiles and standardized container names/ports, adjusts frontend Dockerfile and Vite config, changes frontend API port fallback and tests, tweaks UI copy, adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant MK as Makefile
participant CE as check-env.js
participant DC as docker-compose
participant FE as Frontend (Vite)
participant BE as Backend
Dev->>MK: make dev
MK->>CE: node check-env.js
CE-->>MK: success / exit(1)
alt Env OK
MK->>DC: docker compose --profile backend up -d --build
par Frontend & Backend
MK->>FE: npm run dev (port from ARCHON_UI_PORT or 3737)
FE->>BE: proxy → :${VITE_ARCHON_SERVER_PORT || 8181}
DC->>BE: start backend services
end
else Env error
CE-->>Dev: print errors & guidance
end
sequenceDiagram
autonumber
participant API as mcp_api.py
participant DKR as Docker Engine
API->>API: _get_container_status()
alt no cached reference
API->>API: _resolve_container("archon-mcp")
API->>DKR: lookup container by name
alt found
DKR-->>API: container
API-->>API: return status
else not found
DKR-->>API: not found
API-->>API: return "not_found"
end
else cached ref
API->>DKR: inspect cached container
alt NotFound
API->>API: _resolve_container() retry
API-->>API: status or "not_found"
else other error
API-->>API: return "error"
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
- Resolved Dockerfile conflict: kept our version with --host in package.json - Resolved README conflicts: kept hybrid development mode documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
archon-ui-main/vite.config.ts (1)
18-23: Incorrect Docker detection will break local dev proxying
!!process.env.HOSTNAMEis truthy on most local machines, soisDockerbecomes true outside Docker, forcinghost = 'archon-server'and breaking the proxy. Detect Docker via explicit flags or/.dockerenv.Apply this diff:
- const isDocker = process.env.DOCKER_ENV === 'true' || !!process.env.HOSTNAME; + const isDocker = + process.env.DOCKER_ENV === 'true' || + process.env.CONTAINER === 'docker' || + existsSync('/.dockerenv');docker-compose.yml (1)
126-130: Fix frontend healthcheck to the new UI port (3737)Healthcheck still probes 5173; with the new port it will always report unhealthy.
- healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:5173"] + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:3737"] interval: 30s timeout: 10s retries: 3
♻️ Duplicate comments (1)
Makefile (1)
244-245: restart should be PHONY (static analysis hint)Covered by the PHONY addition diff above. Nothing else to do.
🧹 Nitpick comments (19)
.gitignore (1)
6-7: Scope the ignore to the top-level logs dir and allow a placeholder fileUsing
logswill ignore any file/dir named "logs" anywhere in the repo. If the intent is only the rootlogs/created by the Makefile, scope it and allow a.gitkeepso the directory structure can be committed if needed.Apply this diff:
-logs +/logs/ +!/logs/.gitkeeparchon-ui-main/src/config/api.ts (1)
61-63: Avoid dual sources of truth for API base path
API_BASE_URLis hardcoded to/api, whilegetApiBasePath()derives it based ongetApiUrl(). Prefer one approach to prevent drift. If the codebase always uses the proxy, keep only the constant and remove the function; otherwise, standardize on the function.I can follow up with a repo scan to see which is actually used and send a PR to remove the dead one.
archon-ui-main/vite.config.ts (4)
281-281: Minor: ensure CLI doesn’t override this and use Number() for clarityGiven the
package.jsonhard-codes--port 3737, this config value is currently ignored. After removing the CLI flag, considerNumber(...)for readability.Apply this diff:
- port: parseInt(process.env.ARCHON_UI_PORT || env.ARCHON_UI_PORT || '3737'), // Use configurable port + port: Number(process.env.ARCHON_UI_PORT || env.ARCHON_UI_PORT || '3737'), // Use configurable port
69-76: Use spawn instead of exec to stream output safely
execbuffers stdout/stderr and has a 200KB limit; long test output can truncate or crash.spawnstreams without buffer limits and is more suitable for SSE.High-level change (illustrative; apply to all three exec sites):
- const testProcess = exec('npm run test -- --run', { - cwd: process.cwd() - }); + const testProcess = spawn('npm', ['run', 'test', '--', '--run'], { + cwd: process.cwd(), + stdio: ['ignore', 'pipe', 'pipe'], + env: process.env + });Don’t forget to import:
-import { exec } from 'child_process'; +import { spawn } from 'child_process';Also applies to: 148-156, 234-236
22-23: Disambiguate API vs UI ports to avoid confusion
portis the backend API port, but later you also configure the Vite server port. Rename toapiPortanduiPortfor clarity and propagate todefine/test.env.Sketch:
- const port = process.env.ARCHON_SERVER_PORT || env.ARCHON_SERVER_PORT || '8181'; + const apiPort = process.env.ARCHON_SERVER_PORT || env.ARCHON_SERVER_PORT || '8181'; ... - target: `http://${host}:${port}`, + target: `http://${host}:${apiPort}`, ... - 'import.meta.env.VITE_PORT': JSON.stringify(port), + 'import.meta.env.VITE_PORT': JSON.stringify(apiPort), ... - VITE_PORT: port, + VITE_PORT: apiPort,Also applies to: 285-286, 310-311, 331-333
279-307: Gate dev-only endpoints behind a flagExposing
/api/run-tests*and/api/generate-coverageon0.0.0.0withAccess-Control-Allow-Origin: *is risky if a dev machine is reachable on a LAN. Wrap registration of these routes behind anENABLE_DEV_TEST_ENDPOINTSenv flag.I can add a small helper to conditionally register these middlewares based on
process.env.ENABLE_DEV_TEST_ENDPOINTS === 'true'.Makefile (5)
8-8: Declare all PHONY targets and add an “all” alias to satisfy checkmakeStatic analysis flagged missing “all” and “restart” PHONY. Also many targets are missing from .PHONY. Add them and provide an all alias.
-.PHONY: help dev dev-hybrid dev-docker prod build test clean clean-confirm deep-clean deep-clean-confirm stop logs status install check-env +.PHONY: all help install check-frontend-deps check-env \ + dev dev-hybrid dev-docker dev-local stop-local prod backend frontend stop-prod build \ + test test-frontend test-backend test-coverage stop clean clean-confirm deep-clean deep-clean-confirm \ + logs logs-% status restart lint-frontend lint-backend typecheck format pre-commit health \ + watch-backend watch-mcp watch-agents shell-backend shell-mcp migrate backup-db doctor + +all: help
104-112: Prefer readiness wait over fixed sleep in dev-localA fixed 3s sleep is brittle. Wait until the health endpoint is up or timeout.
@cd python && uv run python -m src.server.main > ../logs/api-server.log 2>&1 & - @sleep 3 + @echo "Waiting for API health endpoint..." + @for i in $$(seq 1 30); do \ + curl -fsS http://localhost:8181/health >/dev/null 2>&1 && { echo "API ready"; break; }; \ + sleep 1; \ + if [ $$i -eq 30 ]; then echo "ERROR: API did not become ready on :8181"; exit 1; fi; \ + done
53-55: Use npm ci for deterministic installs in CI/dev checksnpm ci is faster and honors the lockfile.
check-frontend-deps: - @cd archon-ui-main && npm install --silent + @cd archon-ui-main && npm ci --silent
175-179: docker-compose down is fine, but consider “docker compose” v2 syntax going forwardThe hyphenated docker-compose is legacy; v2 ships as “docker compose”. Not a blocker; consider migrating in a follow-up to match modern Docker.
267-274: Minor: health prints static URLs; consider verifying with curl and returning HTTP codeNot required for this PR, but turning these into active checks would make the target more actionable.
docker-compose.yml (5)
19-19: Align container_name with stated standard “archon-server”PR objective mentions standardizing to names like archon-server, but this value is “Archon-Server”. Use lowercase hyphenated names for consistency across docs, scripts, and service discovery.
- container_name: Archon-Server + container_name: archon-server
56-56: Consistent container_nameMirror the naming convention here as well.
- container_name: Archon-MCP + container_name: archon-mcp
93-93: Consistent container_nameMirror the naming convention here as well.
- container_name: Archon-Agents + container_name: archon-agents
116-116: Consistent container_nameMirror the naming convention here as well.
- container_name: Archon-UI + container_name: archon-ui
3-3: Whitespace: remove trailing spaces flagged by yamllintHarmless but easy to fix.
-# - "frontend": Starts only the frontend UI service +# - "frontend": Starts only the frontend UI serviceREADME.md (3)
180-186: Container name column mismatches compose container_name valuesTable lists archon-ui/archon-server/etc. while docker-compose currently uses capitalized container_name values (e.g., Archon-UI). Either:
- update docker-compose container_name values to lowercase hyphenated (recommended; see compose comments), or
- rename the column to “Service Name” and list archon-server, archon-mcp, etc., which match docker-compose service keys.
-| Service | Container Name | Default URL | Purpose | +| Service | Service Name | Default URL | Purpose |
64-66: Clarify “legacy” Supabase service key wording“Use the legacy one (the longer one)” is ambiguous and could age poorly. Link to Supabase docs or specify the exact key name shown in the dashboard.
I can propose a short, future-proof note or add a docs link if you prefer.
96-108: Minor: table formatting warnings from markdownlint are benignIf you want to silence MD051/format warnings, add a blank line before the table and ensure header delimiter lengths match. Not blocking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.env.example(1 hunks).gitignore(1 hunks)Makefile(1 hunks)README.md(5 hunks)archon-ui-main/Dockerfile(1 hunks)archon-ui-main/package.json(1 hunks)archon-ui-main/src/config/api.ts(1 hunks)archon-ui-main/vite.config.ts(1 hunks)docker-compose.yml(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}: Remove dead code immediately; do not keep legacy/unused functions
Avoid comments that reference change history (e.g., LEGACY, CHANGED, REMOVED); keep comments focused on current functionality
Files:
archon-ui-main/src/config/api.ts
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL
Applied to files:
README.md
🪛 markdownlint-cli2 (0.17.2)
README.md
46-46: Link fragments should be valid
(MD051, link-fragments)
🪛 LanguageTool
README.md
[grammar] ~83-~83: There might be a mistake here.
Context: ...starts all core microservices in Docker: - Server: Core API and business logic (P...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ... website ### 🚀 Quick Command Reference | Command | Description | |---------|----...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...and Reference | Command | Description | |---------|-------------| | make dev |...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ... Description | |---------|-------------| | make dev | Start hybrid dev environm...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ...nt (backend in Docker, frontend local) | | make prod | Start production environ...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...production environment (all in Docker) | | make install | Install all dependenc...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ...ke install| Install all dependencies | |make test| Run all tests | |make ...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...encies | | make test | Run all tests | | make stop | Stop all services | | `m...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ...ts | | make stop | Stop all services | | make logs | View service logs | | `m...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ...es | | make logs | View service logs | | make status | Check service status |...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...| make status | Check service status | | make help | Show all available comma...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...ing.
(QB_NEW_EN)
[grammar] ~338-~338: There might be a mistake here.
Context: ...nt Modes #### Hybrid Mode (Recommended) Best for active development with instant...
(QB_NEW_EN)
[grammar] ~349-~349: There might be a mistake here.
Context: ...t Docker rebuilds #### Full Docker Mode For testing production-like environment:...
(QB_NEW_EN)
[grammar] ~357-~357: There might be a mistake here.
Context: ...es rebuild) #### Manual Service Control Start individual services: ```bash make ...
(QB_NEW_EN)
🪛 checkmake (0.2.2)
Makefile
[warning] 11-11: Target body for "help" exceeds allowed length of 5 (32).
(maxbodylength)
[warning] 181-181: Target body for "clean" exceeds allowed length of 5 (14).
(maxbodylength)
[warning] 306-306: Target body for "doctor" exceeds allowed length of 5 (26).
(maxbodylength)
[warning] 8-8: Missing required phony target "all"
(minphony)
[warning] 244-244: Target "restart" should be declared PHONY.
(phonydeclared)
🪛 YAMLlint (1.37.1)
docker-compose.yml
[error] 3-3: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
archon-ui-main/Dockerfile (1)
21-25: LGTM: Port exposure and CMD align with Vite configEXPOSE 3737 matches the new dev port; npm run dev delegates host/port flags from package.json. No issues.
docker-compose.yml (1)
120-123: Caution: HOST env may clash with Vite/Node semanticsYou’re passing HOST into the container and also letting Vite bind with --host via package.json. If HOST is repurposed elsewhere in Node/Vite context, it could introduce surprises. Not blocking; just document clearly or scope to ARCHON_HOST to avoid conflicts.
Would you like me to scan the UI codebase for HOST usage to ensure there are no collisions?
| # View logs | ||
| make logs # All services | ||
| make logs-server # Specific service | ||
| make watch-backend # Watch backend logs | ||
|
|
||
| # Health check |
There was a problem hiding this comment.
Replace logs-server with the actual pattern-based target
Makefile provides logs-%; use the concrete service name.
- make logs-server # Specific service
+ make logs-archon-server # Specific service
+ # or any service: make logs-archon-mcp, make logs-archon-agents, make logs-frontend📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # View logs | |
| make logs # All services | |
| make logs-server # Specific service | |
| make watch-backend # Watch backend logs | |
| # Health check | |
| # View logs | |
| make logs # All services | |
| make logs-archon-server # Specific service | |
| # or any service: make logs-archon-mcp, make logs-archon-agents, make logs-frontend | |
| make watch-backend # Watch backend logs | |
| # Health check |
🤖 Prompt for AI Agents
In README.md around lines 398 to 403, the example "make logs-server" references
a non-existent literal target instead of the Makefile's pattern target; update
the text to instruct users to run the pattern-based target with a concrete
service name (e.g., "make logs-<service>" replacing <service> with the actual
service like api, backend, or worker) and optionally list common service names
used in the repo so readers know which concrete names to use.
The stop command now explicitly specifies all profiles to ensure all containers are stopped regardless of how they were started.
- Changed 'make lint' to 'make lint-frontend' and 'make lint-backend' - Removed non-existent 'make logs-server' command - Added 'make watch-mcp' and 'make watch-agents' commands - All documented make commands now match what's available in Makefile
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docker-compose.yml (2)
159-163: Frontend healthcheck still probes 5173; update to 3737 to match new UI portThe container now serves on 3737, but the healthcheck curls 5173, causing unhealthy status even when the UI is up.
healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:5173"] + test: ["CMD", "curl", "-f", "http://localhost:3737/"] interval: 30s timeout: 10s retries: 3
19-19: Container names are inconsistent with PR goals/docs; normalize to lowercase hyphenatedPR summary and README list container names as archon-server, archon-mcp, archon-agents, archon-ui, but compose sets Archon-Server, etc. This mismatch confuses users and breaks docs that reference names in docker ps.
- container_name: Archon-Server + container_name: archon-server @@ - container_name: Archon-MCP + container_name: archon-mcp @@ - container_name: Archon-Agents + container_name: archon-agents @@ - container_name: Archon-UI + container_name: archon-uiSide note: service discovery inside the network uses service names (archon-server, …) which are already correct; this change is for UX/logs consistency.
Also applies to: 74-75, 119-120, 149-151
♻️ Duplicate comments (7)
README.md (3)
102-114: Quick Command Reference is missingmake lintvs. actual Makefile targetsThe table doesn’t show lint, but later sections use make lint. Either add a lint alias in the Makefile or adjust docs to lint-frontend/lint-backend.
Option A (preferred): add a Makefile alias lint: lint-frontend lint-backend (see Makefile comment below).
Option B: change docs to show make lint-frontend and make lint-backend.
413-417: Fix logs example to use concrete service names with the pattern targetMakefile exposes logs-%; there is no literal logs-server target.
-make logs-server # Specific service +make logs-archon-server # Specific service +# or: +# make logs-archon-mcp +# make logs-archon-agents +# make logs-frontend
82-87: Correct Full Docker command to include the required “full” profileWithout a profile, docker-compose up starts no services in this config. Use --profile full.
**Option B: Full Docker Mode** ```bash - make prod - # or - docker-compose up --build -d + make prod + # or (explicit profiles) + docker-compose --profile full up --build -d</blockquote></details> <details> <summary>Makefile (4)</summary><blockquote> `122-128`: **stop-local may leave the UI running; kill by common names and port 3737** pkill -f "npx vite" misses npm-run Vite processes. Add broader matches and a port-based fallback. ```diff stop-local: @echo "Stopping local services..." @pkill -f "python -m src.server.main" || true - @pkill -f "npx vite" || true + # Try by known process names first + @pkill -f "vite" || true + @pkill -f "node .*vite" || true + # Fallback: kill anything bound to the UI port + @lsof -ti :${ARCHON_UI_PORT:-3737} | xargs -r kill -9 2>/dev/null || true @echo "Local services stopped"
246-253: Provide a top-level lint alias to match README usageThis keeps docs and Makefile consistent and encourages a one-shot lint.
# Lint frontend code lint-frontend: @cd archon-ui-main && npm run lint @@ # Lint backend code lint-backend: @cd python && uv run ruff check + +# Lint all (frontend + backend) +lint: lint-frontend lint-backend +.PHONY: lintAlso applies to: 263-266
56-59: Harden .env validation: ignore comments and require non-empty valuesCurrent check-env passes when keys are commented or empty. Use a stricter parser.
check-env: - @node -e "const fs=require('fs'); if(!fs.existsSync('.env')){console.error('ERROR: .env file not found! Create one from .env.example');process.exit(1);} const env=fs.readFileSync('.env','utf8'); const missing=[]; if(!env.includes('SUPABASE_URL=')){missing.push('SUPABASE_URL');} if(!env.includes('SUPABASE_SERVICE_KEY=')){missing.push('SUPABASE_SERVICE_KEY');} if(missing.length>0){console.error('ERROR: Missing required env vars: '+missing.join(', '));process.exit(1);} console.log('Environment OK: SUPABASE_URL and SUPABASE_SERVICE_KEY found.');" + @node -e "const fs=require('fs'); const p='.env'; if(!fs.existsSync(p)){console.error('ERROR: .env file not found! Create one from .env.example');process.exit(1);} const lines=fs.readFileSync(p,'utf8').split(/\\r?\\n/).filter(l=>!l.trim().startsWith('#')&&l.includes('=')); const kv=Object.fromEntries(lines.map(l=>{const i=l.indexOf('='); return [l.slice(0,i).trim(), l.slice(i+1).trim()];})); const req=['SUPABASE_URL','SUPABASE_SERVICE_KEY']; const missing=req.filter(k=>!(k in kv)||!kv[k]); if(missing.length){console.error('ERROR: Missing required env vars (non-empty): '+missing.join(', '));process.exit(1);} console.log('Environment OK: SUPABASE_URL and SUPABASE_SERVICE_KEY found.');"
310-316: doctor uses $(shell …) which runs at parse time and fails when tools aren’t installedMove version commands to runtime with $$(…) guarded by command -v checks.
- @command -v docker >/dev/null 2>&1 && echo "✅ Docker: $(shell docker --version | cut -d' ' -f3)" || echo "❌ Docker: Not installed" - @command -v docker-compose >/dev/null 2>&1 && echo "✅ Docker Compose: $(shell docker-compose --version | cut -d' ' -f4)" || echo "❌ Docker Compose: Not installed" - @command -v node >/dev/null 2>&1 && echo "✅ Node.js: $(shell node --version)" || echo "❌ Node.js: Not installed" - @command -v npm >/dev/null 2>&1 && echo "✅ npm: $(shell npm --version)" || echo "❌ npm: Not installed" - @command -v python3 >/dev/null 2>&1 && echo "✅ Python: $(shell python3 --version | cut -d' ' -f2)" || echo "❌ Python: Not installed" - @command -v uv >/dev/null 2>&1 && echo "✅ uv: $(shell uv --version | cut -d' ' -f2)" || echo "❌ uv: Not installed (needed for local development)" - @command -v make >/dev/null 2>&1 && echo "✅ Make: $(shell make --version | head -1 | cut -d' ' -f3)" || echo "❌ Make: Not installed" + @if command -v docker >/dev/null 2>&1; then echo "✅ Docker: $$(docker --version | awk '{print $$3}')"; else echo "❌ Docker: Not installed"; fi + @if command -v docker-compose >/dev/null 2>&1; then echo "✅ Docker Compose: $$(docker-compose --version | awk '{print $$4}')"; else echo "❌ Docker Compose: Not installed"; fi + @if command -v node >/dev/null 2>&1; then echo "✅ Node.js: $$(node --version)"; else echo "❌ Node.js: Not installed"; fi + @if command -v npm >/dev/null 2>&1; then echo "✅ npm: $$(npm --version)"; else echo "❌ npm: Not installed"; fi + @if command -v python3 >/dev/null 2>&1; then echo "✅ Python: $$(python3 --version | awk '{print $$2}')"; else echo "❌ Python: Not installed"; fi + @if command -v uv >/dev/null 2>&1; then echo "✅ uv: $$(uv --version | awk '{print $$2}')"; else echo "❌ uv: Not installed (needed for local development)"; fi + @if command -v make >/dev/null 2>&1; then echo "✅ Make: $$(make --version | head -1 | awk '{print $$3}')"; else echo "❌ Make: Not installed"; fi
🧹 Nitpick comments (8)
docker-compose.yml (2)
153-156: Ensure UI receives the configured API port; set VITE_ARCHON_SERVER_PORT (keep VITE_API_URL for back-compat)Related changes mention the UI reads VITE_ARCHON_SERVER_PORT at runtime with default 8181. Only VITE_API_URL is set here. Set both to avoid drift across branches and keep older builds working.
environment: - - VITE_API_URL=http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181} - - ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181} - - HOST=${HOST:-localhost} + - VITE_API_URL=http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181} + - VITE_ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181} + - ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181} + - HOST=${HOST:-localhost}Please confirm archon-ui-main/src/config/api.ts still falls back to VITE_API_URL if present; otherwise we should remove VITE_API_URL entirely and rely on the new variable + hostname.
1-4: Trim trailing whitespace in header comment (yamllint)
Minor, but keeps lint noise down.-# - "frontend": Starts only the frontend UI service +# - "frontend": Starts only the frontend UI serviceREADME.md (3)
130-133: Also fix the restart example after DB reset to include the profile flagSame profiles behavior applies here; without it, nothing will start.
- docker-compose up -d + docker-compose --profile full up -d
45-49: Broken section anchor: link to “Installing Make” doesn’t match the heading’s generated anchorMD051 likely triggers because the emoji in “## 🛠️ Installing Make” alters the slug. Simplest fix: drop the emoji in the heading so the anchor is #installing-make, matching the link already used in Prerequisites.
-## 🛠️ Installing Make +## Installing MakeIf you prefer to keep the emoji, update the link to the generated slug (typically “#-installing-make”) and verify on GitHub.
Also applies to: 142-166
187-193: Container names in the table should match ComposeTable shows container names as archon-. Compose currently sets Archon- (capitalized). Recommend normalizing Compose (preferred; see docker-compose comment) or adjust this table to match the real names to avoid confusion when users run docker ps.
Makefile (3)
8-9: Add missing PHONY targets and a conventional all targetcheckmake flags restart not PHONY and missing all. Also add lint alias here.
-.PHONY: help dev dev-hybrid dev-docker prod build test clean clean-confirm deep-clean deep-clean-confirm stop logs status install check-env +.PHONY: help dev dev-hybrid dev-docker dev-local prod backend frontend stop-prod build test test-frontend test-backend test-coverage clean clean-confirm deep-clean deep-clean-confirm stop logs logs-% status restart install check-env lint lint-frontend lint-backend typecheck format pre-commit health watch-backend watch-mcp watch-agents shell-backend shell-mcp migrate backup-db doctor all+.PHONY: all +all: help
73-77: Echo dynamic ports in help banners to reflect environment overridesRight now the banner always prints 8181/8051/8052/3737, which is misleading if users set custom ports via .env.
@echo "Backend Services in Docker:" - @echo " API Server: http://localhost:8181" - @echo " MCP Server: http://localhost:8051" - @echo " Agents Service: http://localhost:8052" + @echo " API Server: http://localhost:${ARCHON_SERVER_PORT:-8181}" + @echo " MCP Server: http://localhost:${ARCHON_MCP_PORT:-8051}" + @echo " Agents Service: http://localhost:${ARCHON_AGENTS_PORT:-8052}" @@ - @echo "UI available at: http://localhost:3737" + @echo "UI available at: http://localhost:${ARCHON_UI_PORT:-3737}"Also applies to: 92-93
174-179: Simplify stop: profiles aren’t needed for downdocker-compose down ignores profiles; the extra flags add noise.
stop: @echo "Stopping all containers..." - @docker-compose --profile backend --profile frontend --profile full down || true + @docker-compose down || true @echo "All services stopped"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.env.example(1 hunks)Makefile(1 hunks)README.md(5 hunks)archon-ui-main/Dockerfile(1 hunks)docker-compose.yml(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .env.example
- archon-ui-main/Dockerfile
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL
Applied to files:
README.mdMakefile
🪛 markdownlint-cli2 (0.17.2)
README.md
48-48: Link fragments should be valid
(MD051, link-fragments)
🪛 LanguageTool
README.md
[grammar] ~89-~89: There might be a mistake here.
Context: ...starts all core microservices in Docker: - Server: Core API and business logic (P...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... website ### 🚀 Quick Command Reference | Command | Description | |---------|----...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ...and Reference | Command | Description | |---------|-------------| | make dev |...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ... Description | |---------|-------------| | make dev | Start hybrid dev environm...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...nt (backend in Docker, frontend local) | | make prod | Start production environ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...production environment (all in Docker) | | make install | Install all dependenc...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...ke install| Install all dependencies | |make test| Run all tests | |make ...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...encies | | make test | Run all tests | | make stop | Stop all services | | `m...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...ts | | make stop | Stop all services | | make logs | View service logs | | `m...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...es | | make logs | View service logs | | make status | Check service status |...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...| make status | Check service status | | make help | Show all available comma...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...ing.
(QB_NEW_EN)
[grammar] ~353-~353: There might be a mistake here.
Context: ...nt Modes #### Hybrid Mode (Recommended) Best for active development with instant...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ...t Docker rebuilds #### Full Docker Mode For testing production-like environment:...
(QB_NEW_EN)
[grammar] ~372-~372: There might be a mistake here.
Context: ...es rebuild) #### Manual Service Control Start individual services: ```bash make ...
(QB_NEW_EN)
🪛 checkmake (0.2.2)
Makefile
[warning] 8-8: Missing required phony target "all"
(minphony)
[warning] 244-244: Target "restart" should be declared PHONY.
(phonydeclared)
[warning] 11-11: Target body for "help" exceeds allowed length of 5 (32).
(maxbodylength)
[warning] 181-181: Target body for "clean" exceeds allowed length of 5 (14).
(maxbodylength)
[warning] 306-306: Target body for "doctor" exceeds allowed length of 5 (26).
(maxbodylength)
🪛 YAMLlint (1.37.1)
docker-compose.yml
[error] 3-3: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
README.md (1)
89-94: “Agents (coming soon!)” wording is outdatedarchon-agents is defined in docker-compose.yml and Makefile. Update the note or confirm if it’s still gated behind a feature flag.
Proposed tweak:
- - **Agents (coming soon!)**: AI operations and streaming (Port: 8052) + - **Agents**: AI operations and streaming (Port: 8052)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
327-331: Restart example should pass the required Compose profileWithout
--profile full,docker-compose up -dwon’t start services in this repo’s profile-gated setup.Apply this diff:
1. Restart Docker containers: `docker-compose down && docker-compose up -d` + - With profiles: `docker-compose down && docker-compose --profile full up -d`
♻️ Duplicate comments (1)
README.md (1)
83-87: Include Docker Compose profile in “Full Docker Mode” commandAll services are behind profiles;
docker-compose up --build -dwill start nothing without--profile full. This was flagged earlier and still applies here.Apply this diff:
**Option B: Full Docker Mode** ```bash make prod # or - docker-compose up --build -d + docker-compose --profile full up --build -d</blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (6)</summary><blockquote> <details> <summary>README.md (6)</summary><blockquote> `102-114`: **Expand Quick Command Reference to include doctor and clarify profiles** Add `make doctor` (highlighted elsewhere) and clarify that `make prod` uses the “full” profile to mirror the Compose example. Apply this diff: ```diff | Command | Description | |---------|-------------| | `make dev` | Start hybrid dev environment (backend in Docker, frontend local) | -| `make prod` | Start production environment (all in Docker) | +| `make prod` | Start full Docker environment (profiles: full) | | `make install` | Install all dependencies | | `make test` | Run all tests | +| `make doctor` | Validate tools, env vars, ports, and Docker status | | `make stop` | Stop all services | | `make logs` | View service logs | | `make status` | Check service status | | `make help` | Show all available commands |
333-350: Add frequently used targets to “Development → Quick Commands”Include
make doctorhere as well, since it’s referenced in Troubleshooting. Optionally surfacemake dev-dockeras the “all in Docker” dev-mode.Apply this diff:
# View all available commands make help # Common development commands make install # Install all dependencies make test # Run all tests +make doctor # Validate environment and prerequisites make lint-frontend # Lint frontend code make lint-backend # Lint backend code make logs # View service logs make stop # Stop all services make clean # Clean up containers and volumes +make dev-docker # Dev: run all services in Docker
416-425: Document pattern-based logs target for specific servicesThe Makefile exposes pattern logs (e.g.,
logs-%). Add one concrete example to guide users.Apply this diff:
# View logs make logs # All services +make logs-archon-server # Specific service (pattern: logs-<service>) make watch-backend # Watch backend logs make watch-mcp # Watch MCP logs make watch-agents # Watch agents logs
61-66: Offer .env-based API key configuration as an alternative (from team learnings)Based on retrieved learnings, consider documenting that API keys can be set via
.envin addition to the UI. This can help in CI or headless setups.Apply this diff near either the env copy step or API key step:
# Edit .env and add your Supabase credentials: # SUPABASE_URL=https://your-project.supabase.co # SUPABASE_SERVICE_KEY=your-service-key-here +# (Optional) You can also set provider keys via env: +# OPENAI_API_KEY=sk-... +# LOGFIRE_TOKEN=... +# LOG_LEVEL=infoor:
- Go to **Settings** → Select your LLM/embedding provider and set the API key (OpenAI is default) + - Go to **Settings** → Select your LLM/embedding provider and set the API key (OpenAI is default) + - Alternatively, set keys in `.env` (e.g., `OPENAI_API_KEY`) if you prefer env-driven configIf you want, I can scan the Makefile and compose files to confirm which env var names are consumed and generate a precise list.
Also applies to: 97-101
371-379: Minor wording tweak: “Full Docker Mode” vs.make dev-dockerThis section introduces “Full Docker Mode” under Development but uses
make dev-dockerwhile the Quick Start usesmake prodfor full Docker. A short cross-reference will prevent user confusion.Apply this diff:
#### Full Docker Mode For testing production-like environment: ```bash make dev-docker
- All services run in Docker
- Slower frontend updates (requires rebuild)
- See also Quick Start → Option B which uses
make prod(full profile)--- `89-97`: **Clarify that ports are configurable and tie back to .env keys** Good call-out that ports are configurable. Add a quick reference to the specific env keys to reduce back-and-forth. Apply this diff: ```diff - **UI**: Web interface (Port: 3737) - Ports are configurable in your .env as well! + Ports are configurable in your .env as well (ARCHON_UI_PORT, ARCHON_SERVER_PORT, ARCHON_MCP_PORT, ARCHON_AGENTS_PORT).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL
Applied to files:
README.md
🪛 markdownlint-cli2 (0.17.2)
README.md
48-48: Link fragments should be valid
(MD051, link-fragments)
468-468: Link fragments should be valid
(MD051, link-fragments)
🪛 LanguageTool
README.md
[grammar] ~89-~89: There might be a mistake here.
Context: ...starts all core microservices in Docker: - Server: Core API and business logic (P...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... website ### 🚀 Quick Command Reference | Command | Description | |---------|----...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ...and Reference | Command | Description | |---------|-------------| | make dev |...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ... Description | |---------|-------------| | make dev | Start hybrid dev environm...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...nt (backend in Docker, frontend local) | | make prod | Start production environ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...production environment (all in Docker) | | make install | Install all dependenc...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...ke install| Install all dependencies | |make test| Run all tests | |make ...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...encies | | make test | Run all tests | | make stop | Stop all services | | `m...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...ts | | make stop | Stop all services | | make logs | View service logs | | `m...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...es | | make logs | View service logs | | make status | Check service status |...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...| make status | Check service status | | make help | Show all available comma...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...ing.
(QB_NEW_EN)
[grammar] ~354-~354: There might be a mistake here.
Context: ...nt Modes #### Hybrid Mode (Recommended) Best for active development with instant...
(QB_NEW_EN)
[grammar] ~365-~365: There might be a mistake here.
Context: ...llow network access. This is useful for: - Testing on different devices on your net...
(QB_NEW_EN)
[grammar] ~366-~366: There might be a mistake here.
Context: ...ing on different devices on your network - Accessing the UI from Docker containers ...
(QB_NEW_EN)
[grammar] ~371-~371: There might be a mistake here.
Context: ...les are in place. #### Full Docker Mode For testing production-like environment:...
(QB_NEW_EN)
[grammar] ~379-~379: There might be a mistake here.
Context: ...es rebuild) #### Manual Service Control Start individual services: ```bash make ...
(QB_NEW_EN)
[grammar] ~439-~439: There might be a mistake here.
Context: ...you make changes. ## 🔍 Troubleshooting ### Common Issues and Solutions #### Port Co...
(QB_NEW_EN)
[grammar] ~443-~443: There might be a mistake here.
Context: ...ssues and Solutions #### Port Conflicts If you see "Port already in use" errors:...
(QB_NEW_EN)
[grammar] ~457-~457: There might be a mistake here.
Context: ...` #### Docker Permission Issues (Linux) If you encounter permission errors with ...
(QB_NEW_EN)
[grammar] ~467-~467: There might be a mistake here.
Context: ...docker ``` #### Windows-Specific Issues - Make not found: Install Make via Choco...
(QB_NEW_EN)
[grammar] ~484-~484: There might be a mistake here.
Context: ... #### Frontend Can't Connect to Backend - Check backend is running: `curl http://l...
(QB_NEW_EN)
[grammar] ~485-~485: There might be a mistake here.
Context: ... to Backend - Check backend is running: curl http://localhost:8181/health - Verify port configuration in .env - Fo...
(QB_NEW_EN)
[grammar] ~486-~486: There might be a mistake here.
Context: .../health- Verify port configuration in.env- For custom ports, ensure bothARCHON_SE...
(QB_NEW_EN)
[grammar] ~489-~489: There might be a mistake here.
Context: ...PORTare set #### Docker Compose Hangs Ifdocker-compose` commands hang: ```ba...
(QB_NEW_EN)
[grammar] ~499-~499: There might be a mistake here.
Context: ...icable) ``` #### Hot Reload Not Working - Frontend: Ensure you're running in hyb...
(QB_NEW_EN)
[grammar] ~500-~500: There might be a mistake here.
Context: ...ning in hybrid mode (make dev-hybrid) for best HMR experience - Backend: Chec...
(QB_NEW_EN)
[grammar] ~501-~501: There might be a mistake here.
Context: ...k that volumes are mounted correctly in docker-compose.yml - File permissions: On some systems, mou...
(QB_NEW_EN)
[grammar] ~504-~504: There might be a mistake here.
Context: ...permission issues #### Check Your Setup Run the doctor command to validate your ...
(QB_NEW_EN)
[grammar] ~510-~510: There might be a mistake here.
Context: ...``bash make doctor ``` This will check: - Required tools installation - Environmen...
(QB_NEW_EN)
[grammar] ~511-~511: There might be a mistake here.
Context: ...ill check: - Required tools installation - Environment variables - Port availabilit...
(QB_NEW_EN)
[grammar] ~512-~512: There might be a mistake here.
Context: ...ols installation - Environment variables - Port availability - Docker daemon status...
(QB_NEW_EN)
[grammar] ~513-~513: There might be a mistake here.
Context: ...nvironment variables - Port availability - Docker daemon status ## 📈 Progress <p...
(QB_NEW_EN)
⏰ 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: Backend Tests (Python + pytest)
🔇 Additional comments (1)
README.md (1)
76-81: Hybrid dev flow is clear and aligns with Make targetsGood explanation that backend runs in Docker and frontend runs locally with HMR.
|
Claude finished @Wirasm's task —— View job I'll perform a comprehensive code review of this PR that improves the development environment with Docker Compose profiles and Makefile-driven workflows. SummaryThis PR significantly improves the development workflow by replacing the separate Previous Review CommentsFirst review - no previous comments Issues FoundTotal: 4 critical, 8 important, 12 minor 🔴 Critical (Must Fix)
🟡 Important (Should Fix)
🟢 Minor (Consider)
Security AssessmentNote: This is an early alpha project without authentication. Security focus should be on:
Minor Security Issues:
Performance Considerations
No performance concerns identified Good Practices Observed
Questionable Practices
Test CoverageCurrent Coverage: Good infrastructure coverage with health checks and validation Missing Tests:
RecommendationsMerge Decision:
Priority Actions:
Rationale: Review based on Archon V2 Alpha guidelines and CLAUDE.md principles |
- Create robust environment validation script (check-env.js) that properly parses .env files - Fix Docker healthcheck port mismatch (5173 -> 3737) - Remove hard-coded port flags from package.json to allow environment configuration - Fix Docker detection logic using /.dockerenv instead of HOSTNAME - Normalize container names to lowercase (archon-server, archon-mcp, etc.) - Improve stop-local command with port-based fallback for process killing - Fix API configuration fallback chain to include VITE_PORT - Fix Makefile shell variable expansion using runtime evaluation - Update .PHONY targets with comprehensive list - Add --profile flags to Docker Compose commands in README - Add VITE_ARCHON_SERVER_PORT to docker-compose.yml - Add Node.js 18+ to prerequisites - Use dynamic ports in Makefile help messages - Add lint alias combining frontend and backend linting - Update .env.example documentation - Scope .gitignore logs entry to /logs/ Co-Authored-By: Claude <noreply@anthropic.com>
- Add dynamic container name resolution with three-tier strategy - Support environment variables for custom container names - Add service discovery labels to docker-compose services - Update BackendStartupError with correct container name references
- Update environment variable names to use VITE_ prefix that matches production code - Fix MCP client service tests to use singleton instance export - Update default behavior tests to expect fallback to port 8181 - All 77 frontend tests now pass
Replace aggressive kill -9 with targeted process termination: - Filter processes by command name (node/vite/python/uvicorn) before killing - Use graceful SIGTERM instead of SIGKILL - Add process verification to avoid killing Docker-related processes - Improve logging with descriptive step messages
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
README.md (1)
485-491: Fix remaining anchor usage to match the normalized headingAfter removing the emoji from “Installing Make,” ensure references (here and earlier) still point to #installing-make.
🧹 Nitpick comments (24)
.gitignore (1)
6-6: Allow tracking a placeholder in logs/ without committing log filesIf you want to keep an empty logs/ directory in the repo (for dev UX), ignore the contents but allow a .gitkeep. Otherwise, feel free to skip.
-/logs/ +/logs/* +!/logs/.gitkeeparchon-ui-main/src/components/BackendStartupError.tsx (2)
31-31: Clarify where to find logs (Docker Desktop or CLI)Small UX polish: mention the CLI too so headless devs aren’t blocked.
- Check the <span className="text-red-400 font-bold">Archon API server</span> container logs in Docker Desktop for detailed error information. + Check the <span className="text-red-400 font-bold">Archon API server</span> container logs in Docker Desktop + or via CLI (<span className="text-red-400 font-semibold">docker compose logs archon-server -f --tail=200</span>) + for detailed error information.
36-36: Reference the configurable container name to match .env.exampleTie this to ARCHON_SERVER_CONTAINER_NAME so users who changed it aren’t confused.
- <p>3. Look for the Archon server container (typically named <span className="text-red-400 font-semibold">archon-server</span> or similar)</p> + <p> + 3. Look for the Archon server container (default: <span className="text-red-400 font-semibold">archon-server</span>; + configurable via <code>ARCHON_SERVER_CONTAINER_NAME</code>) + </p>check-env.js (2)
32-37: Strip inline comments (# …) from values when unquotedRight now, VAR=value # note will include the comment in the value. Strip trailing comments when not quoted.
- const value = trimmed.substring(equalIndex + 1).trim(); - - // Remove surrounding quotes if present - const unquotedValue = value.replace(/^["']|["']$/g, ''); + let value = trimmed.substring(equalIndex + 1).trim(); + // If value is unquoted, strip trailing inline comments + const isQuoted = /^["'].*["']$/.test(value); + if (!isQuoted) { + const hashIdx = value.indexOf('#'); + if (hashIdx !== -1) value = value.substring(0, hashIdx).trim(); + } + // Remove surrounding quotes if present + const unquotedValue = value.replace(/^["']|["']$/g, '');
66-72: Warn if SUPABASE_URL uses http for hosted projectsSupabase cloud endpoints are https; a gentle warning helps avoid CORS/mixed-content issues. Keep permissive for self-hosted.
try { new URL(envVars['SUPABASE_URL']); } catch (e) { console.error('ERROR: SUPABASE_URL is not a valid URL: ' + envVars['SUPABASE_URL']); process.exit(1); } + +// Advisory: hosted supabase should be https +try { + const u = new URL(envVars['SUPABASE_URL']); + if (u.hostname.endsWith('.supabase.co') && u.protocol !== 'https:') { + console.warn('WARNING: Hosted Supabase URLs should use https:// to avoid mixed-content issues.'); + } +} catch {}python/src/server/api_routes/mcp_api.py (5)
65-93: Prefer the running container when multiple label matches exist; keep error logs actionableIf multiple containers match the label, favor one that’s running; and when label filtering fails, include the stack trace to aid debugging.
- try: - containers = self.docker_client.containers.list( - all=True, - filters={"label": "archon.service=mcp"} - ) - if containers: - container = containers[0] - self.container_name = container.name - mcp_logger.info(f"Found MCP container via label: {self.container_name}") - return container - except Exception as e: - mcp_logger.debug(f"Label search failed: {e}") + try: + containers = self.docker_client.containers.list( + all=True, + filters={"label": "archon.service=mcp"}, + ) + if containers: + # Prefer a running container if present + preferred = None + for c in containers: + try: + c.reload() + if c.status == "running": + preferred = c + break + except Exception: + pass + container = preferred or containers[0] + self.container_name = container.name + mcp_logger.info(f"Found MCP container via label: {self.container_name}") + return container + except Exception as e: + mcp_logger.debug(f"Label search failed: {e}", exc_info=True)
199-205: Update “not found” guidance to reflect profiles and configurable namesThe message still mentions docker-compose and a specific name. Suggest a more accurate action hint for the new profiles/naming.
- "message": f"MCP container {self.container_name} not found. Run docker-compose up -d archon-mcp", + "message": ( + "MCP container not found. Ensure the service is started (e.g., `make dev` or " + "`docker compose --profile mcp up -d`) and the name matches " + "`ARCHON_MCP_CONTAINER_NAME` or has label `archon.service=mcp`." + ),
421-424: Remove duplicate datetime importAlready imported at the top; this inner import is redundant.
- from datetime import datetime - started_time = datetime.fromisoformat(started_at.replace("Z", "+00:00"))
750-753: Remove unused variable to satisfy Ruff/mypy and keep code cleansupabase_client is assigned but not used.
- supabase_client = get_supabase_client() - config_json = config.model_dump_json()
844-845: Avoid hard-coded “3 modules” in user-facing messageThis can drift; keep it generic until real introspection is implemented.
- "message": "MCP server is running with 3 modules registered. Tool introspection needs to be fixed.", + "message": "MCP server is running and modules are registered. Tool introspection needs to be fixed.",archon-ui-main/test/config/api.test.ts (4)
44-65: Stabilize window.location mocking and restore after testsOverriding window.location is fine, but it’s never restored. This can leak state across tests. Capture the original descriptor and restore it in afterEach.
Apply this diff within this block to make the override configurable and consistent:
- Object.defineProperty(window, 'location', { - value: { - protocol: 'http:', - hostname: 'localhost' - }, - writable: true - }); + const loc = { protocol: 'http:', hostname: 'localhost' } as const; + Object.defineProperty(window, 'location', { value: loc, configurable: true });And add the following (outside this block, e.g., near your top-level beforeEach/afterEach in this describe) to restore:
// Add near line 8–16 in this file (same describe scope) let originalLocation: Location | PropertyDescriptor; beforeEach(() => { originalLocation = Object.getOwnPropertyDescriptor(window, 'location') || window.location; }); afterEach(() => { // Restore window.location if ('value' in (originalLocation as PropertyDescriptor)) { Object.defineProperty(window, 'location', originalLocation as PropertyDescriptor); } else { Object.defineProperty(window, 'location', { value: originalLocation as Location, configurable: true }); } });
152-153: Nice: table-driven port validation now uses VITE_ARCHON_SERVER_PORTThe looped cases are clean and exercise the new env path. Consider adding a boundary negative case (e.g., invalid port like '-1' or '70000') if getApiUrl enforces/normalizes; if not, current scope is fine.
187-191: Avoid brittle double-call throw assertionsAsserting two throws in a row duplicates side effects and couples tightly to the exact error message. Prefer a single call and assert message contents.
- await expect(mcpClientService.createArchonClient()).rejects.toThrow('ARCHON_MCP_PORT environment variable is required'); - await expect(mcpClientService.createArchonClient()).rejects.toThrow('Default value: 8051'); + const err = await mcpClientService.createArchonClient().catch(e => e as Error); + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain('ARCHON_MCP_PORT'); + expect(err.message).toContain('8051'); // default port mention
217-221: Restore global.fetch after mocking to prevent cross-test bleedStore and restore fetch to avoid interference with other tests.
- const { mcpClientService } = await import('../../src/services/mcpClientService'); + const originalFetch = global.fetch; + const { mcpClientService } = await import('../../src/services/mcpClientService'); try { await mcpClientService.createArchonClient(); // Verify the fetch was called with the correct URL expect(global.fetch).toHaveBeenCalledWith( expect.stringContaining('/api/mcp/clients'), expect.objectContaining({ method: 'POST', body: expect.stringContaining('9051') }) ); } catch (error) { // If it fails due to actual API call, that's okay for this test // We're mainly testing that it constructs the URL correctly expect(error).toBeDefined(); } + finally { + global.fetch = originalFetch; + }docker-compose.yml (3)
1-8: Fix trailing whitespace in header (yamllint)YAML lint flagged trailing spaces on Line 3. Trim to keep CI/lint clean.
-# - "frontend": Starts only the frontend UI service +# - "frontend": Starts only the frontend UI service
56-67: Healthcheck quoting is correct; consider depend-on health for consumersThe inline Python check is fine. For more robust startup ordering, have dependent services wait on health rather than just existence.
- depends_on: - - archon-server - - archon-agents + depends_on: + archon-server: + condition: service_healthy + archon-agents: + condition: service_healthyNote: This uses the Compose Specification’s health-based dependency. If you’re targeting older Compose variants that don’t support conditions, keep current behavior.
154-179: Frontend port mapping and env shape align with Vite and getApiUrlExposing 3737 and providing VITE_ARCHON_SERVER_PORT keep the UI → API path predictable. One small enhancement is to also set VITE_PORT explicitly for clarity with some Vite setups.
environment: - VITE_API_URL=http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181} - VITE_ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181} + - VITE_PORT=3737 - ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181} - HOST=${HOST:-localhost}README.md (4)
45-50: Fix broken heading anchor (MD051) and avoid bare anchor mismatchesThe linter flags the “Installing Make” anchor. Easiest: remove the emoji from the heading so the fragment is predictably installing-make.
-## 🛠️ Installing Make +## Installing MakeThis will satisfy MD051 and match links like Installing Make.
85-92: Polish: convert “Ports are configurable” note into an explicit linkDirect readers to the Configuring Custom Ports section to reduce scrolling.
-Ports are configurable in your .env as well! +Ports are configurable via .env (see “Configuring Custom Ports & Hostname” below).
99-103: Wrap bare localhost URL (MD034)Wrap bare URL in <> to satisfy markdownlint and auto-linking.
- - Open http://localhost:3737 + - Open <http://localhost:3737>
340-357: Unify “dev” vs “dev-hybrid” messagingDocs mention both make dev and make dev-hybrid; since dev is an alias to dev-hybrid, prefer make dev everywhere to reduce cognitive load.
-# Start hybrid development environment (recommended for development) -make dev-hybrid +# Start hybrid development environment (recommended for development) +make devMakefile (3)
100-127: Be explicit about API port when starting locallyThe echo says “Starting API Server on port 8181…”, but the command relies on src.server.main defaults. Make the port explicit to avoid drift if defaults change.
- @cd python && uv run python -m src.server.main > ../logs/api-server.log 2>&1 & + @cd python && uv run uvicorn src.server.main:socket_app --host 0.0.0.0 --port 8181 > ../logs/api-server.log 2>&1 &
324-352: Doctor: guard lsof for Windows and fall back to docker compose detectiondoctor will fail on systems without lsof due to -e; guard it and support docker compose as fallback for version reporting.
- @command -v docker-compose >/dev/null 2>&1 && echo "✅ Docker Compose: $$(docker-compose --version | awk '{print $$4}')" || echo "❌ Docker Compose: Not installed" + @if command -v docker-compose >/dev/null 2>&1; then \ + echo "✅ Docker Compose: $$(docker-compose --version | awk '{print $$4}')"; \ + elif docker compose version >/dev/null 2>&1; then \ + echo "✅ Docker Compose: $$(docker compose version | awk 'NR==1{print $$3}')"; \ + else \ + echo "❌ Docker Compose: Not installed"; \ + fi @@ - @echo "=== Port Availability ===" - @lsof -i :8181 >/dev/null 2>&1 && echo "⚠️ Port 8181 (API Server) is in use" || echo "✅ Port 8181 (API Server) is available" - @lsof -i :8051 >/dev/null 2>&1 && echo "⚠️ Port 8051 (MCP Server) is in use" || echo "✅ Port 8051 (MCP Server) is available" - @lsof -i :8052 >/dev/null 2>&1 && echo "⚠️ Port 8052 (Agents) is in use" || echo "✅ Port 8052 (Agents) is available" - @lsof -i :3737 >/dev/null 2>&1 && echo "⚠️ Port 3737 (Frontend) is in use" || echo "✅ Port 3737 (Frontend) is available" + @echo "=== Port Availability ===" + @if command -v lsof >/dev/null 2>&1; then \ + lsof -i :8181 >/dev/null 2>&1 && echo "⚠️ Port 8181 (API Server) is in use" || echo "✅ Port 8181 (API Server) is available"; \ + lsof -i :8051 >/dev/null 2>&1 && echo "⚠️ Port 8051 (MCP Server) is in use" || echo "✅ Port 8051 (MCP Server) is available"; \ + lsof -i :8052 >/dev/null 2>&1 && echo "⚠️ Port 8052 (Agents) is in use" || echo "✅ Port 8052 (Agents) is available"; \ + lsof -i :3737 >/dev/null 2>&1 && echo "⚠️ Port 3737 (Frontend) is in use" || echo "✅ Port 3737 (Frontend) is available"; \ + else \ + echo "ℹ️ Skipping port checks (lsof not found)"; \ + fi
337-341: Unify env validation with check-env.js for non-empty valuesdoctor only greps for presence; leverage the stricter check-env.js to ensure variables are present and non-empty.
- @test -f .env && grep -q "SUPABASE_URL=" .env && echo "✅ SUPABASE_URL is set" || echo "❌ SUPABASE_URL not set" - @test -f .env && grep -q "SUPABASE_SERVICE_KEY=" .env && echo "✅ SUPABASE_SERVICE_KEY is set" || echo "❌ SUPABASE_SERVICE_KEY not set" + @$(MAKE) check-env
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
.env.example(1 hunks).gitignore(1 hunks)Makefile(1 hunks)README.md(5 hunks)archon-ui-main/src/components/BackendStartupError.tsx(1 hunks)archon-ui-main/src/config/api.ts(1 hunks)archon-ui-main/test/config/api.test.ts(5 hunks)archon-ui-main/vite.config.ts(2 hunks)check-env.js(1 hunks)docker-compose.yml(4 hunks)python/src/server/api_routes/mcp_api.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- archon-ui-main/src/config/api.ts
- archon-ui-main/vite.config.ts
🧰 Additional context used
📓 Path-based instructions (7)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}: Remove dead code immediately; do not keep legacy/unused functions
Avoid comments that reference change history (e.g., LEGACY, CHANGED, REMOVED); keep comments focused on current functionality
Files:
archon-ui-main/src/components/BackendStartupError.tsxpython/src/server/api_routes/mcp_api.py
archon-ui-main/src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in archon-ui-main/src/components/
Files:
archon-ui-main/src/components/BackendStartupError.tsx
python/src/{server,mcp,agents}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/src/{server,mcp,agents}/**/*.py: Fail fast on service startup failures, missing configuration, database connection issues, auth failures, critical dependency outages, and invalid data that would corrupt state
External API calls should use retry with exponential backoff and ultimately fail with a clear, contextual error message
Error messages must include context (operation being attempted) and relevant IDs/URLs/data for debugging
Preserve full stack traces in logs (e.g., Python logging with exc_info=True)
Use specific exception types; avoid catching broad Exception unless re-raising with context
Never signal failure by returning None/null; raise a descriptive exception instead
Files:
python/src/server/api_routes/mcp_api.py
python/src/server/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/src/server/**/*.py: For batch processing and background tasks, continue processing but log detailed per-item failures and return both successes and failures
Do not crash the server on a single WebSocket event failure; log the error and continue serving other clients
Files:
python/src/server/api_routes/mcp_api.py
python/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/**/*.py: Target Python 3.12 with a 120-character line length
Use Ruff for linting and Mypy for type checking before commit
Files:
python/src/server/api_routes/mcp_api.py
python/src/server/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep FastAPI application code under python/src/server/ (routes in api_routes/, services in services/, main in main.py)
Files:
python/src/server/api_routes/mcp_api.py
archon-ui-main/test/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place frontend tests under archon-ui-main/test/
Files:
archon-ui-main/test/config/api.test.ts
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL
Applied to files:
check-env.jsREADME.mdMakefile
🧬 Code graph analysis (1)
python/src/server/api_routes/mcp_api.py (1)
python/src/server/services/search/reranking_strategy.py (1)
from_env(227-233)
🪛 YAMLlint (1.37.1)
docker-compose.yml
[error] 3-3: trailing spaces
(trailing-spaces)
🪛 markdownlint-cli2 (0.17.2)
README.md
49-49: Link fragments should be valid
(MD051, link-fragments)
102-102: Bare URL used
(MD034, no-bare-urls)
486-486: Link fragments should be valid
(MD051, link-fragments)
🪛 LanguageTool
README.md
[grammar] ~85-~85: There might be a mistake here.
Context: ...starts all core microservices in Docker: - Server: Core API and business logic (P...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...Core API and business logic (Port: 8181) - MCP Server: Protocol interface for AI ...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...ol interface for AI clients (Port: 8051) - Agents (coming soon!): AI operations a...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...AI operations and streaming (Port: 8052) - UI: Web interface (Port: 3737) Por...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...ant updates. 5. Configure API Keys: - Open http://localhost:3737 - Go to **...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ... website ### 🚀 Quick Command Reference | Command | Description ...
(QB_NEW_EN)
[grammar] ~146-~146: There might be a mistake here.
Context: ...ing.
(QB_NEW_EN)
[grammar] ~338-~338: There might be a mistake here.
Context: ...hostname and MCP port ## 🔧 Development ### Quick Commands ```bash # Start hybrid de...
(QB_NEW_EN)
[grammar] ~375-~375: There might be a mistake here.
Context: ...ing on different devices on your network - Accessing the UI from Docker containers ...
(QB_NEW_EN)
[grammar] ~452-~452: There might be a mistake here.
Context: ...you make changes. ## 🔍 Troubleshooting ### Common Issues and Solutions #### Port Co...
(QB_NEW_EN)
[grammar] ~523-~523: There might be a mistake here.
Context: ...ning in hybrid mode (make dev-hybrid) for best HMR experience - Backend: Chec...
(QB_NEW_EN)
[grammar] ~538-~538: There might be a mistake here.
Context: ...ols installation - Environment variables - Port availability - Docker daemon status...
(QB_NEW_EN)
[grammar] ~539-~539: There might be a mistake here.
Context: ...nvironment variables - Port availability - Docker daemon status ## 📈 Progress <p...
(QB_NEW_EN)
🪛 checkmake (0.2.2)
Makefile
[warning] 17-17: Target body for "help" exceeds allowed length of 5 (32).
(maxbodylength)
[warning] 197-197: Target body for "clean" exceeds allowed length of 5 (14).
(maxbodylength)
[warning] 325-325: Target body for "doctor" exceeds allowed length of 5 (26).
(maxbodylength)
[warning] 8-8: Missing required phony target "all"
(minphony)
[warning] 8-8: Missing required phony target "clean"
(minphony)
[warning] 8-8: Missing required phony target "test"
(minphony)
[warning] 173-173: Target "test" should be declared PHONY.
(phonydeclared)
[warning] 260-260: Target "restart" should be declared PHONY.
(phonydeclared)
[warning] 271-271: Target "lint" should be declared PHONY.
(phonydeclared)
⏰ 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: Backend Tests (Python + pytest)
🔇 Additional comments (12)
.env.example (2)
39-45: Container naming block looks good and aligns with compose profilesThese entries match the compose changes and help with discovery. No issues from my side.
46-51: VITE_PORT injection and fallback verified
- Fallback logic in src/config/api.ts
getApiUrl() checks, in order,import.meta.env.VITE_ARCHON_SERVER_PORT, thenimport.meta.env.VITE_PORT, then defaults to'8181'(archon-ui-main/src/config/api.ts:24–29).- Vite configuration (vite.config.ts)
Both thedefineblock and theenvblock exposeVITE_PORTtoimport.meta.envat build time (archon-ui-main/vite.config.ts:308–311, 330–333).- Test coverage
Tests delete bothVITE_ARCHON_SERVER_PORTandVITE_PORTand assert the default 8181 is used, confirming the same fallback order (archon-ui-main/test/config/api.test.ts:44–51, setup.ts).Nit (optional): consider adding a test case where only
VITE_PORTis set (andVITE_ARCHON_SERVER_PORTis not) to explicitly cover that intermediate fallback path.archon-ui-main/test/config/api.test.ts (2)
66-84: LGTM: respects VITE_ARCHON_SERVER_PORT precedence in devThe tests correctly assert the new precedence chain and validate the http localhost case. No issues.
85-103: LGTM: https handling and custom portGood coverage for https + custom port with VITE_ARCHON_SERVER_PORT. This guards the common reverse-proxy and TLS dev scenarios.
docker-compose.yml (3)
12-35: LGTM: backend profiles + explicit container_name and labelsProfiles, stable container names, and labels improve DX and service discovery. The build arg and env passthrough for ARCHON_SERVER_PORT are coherent.
70-101: LGTM: MCP service discovery using service DNS namesAPI_SERVICE_URL and AGENTS_SERVICE_URL use compose service names; this avoids host-network coupling and works across profiles.
154-179: CORS check: ensure API allows the UI originWith the UI served at http://localhost:3737 (or ${HOST}:${ARCHON_UI_PORT}), verify FastAPI CORS settings include this origin. Otherwise, API calls will fail in browser when running full Docker.
Would you like me to scan python/src/server CORS config and open a small PR to align allowed origins with HOST/ARCHON_UI_PORT?
README.md (3)
77-83: Good: include “--profile full” in docker-compose exampleThis prevents “no services started” surprises due to profiles. Nice catch.
106-118: LGTM: Quick Command Reference matches Makefile targetsThe table reflects help/dev/prod/test/stop/logs/status/help. Consistent with Makefile.
334-335: Consistent profile usage in reset pathUsing --profile full here keeps the doc consistent. Good.
Makefile (2)
66-92: LGTM: dev alias and profile-backed backend startupAlias dev → dev-hybrid is convenient. Using profiles to start only backend services aligns with the docker-compose changes.
128-144: Improved stop-local looks solidKilling by known process names plus port fallback should reliably stop the dev servers. Nice touch with the whitelisting of process names before kill.
| // Basic validation for service key (should be non-trivial) | ||
| if (envVars['SUPABASE_SERVICE_KEY'].length < 10) { | ||
| console.error('ERROR: SUPABASE_SERVICE_KEY appears to be invalid (too short)'); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Detect anon vs service_role by decoding the JWT
Length check can pass for anon keys. Decode the JWT payload (base64url) and assert role === "service_role" for a precise, actionable failure.
-// Basic validation for service key (should be non-trivial)
-if (envVars['SUPABASE_SERVICE_KEY'].length < 10) {
- console.error('ERROR: SUPABASE_SERVICE_KEY appears to be invalid (too short)');
- process.exit(1);
-}
+// Validate service key: should be JWT with "role":"service_role"
+const key = envVars['SUPABASE_SERVICE_KEY'];
+if (!key || key.split('.').length < 3) {
+ console.error('ERROR: SUPABASE_SERVICE_KEY is not a valid JWT.');
+ process.exit(1);
+}
+try {
+ const payloadB64 = key.split('.')[1].replace(/-/g, '+').replace(/_/g, '/');
+ const padded = payloadB64 + '==='.slice((payloadB64.length + 3) % 4);
+ const payload = JSON.parse(Buffer.from(padded, 'base64').toString('utf8'));
+ const role = (payload?.role || payload?.['https://supabase.io/role'] || '').toString();
+ if (role.toLowerCase() !== 'service_role') {
+ console.error('ERROR: SUPABASE_SERVICE_KEY is not a service_role key (detected role: ' + role + '). Use the Service Role key from Supabase.');
+ process.exit(1);
+ }
+} catch (e) {
+ console.error('ERROR: Failed to inspect SUPABASE_SERVICE_KEY JWT payload:', e?.message || e);
+ process.exit(1);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Basic validation for service key (should be non-trivial) | |
| if (envVars['SUPABASE_SERVICE_KEY'].length < 10) { | |
| console.error('ERROR: SUPABASE_SERVICE_KEY appears to be invalid (too short)'); | |
| process.exit(1); | |
| } | |
| // Validate service key: should be JWT with "role":"service_role" | |
| const key = envVars['SUPABASE_SERVICE_KEY']; | |
| if (!key || key.split('.').length < 3) { | |
| console.error('ERROR: SUPABASE_SERVICE_KEY is not a valid JWT.'); | |
| process.exit(1); | |
| } | |
| try { | |
| const payloadB64 = key.split('.')[1].replace(/-/g, '+').replace(/_/g, '/'); | |
| const padded = payloadB64 + '==='.slice((payloadB64.length + 3) % 4); | |
| const payload = JSON.parse(Buffer.from(padded, 'base64').toString('utf8')); | |
| const role = (payload?.role || payload?.['https://supabase.io/role'] || '').toString(); | |
| if (role.toLowerCase() !== 'service_role') { | |
| console.error( | |
| 'ERROR: SUPABASE_SERVICE_KEY is not a service_role key (detected role: ' + role + '). ' + | |
| 'Use the Service Role key from Supabase.' | |
| ); | |
| process.exit(1); | |
| } | |
| } catch (e) { | |
| console.error('ERROR: Failed to inspect SUPABASE_SERVICE_KEY JWT payload:', e?.message || e); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In check-env.js around lines 74 to 78, the current length-based validation can
accept anon keys; instead decode the SUPABASE_SERVICE_KEY JWT payload (base64url
decode the middle segment), parse the JSON, and assert that the "role" claim
equals "service_role"; if decoding/parsing fails or role !== "service_role", log
a clear error and exit with non-zero status. Ensure you handle both base64url
variants (convert '-'/'_' if necessary) and wrap decoding/parsing in try/catch
so any malformed token yields the same actionable failure message.
| self.container = self._resolve_container() | ||
| if not self.container: | ||
| mcp_logger.warning("MCP container not found during initialization") | ||
| except Exception as e: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve stack traces in logs (exc_info=True) per server guidelines
Consistently include stack traces for unexpected failures to speed up incident triage.
- mcp_logger.error(f"Failed to initialize Docker client: {str(e)}")
+ mcp_logger.error(f"Failed to initialize Docker client: {str(e)}", exc_info=True)- mcp_logger.error(f"Error getting container status: {str(e)}")
+ mcp_logger.error(f"Error getting container status: {str(e)}", exc_info=True)- mcp_logger.error(f"Docker API error during MCP startup - error={str(e)}")
+ mcp_logger.error(f"Docker API error during MCP startup - error={str(e)}", exc_info=True)- mcp_logger.error(
- f"Exception during MCP server startup - error={str(e)}, error_type={type(e).__name__}"
- )
+ mcp_logger.error(
+ f"Exception during MCP server startup - error={str(e)}, error_type={type(e).__name__}",
+ exc_info=True,
+ )- mcp_logger.error(f"Docker API error during MCP stop - error={str(e)}")
+ mcp_logger.error(f"Docker API error during MCP stop - error={str(e)}", exc_info=True)- mcp_logger.error(
- f"Exception during MCP server stop - error={str(e)}, error_type={type(e).__name__}"
- )
+ mcp_logger.error(
+ f"Exception during MCP server stop - error={str(e)}, error_type={type(e).__name__}",
+ exc_info=True,
+ )- except APIError as e:
- if "container not found" not in str(e).lower():
- self._add_log("ERROR", f"Docker API error reading logs: {str(e)}")
- except Exception as e:
- self._add_log("ERROR", f"Error reading container logs: {str(e)}")
+ except APIError as e:
+ if "container not found" not in str(e).lower():
+ self._add_log("ERROR", f"Docker API error reading logs: {str(e)}")
+ mcp_logger.error("Docker API error reading logs", exc_info=True)
+ except Exception as e:
+ self._add_log("ERROR", f"Error reading container logs: {str(e)}")
+ mcp_logger.error("Unexpected error while reading container logs", exc_info=True)Also applies to: 143-145, 268-275, 279-281, 366-367, 376-378, 503-506
- Reduced Makefile from 344 lines (43 targets) to 83 lines (8 essential targets) - Removed unnecessary environment variables (*_CONTAINER_NAME variables) - Fixed Windows compatibility by removing Unix-specific commands - Added security fixes to check-env.js (path validation) - Simplified MCP container discovery to use fixed container names - Fixed 'make stop' to properly handle Docker Compose profiles - Updated documentation to reflect simplified workflow - Restored original .env.example with comprehensive Supabase key documentation This addresses all critical issues from code review: - Cross-platform compatibility ✅ - Security vulnerabilities fixed ✅ - 81% reduction in complexity ✅ - Maintains all essential functionality ✅ All tests pass: Frontend (77/77), Backend (267/267)
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 (1)
python/src/server/api_routes/mcp_api.py (1)
185-199: Avoid blocking the event loop: run container.start() in an executor (mirrors stop_server())
self.container.start()is a synchronous Docker API call. In an async context this can block the event loop and degrade server responsiveness. Userun_in_executoras you already do for.stop().- # Start the container - self.container.start() + # Start the container (non-blocking) + await asyncio.get_event_loop().run_in_executor( + None, self.container.start + )
♻️ Duplicate comments (3)
python/src/server/api_routes/mcp_api.py (1)
88-91: Preserve stack traces in logs (exc_info=True) and use consistent contextual loggingSeveral error paths log without stack traces, which slows incident triage. Per our server guidelines and past review, add
exc_info=True. For API layer logs, keep current string style but includeexc_info=Truefor parity.Apply:
@@ - mcp_logger.error(f"Failed to initialize Docker client: {str(e)}") + mcp_logger.error(f"Failed to initialize Docker client: {str(e)}", exc_info=True) @@ - mcp_logger.error(f"Error getting container status: {str(e)}") + mcp_logger.error(f"Error getting container status: {str(e)}", exc_info=True) @@ - mcp_logger.error(f"Docker API error during MCP startup - error={str(e)}") + mcp_logger.error(f"Docker API error during MCP startup - error={str(e)}", exc_info=True) @@ - mcp_logger.error( - f"Exception during MCP server startup - error={str(e)}, error_type={type(e).__name__}" - ) + mcp_logger.error( + f"Exception during MCP server startup - error={str(e)}, error_type={type(e).__name__}", + exc_info=True, + ) @@ - mcp_logger.error(f"Docker API error during MCP stop - error={str(e)}") + mcp_logger.error(f"Docker API error during MCP stop - error={str(e)}", exc_info=True) @@ - mcp_logger.error( - f"Exception during MCP server stop - error={str(e)}, error_type={type(e).__name__}" - ) + mcp_logger.error( + f"Exception during MCP server stop - error={str(e)}, error_type={type(e).__name__}", + exc_info=True, + ) @@ - except APIError as e: - if "container not found" not in str(e).lower(): - self._add_log("ERROR", f"Docker API error reading logs: {str(e)}") - except Exception as e: - self._add_log("ERROR", f"Error reading container logs: {str(e)}") + except APIError as e: + if "container not found" not in str(e).lower(): + self._add_log("ERROR", f"Docker API error reading logs: {str(e)}") + mcp_logger.error("Docker API error reading logs", exc_info=True) + except Exception as e: + self._add_log("ERROR", f"Error reading container logs: {str(e)}") + mcp_logger.error("Unexpected error while reading container logs", exc_info=True) @@ - api_logger.error("MCP server start API failed - error=%s", str(e)) + api_logger.error("MCP server start API failed - error=%s", str(e), exc_info=True) @@ - api_logger.error(f"MCP server stop API failed - error={str(e)}") + api_logger.error(f"MCP server stop API failed - error={str(e)}", exc_info=True) @@ - api_logger.error(f"MCP server status API failed - error={str(e)}") + api_logger.error(f"MCP server status API failed - error={str(e)}", exc_info=True) @@ - api_logger.error("MCP server logs API failed", error=str(e)) + api_logger.error("MCP server logs API failed", error=str(e), exc_info=True) @@ - api_logger.error("MCP server clear logs API failed", error=str(e)) + api_logger.error("MCP server clear logs API failed", error=str(e), exc_info=True) @@ - api_logger.error("Failed to get MCP configuration", error=str(e)) + api_logger.error("Failed to get MCP configuration", error=str(e), exc_info=True) @@ - api_logger.error("Failed to debug MCP server tools", error=str(e)) + api_logger.error("Failed to debug MCP server tools", error=str(e), exc_info=True) @@ - api_logger.error("Failed to get MCP tools", error=str(e)) + api_logger.error("Failed to get MCP tools", error=str(e), exc_info=True)Also applies to: 114-116, 235-246, 246-258, 334-344, 345-355, 472-477, 561-564, 580-583, 600-603, 619-621, 637-641, 701-703, 826-827, 837-841
check-env.js (1)
66-71: Validate SUPABASE_SERVICE_KEY by decoding JWT and checking role=service_roleLength checks accept anon keys. Decode the JWT payload and assert
role === "service_role"for an actionable failure.-// Basic validation for service key -if (envVars['SUPABASE_SERVICE_KEY'].length < 10) { - console.error('ERROR: SUPABASE_SERVICE_KEY appears to be invalid (too short)'); - console.error(' Please check your Supabase project settings'); - process.exit(1); -} +// Validate service key: must be a JWT with role=service_role +const key = envVars['SUPABASE_SERVICE_KEY']; +if (!key || key.split('.').length < 3) { + console.error('ERROR: SUPABASE_SERVICE_KEY is not a valid JWT.'); + process.exit(1); +} +try { + const payloadB64 = key.split('.')[1].replace(/-/g, '+').replace(/_/g, '/'); + const padded = payloadB64 + '==='.slice((payloadB64.length + 3) % 4); + const payload = JSON.parse(Buffer.from(padded, 'base64').toString('utf8')); + const role = (payload?.role || payload?.['https://supabase.io/role'] || '').toString().toLowerCase(); + if (role !== 'service_role') { + console.error(`ERROR: SUPABASE_SERVICE_KEY is not a service_role key (detected role: ${role || 'unknown'}).`); + console.error(' Use the "Service Role" key from Supabase project settings.'); + process.exit(1); + } +} catch (e) { + console.error('ERROR: Failed to inspect SUPABASE_SERVICE_KEY JWT payload:', e?.message || e); + process.exit(1); +}README.md (1)
334-336: Anchor link breakage: normalize the “Installing Make” headingTo satisfy MD051 and ensure the fragment
#installing-makeworks, remove the emoji from the heading.-## 🛠️ Installing Make +## Installing Make
🧹 Nitpick comments (12)
python/src/server/api_routes/mcp_api.py (4)
169-175: Update remediation message to match profiles/Make workflowThe guidance says “Run docker-compose up -d archon-mcp”, but the repo uses Compose profiles and Make targets. Suggest messaging that matches the new DX.
- "message": f"MCP container {self.container_name} not found. Run docker-compose up -d archon-mcp", + "message": ( + f"MCP container {self.container_name} not found. " + "Start it with `make dev` (hybrid) or " + "`docker-compose --profile backend up -d archon-mcp`." + ),
65-80: Allow container name override via env var (future-proof discovery)Hard-coding "archon-mcp" works with current compose, but letting ops set
ARCHON_MCP_CONTAINER(with default to "archon-mcp") makes this resilient to env variations.- def _resolve_container(self): + def _resolve_container(self): """Simple container resolution - just use fixed name.""" - if not self.docker_client: + if not self.docker_client: return None try: - # Simple: Just look for the fixed container name - container = self.docker_client.containers.get("archon-mcp") - self.container_name = "archon-mcp" + import os + name = os.getenv("ARCHON_MCP_CONTAINER", "archon-mcp") + container = self.docker_client.containers.get(name) + self.container_name = name mcp_logger.info("Found MCP container") return container except NotFound: - mcp_logger.warning("MCP container not found - is it running?") - self.container_name = "archon-mcp" + mcp_logger.warning("MCP container not found - is it running?") + self.container_name = os.getenv("ARCHON_MCP_CONTAINER", "archon-mcp") return None
720-733: Remove unused variable supabase_client
supabase_client = get_supabase_client()is assigned but never used.- supabase_client = get_supabase_client()
794-816: Gate debug placeholder tools behind an environment flag or return 501Returning a “debug_placeholder” tool in production is confusing for users and UI logic. Either hide it behind
MCP_DEBUG_PLACEHOLDER=true, or return a neutral response that states introspection isn’t implemented.Option A (flag):
- if is_running: + import os + if is_running and os.getenv("MCP_DEBUG_PLACEHOLDER", "false").lower() == "true": return { "tools": [ { "name": "debug_placeholder", "description": "MCP server is running and modules are registered, but tool introspection is not working yet", "module": "debug", "parameters": [], } ], "count": 1, "server_running": True, "source": "debug_placeholder", "message": "MCP server is running with 3 modules registered. Tool introspection needs to be fixed.", } + elif is_running: + return { + "tools": [], + "count": 0, + "server_running": True, + "source": "introspection_unavailable", + "message": "Tool introspection not implemented yet.", + }check-env.js (1)
10-15: Path “security check” is redundant given resolution logic
envPathis derived frompath.resolve(projectRoot, '.env'), sostartsWith(projectRoot)will always be true. Consider removing this to simplify flow.Makefile (2)
26-33: Check for Node and support both docker-compose and docker compose
node check-env.jsfails ungracefully when Node isn’t installed. Also support the Docker Compose plugin as many users don’t have the legacydocker-composebinary.check: @echo "Checking environment..." - @node check-env.js + @command -v node >/dev/null 2>&1 || { echo "✗ Node.js not found (install Node 18+)"; exit 1; } + @node check-env.js @echo "Checking Docker..." @docker --version > /dev/null 2>&1 && echo "✓ Docker installed" || echo "✗ Docker not found" - @docker-compose --version > /dev/null 2>&1 && echo "✓ Docker Compose installed" || echo "✗ Docker Compose not found" + @if command -v docker-compose >/dev/null 2>&1; then \ + echo "✓ Docker Compose installed"; \ + elif docker compose version >/dev/null 2>&1; then \ + echo "✓ Docker Compose (plugin) installed"; \ + else \ + echo "✗ Docker Compose not found"; \ + fi
7-18: checkmake warnings: long help targetNon-blocking, but if you care about checkmake, split long echo blocks or move to a script. Otherwise, ignore.
docker-compose.yml (3)
3-3: Trim trailing spacesMinor YAML lint fix.
-# - "frontend": Starts only the frontend UI service +# - "frontend": Starts only the frontend UI service
160-164: Frontend healthcheck uses curl which may not exist in the Node imageUse a Node-based healthcheck to avoid relying on curl/wget availability.
- healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:3737"] + healthcheck: + test: + [ + "CMD-SHELL", + "node -e \"require('http').get('http://localhost:3737', r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))\"" + ] interval: 30s timeout: 10s retries: 3
92-99: Optional: gate dependent services on health to improve startup robustnessIf you want stricter ordering, consider health-based dependencies (supported by modern Compose). Validate your Compose version first.
Example (only if your toolchain supports it):
depends_on: archon-server: condition: service_healthy archon-agents: condition: service_healthyAlso applies to: 130-139, 153-159
README.md (2)
106-118: Quick Command Reference: ensure targets exist (“lint”, “check”, etc.)After adding the
prod/doctoraliases and keepinglint, this table looks good. If you don’t add aliases, update rows accordingly.
413-415: Docs reference non-existent “make stop-local”Either remove this line or add a
stop-localtarget. Suggest removing here and relying onmake stopwhich already handles containers.-make stop -make stop-local +make stop
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
Makefile(1 hunks)README.md(5 hunks)archon-ui-main/src/config/api.ts(1 hunks)check-env.js(1 hunks)docker-compose.yml(4 hunks)python/src/server/api_routes/mcp_api.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- archon-ui-main/src/config/api.ts
🧰 Additional context used
📓 Path-based instructions (5)
python/src/{server,mcp,agents}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/src/{server,mcp,agents}/**/*.py: Fail fast on service startup failures, missing configuration, database connection issues, auth failures, critical dependency outages, and invalid data that would corrupt state
External API calls should use retry with exponential backoff and ultimately fail with a clear, contextual error message
Error messages must include context (operation being attempted) and relevant IDs/URLs/data for debugging
Preserve full stack traces in logs (e.g., Python logging with exc_info=True)
Use specific exception types; avoid catching broad Exception unless re-raising with context
Never signal failure by returning None/null; raise a descriptive exception instead
Files:
python/src/server/api_routes/mcp_api.py
python/src/server/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/src/server/**/*.py: For batch processing and background tasks, continue processing but log detailed per-item failures and return both successes and failures
Do not crash the server on a single WebSocket event failure; log the error and continue serving other clients
Files:
python/src/server/api_routes/mcp_api.py
python/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/**/*.py: Target Python 3.12 with a 120-character line length
Use Ruff for linting and Mypy for type checking before commit
Files:
python/src/server/api_routes/mcp_api.py
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}: Remove dead code immediately; do not keep legacy/unused functions
Avoid comments that reference change history (e.g., LEGACY, CHANGED, REMOVED); keep comments focused on current functionality
Files:
python/src/server/api_routes/mcp_api.py
python/src/server/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep FastAPI application code under python/src/server/ (routes in api_routes/, services in services/, main in main.py)
Files:
python/src/server/api_routes/mcp_api.py
🧠 Learnings (5)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to python/src/{server,mcp,agents}/**/*.py : Preserve full stack traces in logs (e.g., Python logging with exc_info=True)
Applied to files:
python/src/server/api_routes/mcp_api.py
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to python/src/{server,mcp,agents}/**/*.py : Use specific exception types; avoid catching broad Exception unless re-raising with context
Applied to files:
python/src/server/api_routes/mcp_api.py
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to python/src/{server,mcp,agents}/**/*.py : Error messages must include context (operation being attempted) and relevant IDs/URLs/data for debugging
Applied to files:
python/src/server/api_routes/mcp_api.py
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to python/src/{server,mcp,agents}/**/*.py : External API calls should use retry with exponential backoff and ultimately fail with a clear, contextual error message
Applied to files:
python/src/server/api_routes/mcp_api.py
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL
Applied to files:
check-env.jsREADME.mdMakefile
🪛 YAMLlint (1.37.1)
docker-compose.yml
[error] 3-3: trailing spaces
(trailing-spaces)
🪛 markdownlint-cli2 (0.17.2)
README.md
49-49: Link fragments should be valid
(MD051, link-fragments)
102-102: Bare URL used
(MD034, no-bare-urls)
434-434: Link fragments should be valid
(MD051, link-fragments)
🪛 LanguageTool
README.md
[grammar] ~85-~85: There might be a mistake here.
Context: ...starts all core microservices in Docker: - Server: Core API and business logic (P...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...Core API and business logic (Port: 8181) - MCP Server: Protocol interface for AI ...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...ol interface for AI clients (Port: 8051) - Agents (coming soon!): AI operations a...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...AI operations and streaming (Port: 8052) - UI: Web interface (Port: 3737) Por...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...ant updates. 5. Configure API Keys: - Open http://localhost:3737 - Go to **...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ... website ### 🚀 Quick Command Reference | Command | Description ...
(QB_NEW_EN)
[grammar] ~146-~146: There might be a mistake here.
Context: ...ing.
(QB_NEW_EN)
[grammar] ~338-~338: There might be a mistake here.
Context: ...hostname and MCP port ## 🔧 Development ### Quick Start ```bash # Install dependenci...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...velopment with instant frontend updates: - Backend services run in Docker (isolated...
(QB_NEW_EN)
[grammar] ~400-~400: There might be a mistake here.
Context: ...you make changes. ## 🔍 Troubleshooting ### Common Issues and Solutions #### Port Co...
(QB_NEW_EN)
[grammar] ~471-~471: There might be a mistake here.
Context: ...ning in hybrid mode (make dev-hybrid) for best HMR experience - Backend: Chec...
(QB_NEW_EN)
[grammar] ~472-~472: There might be a mistake here.
Context: ...k that volumes are mounted correctly in docker-compose.yml - File permissions: On some systems, mou...
(QB_NEW_EN)
[grammar] ~486-~486: There might be a mistake here.
Context: ...ols installation - Environment variables - Port availability - Docker daemon status...
(QB_NEW_EN)
[grammar] ~487-~487: There might be a mistake here.
Context: ...nvironment variables - Port availability - Docker daemon status ## 📈 Progress <p...
(QB_NEW_EN)
🪛 checkmake (0.2.2)
Makefile
[warning] 7-7: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 35-35: Target body for "dev" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 72-72: Target body for "clean" exceeds allowed length of 5 (9).
(maxbodylength)
[warning] 5-5: Missing required phony target "all"
(minphony)
⏰ 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: Backend Tests (Python + pytest)
🔇 Additional comments (1)
README.md (1)
49-50: Fix internal links to “Installing Make” after heading changeIf you adopt the heading normalization, these references remain correct (
#installing-make). Verify once.Also applies to: 434-434
| SHELL := /bin/bash | ||
| .SHELLFLAGS := -ec | ||
|
|
||
| .PHONY: help dev dev-docker stop test lint clean install check |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expose doc-referenced targets: add prod and doctor aliases
README references make prod and make doctor, but only dev-docker and check exist. Add aliases to avoid confusion.
-.PHONY: help dev dev-docker stop test lint clean install check
+.PHONY: help dev dev-docker stop test lint clean install check prod doctor all+.DEFAULT_GOAL := help
+all: help
+
+# Aliases for docs
+prod: dev-docker
+doctor: check🧰 Tools
🪛 checkmake (0.2.2)
[warning] 5-5: Missing required phony target "all"
(minphony)
🤖 Prompt for AI Agents
In Makefile around line 5, the .PHONY list and targets are missing aliases
referenced in the README (make prod and make doctor); add prod and doctor to the
.PHONY line and create two simple alias targets where prod depends on the
existing dev-docker target and doctor depends on the existing check target so
invoking make prod runs dev-docker and make doctor runs check.
- Split test command into test-fe and test-be for targeted testing - Split lint command into lint-fe and lint-be for targeted linting - Keep original test and lint commands that run both - Update help text with new commands for better developer experience
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
49-49: Fix broken anchor: remove emoji from “Installing Make” heading (MD051)The link Installing Make may not resolve due to the emoji in the target heading. Normalize the heading to ensure a stable fragment.
Apply:
-## 🛠️ Installing Make +## Installing MakeRepeat if there are other links pointing to this heading (also flagged at Line 426).
🧹 Nitpick comments (9)
Makefile (7)
5-7: Add common aliases and expose “all”, “prod”, and “doctor”Docs/PR summary mention “doctor” and some users expect “prod”; also some linters want an “all” target. Provide lightweight aliases without changing behavior.
Apply:
-.PHONY: help dev dev-docker stop test test-fe test-be lint lint-fe lint-be clean install check +.PHONY: help dev dev-docker stop test test-fe test-be lint lint-fe lint-be clean install check all prod doctor + +# Aliases +all: help +prod: dev-docker +doctor: checkPlease confirm whether the README or external docs still reference “make prod”/“make doctor”; if so, these aliases keep everything consistent.
1-4: Parameterize ports and Compose binary for portabilityHardcoding ports and docker-compose reduces flexibility. Define variables up-top so
ARCHON_UI_PORT=4000 make devprints correct URLs and let users with the v2 plugin usedocker compose.Apply:
# Archon Makefile - Simple, Secure, Cross-Platform SHELL := /bin/bash .SHELLFLAGS := -ec +# Ports (overridable via env) +ARCHON_UI_PORT ?= 3737 +ARCHON_SERVER_PORT ?= 8181 +ARCHON_MCP_PORT ?= 8051 +ARCHON_AGENTS_PORT ?= 8052 + +# Compose CLI (set to 'docker-compose' if you prefer the legacy standalone) +DOCKER_COMPOSE ?= docker compose
30-37: Strengthen environment checks and support both Compose CLIsCurrently this only checks Docker and the legacy docker-compose. Add Node/uv checks (hybrid/local flows need them) and detect the v2 plugin.
Apply:
check: @echo "Checking environment..." - @node check-env.js - @echo "Checking Docker..." - @docker --version > /dev/null 2>&1 && echo "✓ Docker installed" || echo "✗ Docker not found" - @docker-compose --version > /dev/null 2>&1 && echo "✓ Docker Compose installed" || echo "✗ Docker Compose not found" + # Node is required for check-env.js and frontend dev + @if command -v node >/dev/null 2>&1; then echo "✓ Node.js: $$(node --version)"; else echo "✗ Node.js not found (install Node 18+)"; exit 1; fi + @node check-env.js + @echo "Checking Docker..." + @if command -v docker >/dev/null 2>&1; then echo "✓ Docker: $$(docker --version)"; else echo "✗ Docker not found"; fi + @if command -v docker-compose >/dev/null 2>&1; then \ + echo "✓ Docker Compose (standalone): $$(docker-compose version | head -1)"; \ + elif docker compose version >/dev/null 2>&1; then \ + echo "✓ Docker Compose (plugin): $$(docker compose version | head -1)"; \ + else \ + echo "✗ Docker Compose not found"; \ + fi + @if command -v uv >/dev/null 2>&1; then echo "✓ uv: $$(uv --version | awk '{print $$2}')"; else echo "ℹ️ uv not found (needed for local backend dev/tests)"; fi
38-46: Echo dynamic URLs from env for better DXUse parametrized ports and HOST fallback so the printed endpoints match customized runs.
Apply:
dev: check @echo "Starting hybrid development..." @echo "Backend: Docker | Frontend: Local with hot reload" - @docker-compose --profile backend up -d --build - @echo "Backend running at http://localhost:8181" + @$(DOCKER_COMPOSE) --profile backend up -d --build + @echo "Backend running at http://$${HOST:-localhost}:$(ARCHON_SERVER_PORT)" @echo "Starting frontend..." @cd archon-ui-main && npm run dev
47-54: Prefer $(DOCKER_COMPOSE) and parameterized portsAligns with earlier portability suggestions and avoids stale hardcoded ports.
Apply:
dev-docker: check @echo "Starting full Docker environment..." - @docker-compose --profile full up -d --build + @$(DOCKER_COMPOSE) --profile full up -d --build @echo "✓ All services running" - @echo "Frontend: http://localhost:3737" - @echo "API: http://localhost:8181" + @echo "Frontend: http://$${HOST:-localhost}:$(ARCHON_UI_PORT)" + @echo "API: http://$${HOST:-localhost}:$(ARCHON_SERVER_PORT)"
56-60: Use $(DOCKER_COMPOSE) and keep profiles explicitMinor consistency improvement.
Apply:
stop: @echo "Stopping all services..." - @docker-compose --profile backend --profile frontend --profile full down + @$(DOCKER_COMPOSE) --profile backend --profile frontend --profile full down @echo "✓ Services stopped"
88-97: Consistent Compose usage in cleanMirror earlier change and keep behavior unchanged.
Apply:
if [[ $$REPLY =~ ^[Yy]$$ ]]; then \ - docker-compose down -v --remove-orphans; \ + $(DOCKER_COMPOSE) down -v --remove-orphans; \ echo "✓ Cleaned"; \ else \ echo "Cancelled"; \ fiREADME.md (2)
326-326: Optional: prefer plugin syntax in docs or note both formsModern Compose is
docker compose(plugin); many users still havedocker-compose. Consider noting both to reduce friction.Apply:
-1. Restart Docker containers: `docker-compose down && docker-compose --profile full up -d` +1. Restart Docker containers: `docker compose down && docker compose --profile full up -d` (or use `docker-compose` if you’re on the standalone CLI)
403-410: Add Windows alternative for port diagnosticsImprove UX for Windows contributors who may not have lsof.
Apply:
-# Check what's using a port (e.g., 3737) -lsof -i :3737 +# Check what's using a port (e.g., 3737) +# macOS/Linux: +lsof -i :3737 +# Windows (PowerShell): +netstat -ano | Select-String ":3737"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Makefile(1 hunks)README.md(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL
Applied to files:
README.mdMakefile
🪛 markdownlint-cli2 (0.17.2)
README.md
49-49: Link fragments should be valid
(MD051, link-fragments)
426-426: Link fragments should be valid
(MD051, link-fragments)
🪛 LanguageTool
README.md
[grammar] ~85-~85: There might be a mistake here.
Context: ...starts all core microservices in Docker: - Server: Core API and business logic (P...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ... website ### 🚀 Quick Command Reference | Command | Description ...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...ing.
(QB_NEW_EN)
[grammar] ~278-~278: There might be a mistake here.
Context: ... following ports: - archon-ui: 3737 - archon-server: 8181 - archon-mcp: ...
(QB_NEW_EN)
[grammar] ~279-~279: There might be a mistake here.
Context: ...hon-ui**: 3737 - archon-server: 8181 - archon-mcp: 8051 - archon-agents: ...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...on-server**: 8181 - archon-mcp: 8051 - archon-agents: 8052 - archon-docs:...
(QB_NEW_EN)
[grammar] ~281-~281: There might be a mistake here.
Context: ...on-mcp**: 8051 - archon-agents: 8052 - archon-docs: 3838 (optional) ### Chan...
(QB_NEW_EN)
[grammar] ~330-~330: There might be a mistake here.
Context: ...hostname and MCP port ## 🔧 Development ### Quick Start ```bash # Install dependenci...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...e - make dev-docker For all services in Docker environment: - All services run...
(QB_NEW_EN)
[grammar] ~394-~394: There might be a mistake here.
Context: ...you make changes. ## 🔍 Troubleshooting ### Common Issues and Solutions #### Port Co...
(QB_NEW_EN)
[grammar] ~452-~452: There might be a mistake here.
Context: ...'re running in hybrid mode (make dev) for best HMR experience - Backend: Chec...
(QB_NEW_EN)
🪛 checkmake (0.2.2)
Makefile
[warning] 7-7: Target body for "help" exceeds allowed length of 5 (14).
(maxbodylength)
[warning] 39-39: Target body for "dev" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 88-88: Target body for "clean" exceeds allowed length of 5 (9).
(maxbodylength)
[warning] 5-5: Missing required phony target "all"
(minphony)
⏰ 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: Backend Tests (Python + pytest)
🔇 Additional comments (10)
Makefile (4)
2-3: Shell and fail-fast behavior look goodUsing Bash with -ec is appropriate for predictable, fail-fast recipes.
62-73: Backend test invocation is solidUsing uv to run pytest is consistent with install: uv sync.
83-86: Ruff with --fix in CI/dev is fine; ensure it’s acceptable for contributorsAutomatic fixes can mask lint warnings; if intentional, keep as-is.
Do we want a non-destructive lint (no --fix) mode for pre-PR checks?
99-99: Nice touch with the default “help” goalGood DX improvement.
README.md (6)
75-86: Correct use of Docker Compose profiles and service listThe “Full Docker Mode” snippet correctly uses --profile full and the service/port list matches the PR changes.
98-110: Quick Command Reference aligns with MakefileCommands listed here map to targets present in the Makefile.
126-128: Use the same profile-aware command for restartsNice consistency with earlier “full” example.
274-283: Ports section looks accurate with new defaultsMatches UI 3737 and API 8181; aligns with container names used elsewhere.
432-437: Verify Vite env var name for backend portDocs mention VITE_ARCHON_SERVER_PORT. Ensure vite.config.ts actually consumes this variable (or the appropriate one) so users aren’t misled.
If it uses a different name (e.g., VITE_API_PORT or VITE_SERVER_PORT), update the README accordingly.
384-390: Log commands match service namesService names in examples are consistent with container naming.
- Prefer 'docker compose' (plugin) over 'docker-compose' (standalone) - Add better error handling in Makefile with proper exit on failures - Add Node.js check before running environment scripts - Pass environment variables correctly to frontend in hybrid mode - Update all documentation to use modern 'docker compose' syntax - Auto-detect which Docker Compose version is available
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
docker-compose.yml (4)
3-3: Trim trailing whitespace to satisfy yamllintThere’s a trailing space at the end of Line 3. Remove to keep CI/linting green.
-# - "frontend": Starts only the frontend UI service +# - "frontend": Starts only the frontend UI service
160-164: Healthcheck may fail if curl isn’t present in the UI imageNode images often lack curl. Since the UI container already has Node (for Vite), swap to a Node-based check to remove the curl dependency.
- healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:3737"] + healthcheck: + test: + [ + "CMD", + "sh", + "-c", + "node -e \"require('http').get('http://localhost:3737',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))\"" + ] interval: 30s timeout: 10s retries: 3
92-95: Optional: wait for dependencies to be healthy before startingCompose now supports health-based start ordering. Consider adding conditions so UI/MCP won’t start until API and Agents are healthy. Helps flakiness on cold starts.
archon-mcp: depends_on: - - archon-server - - archon-agents + archon-server: + condition: service_healthy + archon-agents: + condition: service_healthy frontend: - depends_on: - - archon-server + depends_on: + archon-server: + condition: service_healthyAlso applies to: 167-169
35-35: Security note: docker.sock mount grants root-equivalent controlMounting /var/run/docker.sock gives the container full control over the host Docker daemon. Acceptable for local dev, but risky elsewhere. If you later run this in shared environments, consider:
- docker-socket-proxy with least-privilege API
- Rootless Docker or user namespaces
- Restrict via profiles so it’s never enabled outside dev
Makefile (5)
8-8: Add the promised “doctor” alias to align with PR objectivesThe PR description advertises make doctor. Expose it as an alias to check for consistency with docs and contributor expectations.
-.PHONY: help dev dev-docker stop test test-fe test-be lint lint-fe lint-be clean install check +.PHONY: help dev dev-docker stop test test-fe test-be lint lint-fe lint-be clean install check doctor+# Alias: make doctor +doctor: check
36-41: Actually enforce Node ≥ 18 in the check targetCurrent check only asserts Node is present, not version. Enforce the minimum to avoid Vite/HMR surprises.
- @node -v >/dev/null 2>&1 || { echo "✗ Node.js not found (require Node 18+)."; exit 1; } + @node -v >/dev/null 2>&1 || { echo "✗ Node.js not found (requires Node 18+)."; exit 1; } + @node -e "const v=process.versions.node.split('.').map(Number); if(v[0]<18){console.error('✗ Node.js '+process.versions.node+' found, but 18+ required'); process.exit(1)}" || exit 1 @node check-env.js
62-64: Echo dynamic ports in dev-docker using .env, not hardcoded valuesRespect ARCHON_UI_PORT/ARCHON_SERVER_PORT so the printed URLs match custom setups.
- @echo "Frontend: http://localhost:3737" - @echo "API: http://localhost:8181" + @set -a; [ -f .env ] && . ./.env; set +a; \ + echo "Frontend: http://$${HOST:-localhost}:$${ARCHON_UI_PORT:-3737}"; \ + echo "API: http://$${HOST:-localhost}:$${ARCHON_SERVER_PORT:-8181}"
80-83: Guard backend tests when uv isn’t installed (dev ergonomics)test-be assumes uv is on PATH. Either document uv in prerequisites or add a friendly guard/fallback to avoid opaque failures.
test-be: @echo "Running backend tests..." - @cd python && uv run pytest + @if command -v uv >/dev/null 2>&1; then cd python && uv run pytest; else \ + echo "✗ uv not found. Install uv (https://github.com/astral-sh/uv) or run: cd python && pytest"; exit 1; fi
1-7: Minor: avoid $(shell …) at parse time for COMPOSE detectionNot critical, but parse-time evaluation can surprise users without Docker installed. If you want to defer to runtime, compute inside recipes. Fine to keep as-is given the current checks.
README.md (1)
98-110: Expose “make doctor” in the Quick Command ReferenceIf you add the doctor alias (doctor: check), include it in the table for discoverability.
| `make check` | Check environment setup | +| `make doctor` | Alias for `make check` | | `make clean` | Remove containers and volumes (with confirmation) |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
Makefile(1 hunks)README.md(7 hunks)docker-compose.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL
Applied to files:
MakefileREADME.md
🪛 checkmake (0.2.2)
Makefile
[warning] 10-10: Target body for "help" exceeds allowed length of 5 (14).
(maxbodylength)
[warning] 34-34: Target body for "check" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 45-45: Target body for "dev" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 98-98: Target body for "clean" exceeds allowed length of 5 (9).
(maxbodylength)
[warning] 8-8: Missing required phony target "all"
(minphony)
🪛 markdownlint-cli2 (0.17.2)
README.md
49-49: Link fragments should be valid
(MD051, link-fragments)
430-430: Link fragments should be valid
(MD051, link-fragments)
🪛 LanguageTool
README.md
[grammar] ~85-~85: There might be a mistake here.
Context: ...starts all core microservices in Docker: - Server: Core API and business logic (P...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ... website ### 🚀 Quick Command Reference | Command | Description ...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...ails> ## 🛠️ Installing Make (OPTIONAL) Make is required for the local developmen...
(QB_NEW_EN)
[grammar] ~278-~278: There might be a mistake here.
Context: ... following ports: - archon-ui: 3737 - archon-server: 8181 - archon-mcp: ...
(QB_NEW_EN)
[grammar] ~279-~279: There might be a mistake here.
Context: ...hon-ui**: 3737 - archon-server: 8181 - archon-mcp: 8051 - archon-agents: ...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...on-server**: 8181 - archon-mcp: 8051 - archon-agents: 8052 - archon-docs:...
(QB_NEW_EN)
[grammar] ~281-~281: There might be a mistake here.
Context: ...on-mcp**: 8051 - archon-agents: 8052 - archon-docs: 3838 (optional) ### Chan...
(QB_NEW_EN)
[grammar] ~330-~330: There might be a mistake here.
Context: ...hostname and MCP port ## 🔧 Development ### Quick Start ```bash # Install dependenci...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...e - make dev-docker For all services in Docker environment: - All services run...
(QB_NEW_EN)
[grammar] ~398-~398: There might be a mistake here.
Context: ...you make changes. ## 🔍 Troubleshooting ### Common Issues and Solutions #### Port Co...
(QB_NEW_EN)
[grammar] ~456-~456: There might be a mistake here.
Context: ...'re running in hybrid mode (make dev) for best HMR experience - Backend: Chec...
(QB_NEW_EN)
🪛 YAMLlint (1.37.1)
docker-compose.yml
[error] 3-3: trailing spaces
(trailing-spaces)
⏰ 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: Backend Tests (Python + pytest)
🔇 Additional comments (7)
docker-compose.yml (4)
12-12: Profiles look correct and map cleanly to the three dev modesNice use of Compose profiles to model hybrid/frontend/full flows. This aligns with the PR objective and keeps local workflows simple.
Also applies to: 67-67, 112-112, 147-147
19-19: Consistent, lowercase container names improve service discoveryStandardizing to archon-server/archon-mcp/archon-agents/archon-ui will prevent resolution bugs across code and docs. Good change.
Also applies to: 74-75, 119-120, 149-150
151-152: Port fix: 3737->3737 for the UI is correctMapping ${ARCHON_UI_PORT:-3737}:3737 matches the UI’s internal port and the updated Vite config.
153-156: Frontend API URL wiring matches browser-origin expectationsUsing HOST+ARCHON_SERVER_PORT for VITE_API_URL is correct, since the browser needs to call localhost:8181 in Docker mode. Good alignment with vite config changes.
README.md (3)
77-91: Deployment commands are accurate and match docker-compose profilesGood clarification of the full profile and the make dev-docker alternative. The port list matches the compose file.
326-326: Nice: restart example pins the full profileExplicitly including --profile full prevents confusion when nothing starts. Good call.
456-459: HMR troubleshooting note aligns with the new hybrid workflowReferencing make dev here is correct and consistent.
| - (OPTIONAL) [Make](https://www.gnu.org/software/make/) (see [Installing Make](#installing-make) below) | ||
|
|
There was a problem hiding this comment.
Fix broken “Installing Make” anchor and clarify optionality
The link points to #installing-make, but the heading slug differs due to emoji/(OPTIONAL). Also, the section calls Make “required” while the header says “(OPTIONAL)”. Normalize both.
- - (OPTIONAL) [Make](https://www.gnu.org/software/make/) (see [Installing Make](#installing-make) below)
+ - (OPTIONAL) [Make](https://www.gnu.org/software/make/) (see [Installing Make](#installing-make) below)
@@
-## 🛠️ Installing Make (OPTIONAL)
-
-Make is required for the local development workflow. Installation varies by platform:
+## Installing Make
+
+Make is recommended. It’s required only if you use the Make-driven workflow (e.g., make dev, make dev-docker). Full Docker mode works without it. Installation varies by platform:Also applies to: 138-141
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
49-49: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
In README.md around lines 49-50 and 138-141, the "Installing Make" anchor is
broken because the heading slug includes "(OPTIONAL)"/emoji and the body text
contradicts the heading by calling Make required; update either the heading or
the link so the anchor matches (e.g., remove "(OPTIONAL)"/emoji from the heading
or change the link to the actual slug), and make the copy consistent by choosing
one stance (mark Make as optional everywhere or mark it as required everywhere)
and adjust the heading text and any inline references accordingly so links and
wording are normalized.
- Add Node.js 18+ as prerequisite for hybrid development - Mark Make as optional throughout the documentation - Update all docker-compose commands to modern 'docker compose' syntax - Add Make command alternatives for testing (make test, test-fe, test-be) - Document make dev for hybrid development mode - Remove linting requirements until codebase errors are resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
CONTRIBUTING.md (9)
56-61: Recommend updating Node requirement to current LTS and add “make doctor” hint.Node 18.x is EOL; recommend 20+ LTS. Also, call out the new “make doctor” to preempt common setup issues. Minor phrasing tweak on the Make bullet.
Using retrieved learnings: contributors often miss required env keys. Consider explicitly listing SUPABASE_URL and SUPABASE_SERVICE_KEY in the setup section.
- - [Node.js 18+](https://nodejs.org/) (for hybrid development mode) + - [Node.js 20+ (LTS)](https://nodejs.org/) (recommended for hybrid development mode; 18.x is EOL) - - (Optional) [Make](https://www.gnu.org/software/make/) for simplified workflows + - [Make](https://www.gnu.org/software/make/) (optional, recommended) for simplified workflows + - Run `make doctor` (if Make is installed) to validate Docker, Node, and required env varsIf you want, I can also add a short “Required environment variables” list under “Initial Setup → Environment Configuration”:
- SUPABASE_URL, SUPABASE_SERVICE_KEY (required), OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL (optional).
79-84: Surface “make doctor” and port customization alongside compose/make spins.Good switch to Docker Compose profiles and make dev-docker. Suggest inserting a quick env validation step and a one-liner showing how to override the UI port (matches the PR behavior).
```bash - # Using Docker Compose directly + # Validate your environment (optional but recommended) + make doctor + + # Using Docker Compose directly docker compose --profile full up --build -d - + - # Or using Make (if installed) - make dev-docker + # Or using Make (if installed) + make dev-docker + + # Customize the UI port (defaults to 3737) + ARCHON_UI_PORT=4000 make dev-docker--- `220-228`: **Add one-time local dependency installs before “manual” test commands.** Manual test commands will fail outside Docker without deps installed. Add a brief one-time setup snippet. ```diff ```bash # Using Make (if installed) make test # Run all tests make test-fe # Frontend tests only make test-be # Backend tests only - - # Or manually + + # Or manually (outside Docker) + # One-time setup + python -m venv .venv && source .venv/bin/activate + pip install -r python/requirements-dev.txt + cd archon-ui-main && npm ci && cd .. + + # Then run: cd python && python -m pytest # Backend tests cd archon-ui-main && npm run test # Frontend tests--- `230-232`: **Add teardown guidance for the “full” profile run.** A quick down -v avoids orphaned containers/volumes and surprises. ```diff # Full integration test docker compose --profile full up --build -d # Test via UI at http://localhost:3737 + # Tear down when done + docker compose --profile full down -v
336-345: Small clarity boost on pytest usage.Offer markers example for faster iteration; harmless and helpful.
# Or manually cd python && python -m pytest tests/ # Run specific test categories - python -m pytest -m unit # Unit tests only - python -m pytest -m integration # Integration tests only + python -m pytest -m unit # Unit tests only + python -m pytest -m integration # Integration tests only + # Tip: narrow to a file or test: python -m pytest tests/path/test_file.py::TestClass::test_case -q
351-351: Tighten phrasing for this guideline.- # Maintain consistency with the codebase + # Maintain consistency with existing patterns in this codebase
368-371: Front-end tests: call out CI/coverage variants near the basic command.You show coverage/UI modes later; adding the common “ci” alias here helps contributors scanning this section.
# Using Make (if installed) make test-fe - + # Or CI-style with watch disabled + npm run test:ci
383-389: Document all three dev modes and clarify port behavior for local frontend.PR introduces three modes; surface dev-local here and state how ARCHON_UI_PORT affects both Make and manual runs.
```bash # Using Make for hybrid mode (if installed) make dev # Backend in Docker, frontend local - + # All services in Docker + make dev-docker + # Local API + UI; MCP/Agents in Docker + make dev-local + + # Customize the frontend port (defaults to 3737) + ARCHON_UI_PORT=4000 make dev + # Or manually for faster iteration cd archon-ui-main && npm run dev # Still connects to Docker backend services + # Available at http://localhost:$ARCHON_UI_PORT (defaults to 3737)--- `56-59`: **Minor list punctuation/flow fix flagged by grammar tooling.** LanguageTool flagged this list; the revision above (Node LTS + Make bullet tweak) should also resolve the style warning by improving parallel structure. Nothing blocking here. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 687e78b5636220e080a9481765ecb045629d68a8 and 199d41899fb71ef15ced5bb469acde8d4466a16a. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `CONTRIBUTING.md` (7 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (2)</summary> <details> <summary>📚 Learning: 2025-08-21T11:22:33.541Z</summary>Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in the .env file; optionally OPENAI_API_KEY, LOGFIRE_TOKEN, LOG_LEVEL**Applied to files:** - `CONTRIBUTING.md` </details> <details> <summary>📚 Learning: 2025-08-21T11:22:33.541Z</summary>Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to archon-ui-main/test/** : Place frontend tests under archon-ui-main/test/**Applied to files:** - `CONTRIBUTING.md` </details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>CONTRIBUTING.md</summary> [grammar] ~56-~56: There might be a mistake here. Context: ...dejs.org/) (for hybrid development mode) - [Supabase](https://supabase.com/) account... (QB_NEW_EN) --- [grammar] ~57-~57: There might be a mistake here. Context: ...supabase.com/) account (free tier works) - [OpenAI API key](https://platform.openai.... (QB_NEW_EN) --- [grammar] ~58-~58: There might be a mistake here. Context: ...om/api-keys) or alternative LLM provider - (Optional) [Make](https://www.gnu.org/so... (QB_NEW_EN) --- [grammar] ~59-~59: There might be a mistake here. Context: ...software/make/) for simplified workflows - Basic knowledge of Python (FastAPI) and ... (QB_NEW_EN) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: Backend Tests (Python + pytest) </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Aligns frontend service naming with other services (archon-server, archon-mcp, archon-agents) for better consistency in Docker image naming patterns.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
docker-compose.yml (7)
1-8: Profiles header is clear; fix trailing space and consider adding an "agents" profile to match Local mode.
- Remove the trailing spaces on Line 3 flagged by yamllint.
- Given the PR claims a Local mode (API + UI local; MCP/Agents in Docker), add a dedicated "agents" profile to document and enable MCP + Agents only.
Apply this diff:
# Docker Compose Profiles Strategy: # - "backend": Starts only backend services (server, mcp, agents) for hybrid development -# - "frontend": Starts only the frontend UI service +# - "frontend": Starts only the frontend UI service # - "full": Starts all services for complete Docker deployment +# - "agents": Starts only MCP and Agents (for Local mode: API + UI local) # Use --profile flag to control which services start: # docker compose --profile backend up # Backend only for hybrid dev (or docker-compose) # docker compose --profile full up # Everything in Docker (or docker-compose) +# docker compose --profile agents up # MCP + Agents only
35-37: Mapping the Docker socket grants root-equivalent power; gate behind a profile or proxy for safety.Even in dev, binding /var/run/docker.sock is high-privilege. Consider:
- Using a docker-socket-proxy container, or
- Adding a dedicated profile (e.g., docker-sock) so teams can opt-out safely.
Example (illustrative; pick naming that fits your Make targets):
- - /var/run/docker.sock:/var/run/docker.sock # Docker socket for MCP container control + - /var/run/docker.sock:/var/run/docker.sock # Docker socket (dev only; consider gating via a 'docker-sock' profile)
36-37: Nit: comment says “UI test execution” but this mounts python tests into the server container.Clarify to avoid confusion during onboarding.
- - ./python/tests:/app/tests # Mount tests for UI test execution + - ./python/tests:/app/tests # Mount server tests for dev execution
112-121: Mirror the new "agents" profile on archon-agents.Keeps profiles coherent and enables MCP/Agents-only brings up cleanly.
archon-agents: - profiles: ["backend", "full"] + profiles: ["backend", "agents", "full"]
146-169: Frontend depends_on archon-server may confuse profile-only runs; consider removing it.When running with --profile frontend, Compose ignores dependencies that are not enabled by the same profile, leading to noisy warnings. Dropping depends_on here keeps the frontend profile decoupled for hybrid dev (API local).
archon-frontend: @@ - depends_on: - - archon-server + # No depends_on: allows running with --profile frontend while API runs locally
67-105: Add “agents” profile and enable health-based startup orderingThe
archon-mcpservice indocker-compose.ymlstill only has profiles["backend", "full"]and uses a simple list underdepends_on. To support an “agents-only” mode and ensure MCP waits for healthy dependencies, please update as follows:• In
docker-compose.yml, locate thearchon-mcpservice block (around lines 66–105)
– Add"agents"to theprofilesarray
– Changedepends_onto usecondition: service_healthyfor botharchon-serverandarchon-agents• (Optional) Mirror the same
"agents"profile on thearchon-agentsservice if you plan to start it in isolationProposed diff for
archon-mcp:archon-mcp: - profiles: ["backend", "full"] + profiles: ["backend", "agents", "full"] @@ - depends_on: - - archon-server - - archon-agents + depends_on: + archon-server: + condition: service_healthy + archon-agents: + condition: service_healthy
153-157: Simplify environment configuration by removing VITE_API_URLIn
docker-compose.yml(lines 153–157), we currently set both:
VITE_API_URL=http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181}VITE_ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181}However,
getApiUrl()insrc/config/api.tsalways usesVITE_API_URLwhen present, and only falls back to constructing the URL fromVITE_ARCHON_SERVER_PORTin development. This dual‐source approach can lead to drift if the two ever diverge. We can safely removeVITE_API_URLand rely solely onVITE_ARCHON_SERVER_PORT:- - VITE_API_URL=http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181} - - VITE_ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181} + - VITE_ARCHON_SERVER_PORT=${ARCHON_SERVER_PORT:-8181}• Verified that
getApiUrl()returnsimport.meta.env.VITE_API_URL ? import.meta.env.VITE_API_URL : `${window.location.protocol}//${window.location.hostname}:${import.meta.env.VITE_ARCHON_SERVER_PORT||'8181'}`when
VITE_API_URLis unset.
• Confirmed withrg -nP 'EXPOSE\s+3737' archon-ui-main/Dockerfilethat port 3737 is exposed.This change removes configuration duplication without impacting runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docker-compose.yml(4 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
docker-compose.yml
[error] 3-3: trailing spaces
(trailing-spaces)
⏰ 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). (2)
- GitHub Check: Docker Build Tests (server)
- GitHub Check: Backend Tests (Python + pytest)
🔇 Additional comments (3)
docker-compose.yml (3)
12-21: Profiles and container_name on archon-server look good.The profiles mapping and standardized container_name should help service discovery and dev UX.
146-152: Port mapping change to 3737 is consistent and matches the stated intent.Good standardization: host ${ARCHON_UI_PORT:-3737} → container 3737 lines up with the Vite/Dockerfile changes.
160-160: Healthcheck tool available—no changes neededThe
archon-ui-main/Dockerfilealready installs curl:
- archon-ui-main/Dockerfile (line 7):
RUN apk add --no-cache python3 make g++ git curlSince curl is present in the image, the existing healthcheck command is valid and does not require switching to wget or adding curl.
|
Error introduced in this PR (validated with Running |
* Add Supabase key validation and simplify frontend state management
- Add backend validation to detect and warn about anon vs service keys
- Prevent startup with incorrect Supabase key configuration
- Consolidate frontend state management following KISS principles
- Remove duplicate state tracking and sessionStorage polling
- Add clear error display when backend fails to start
- Improve .env.example documentation with detailed key selection guide
- Add comprehensive test coverage for validation logic
- Remove unused test results checking to eliminate 404 errors
The implementation now warns users about key misconfiguration while
maintaining backward compatibility. Frontend state is simplified with
MainLayout as the single source of truth for backend status.
* Fix critical issues from code review
- Use python-jose (already in dependencies) instead of PyJWT for JWT decoding
- Make unknown Supabase key roles fail fast per alpha principles
- Skip all JWT validations (not just signature) when checking role
- Update tests to expect failure for unknown roles
Fixes:
- No need to add PyJWT dependency - python-jose provides JWT functionality
- Unknown key types now raise ConfigurationError instead of warning
- JWT decode properly skips all validations to only check role claim
* Remove unnecessary startup delay script from frontend Dockerfile
- Rolled back to match main branch Dockerfile
- Removed 3-second sleep script that was added for backend readiness
- Container now runs npm directly without intermediate script
- Tested and verified all services start correctly without the delay
* Fix document deletion persistence issue (#278)
- Fixed projectService methods to include project_id parameter in API calls
- Updated deleteDocument() to use correct endpoint: /api/projects/{projectId}/docs/{docId}
- Updated getDocument() and updateDocument() to use correct endpoints with project_id
- Modified DocsTab component to call backend API when deleting documents
- Documents now properly persist deletion after page refresh
The issue was that document deletion was only happening in UI state and never
reached the backend. The service methods were using incorrect API endpoints
that didn't include the required project_id parameter.
* Add comprehensive test coverage for document CRUD operations
- Add Document interface for type safety
- Fix error messages to include projectId context
- Add unit tests for all projectService document methods
- Add integration tests for DocsTab deletion flow
- Update vitest config to include new test files
* MCP server consolidation and simplification
- Consolidated multiple MCP modules into unified project_module
- Removed redundant project, task, document, and version modules
- Identified critical issue with async project creation losing context
- Updated CLAUDE.md with project instructions
This commit captures the current state before refactoring to split
consolidated tools into separate operations for better clarity and
to solve the async project creation context issue.
* Improve MCP tool usability and documentation
- Fix parameter naming confusion in RAG tools (source → source_domain)
- Add clarification that source_domain expects domain names not IDs
- Improve manage_versions documentation with clear examples
- Add better error messages for validation failures
- Enhance manage_document with non-PRP examples
- Add comprehensive documentation to get_project_features
- Fix content parameter type in manage_versions to accept Any type
These changes address usability issues discovered during testing without
breaking existing functionality.
* Refactor MCP server structure and add separate project tools
- Rename src/mcp to src/mcp_server for clarity
- Update all internal imports to use new path
- Create features/projects directory for modular tool organization
- Add separate, simple project tools (create, list, get, delete, update)
- Keep consolidated tools for backward compatibility (via env var)
- Add USE_SEPARATE_PROJECT_TOOLS env var to toggle between approaches
The new separate tools:
- Solve the async project creation context loss issue
- Provide clearer, single-purpose interfaces
- Remove complex PRP examples for simplicity
- Handle project creation polling automatically
* Update bug_report.yml
Changing Archon Alpha to Beta in the issue template
* Update README.md
Added note in the README
* Fix project cards horizontal scrollbar visibility (#295)
Addresses issue #293 by replacing hide-scrollbar with scrollbar-thin
class to ensure users can see and interact with the horizontal scrollbar
when project cards overflow.
* Fix missing feature field in project tasks API response
Resolves issue #282 by adding feature field to task dictionary in
TaskService.list_tasks() method. The project tasks API endpoint was
excluding the feature field while individual task API included it,
causing frontend to default to 'General' instead of showing custom
feature values.
Changes:
- Add feature field to task response in list_tasks method
- Maintains compatibility with existing API consumers
- All 212 tests pass with this change
* Remove consolidated project module in favor of separated tools
The consolidated project module contained all project, task, document,
version, and feature management in a single 922-line file. This has been
replaced with focused, single-purpose tools in separate modules.
* Remove feature flags from Docker configuration
Removed USE_SEPARATE_PROJECT_AND_TASK_TOOLS and PROJECTS_ENABLED
environment variables as the separated tools are now the default.
* Add document and version management tools
Extract document management functionality into focused tools:
- create_document: Create new documents with metadata
- list_documents: List all documents in a project
- get_document: Retrieve specific document details
- update_document: Modify existing documents
- delete_document: Remove documents from projects
Extract version control functionality:
- create_version: Create immutable snapshots
- list_versions: View version history
- get_version: Retrieve specific version content
- restore_version: Rollback to previous versions
Includes improved documentation and error messages based on testing.
* Add task management tools with smart routing
Extract task functionality into focused tools:
- create_task: Create tasks with sources and code examples
- list_tasks: List tasks with project/status filtering
- get_task: Retrieve task details
- update_task: Modify task properties
- delete_task: Archive tasks (soft delete)
Preserves intelligent endpoint routing:
- Project-specific: /api/projects/{id}/tasks
- Status filtering: /api/tasks?status=X
- Assignee filtering: /api/tasks?assignee=X
* Add feature management tool for project capabilities
Extract get_project_features as a standalone tool with enhanced
documentation explaining feature structures and usage patterns.
Features track functional components like auth, api, and database.
* Update project tools to use simplified approach
Remove complex PRP validation logic and focus on core functionality.
Maintains backward compatibility with existing API endpoints.
* Register all separated tools in MCP server
Update MCP server to use the new modular tool structure:
- Projects and tasks from existing modules
- Documents and versions from new modules
- Feature management from standalone module
Remove all feature flag logic as separated tools are now default.
* Update MCP Dockerfile to support new module structure
Create documents directory and ensure all new modules are properly
included in the container build.
* Clean up unused imports in RAG module
Remove import of deleted project_module.
* Fix type errors and remove trailing whitespace
- Add explicit type annotations for params dictionaries to resolve mypy errors
- Remove trailing whitespace from blank lines (W293 ruff warnings)
- Ensure type safety in task_tools.py and document_tools.py
* Add comprehensive unit tests for MCP server features
- Create test structure mirroring features folder organization
- Add tests for document tools (create, list, update, delete)
- Add tests for version tools (create, list, restore, invalid field handling)
- Add tests for task tools (create with sources, list with filters, update, delete)
- Add tests for project tools (create with polling, list, get)
- Add tests for feature tools (get features with various structures)
- Mock HTTP client for all external API calls
- Test both success and error scenarios
- 100% test coverage for critical tool functions
* Updating the Logo for Archon
* Add Stage 1 workflow for external PR info collection
- Collects PR information without requiring secrets
- Triggers on pull_request events and @claude-review-ext comments
- Uploads PR details as artifact for secure processing
* Add Stage 2 secure review workflow for external PRs
- Runs after Stage 1 via workflow_run trigger
- Has access to repository secrets
- Downloads PR artifact and performs review
- Maintains security by never checking out fork code
* Add documentation for external PR review workflows
- Explains the two-stage security model
- Provides usage instructions for contributors and maintainers
- Includes troubleshooting and security considerations
* Fix base branch checkout in Stage 2 workflow
- Extract PR base branch from artifact instead of using workflow branch
- Add step to switch to correct base branch after downloading PR info
- Use PR base branch for diff generation instead of workflow branch
* Fix external PR workflow permissions and error handling
- Grant pull-requests write permission for comment posting
- Add try-catch error handling with continue-on-error
- Ensure workflow continues even if comment posting fails
* Simplify authorization for external PR workflows
- Move authorization check to Stage 1 job condition
- Remove complex authorization job from Stage 2
- Fix duplicate exec declaration error
- Add unauthorized user message handling in Stage 1
- Trust Stage 1's authorization in Stage 2
* Fix exec declaration error in Stage 2 workflow
- Replace async exec with execSync to avoid declaration issues
- Add proper error handling for artifact extraction
- Use childProcess module directly instead of destructuring
* Fix Claude Code Action authentication and context issues
- Remove invalid pr_number parameter
- Add explicit github_token to fix OIDC failure in workflow_run
- Add mode: review for proper review mode
- Create fake event.json to provide PR context
- Set environment variables to simulate PR event
* Remove invalid mode parameter and improve event context
- Remove invalid mode: review parameter
- Update event context to simulate issue_comment
- Add direct_prompt to guide Claude to review the diff
- Update instructions to use Read tool for pr-diff.patch
* Simplify fork PR review to single workflow with pull_request_target
- Remove complex two-stage workflow approach
- Use pull_request_target with security safeguards
- Add first-time contributor check and approval requirement
- Never checkout PR code - only analyze diff
- Mirror full review format from main claude-review workflow
- Manual trigger via @claude-review-fork for maintainers
* Fix Claude checkout issue for forked PRs
- Add environment overrides to prevent PR branch checkout
- Add explicit github_token for authentication
- Add direct_prompt to guide Claude to use diff file
- Override GITHUB_REF and GITHUB_SHA to stay on base branch
* Override event context to prevent PR checkout
- Set GITHUB_EVENT_NAME to workflow_dispatch to avoid PR detection
- Use override_prompt instead of direct_prompt for better control
- Create wrapper script for debugging
- Explicitly tell Claude not to checkout code
* fix: Restructure fork review workflow to avoid PR branch checkout
- Create isolated review context directory to prevent PR detection
- Move diff to changes.patch file in review-context directory
- Add explicit REVIEW_INSTRUCTIONS.md file for guidance
- Use standard 'prompt' parameter instead of 'override_prompt'
- This approach should prevent Claude Action from auto-detecting PR context
* Remove claude-review-fork.yml workflow
The Claude Code Action is not compatible with reviewing PRs from forks.
It always attempts to checkout the PR branch which fails for security reasons.
* Fix: Allow HTTP for local Supabase connections (#323)
- Modified validate_supabase_url() to allow HTTP for local development
- HTTP is now allowed for localhost, 127.0.0.1, host.docker.internal, and 0.0.0.0
- HTTPS is still required for production/non-local environments
- Fixes server startup failure when using local Supabase with Docker
* Update README.md (#332)
Makes it easier to copy paste & run the command in on single shot
* feat(mcp): Add robust error handling and timeout configuration
Critical improvements to MCP server reliability and client experience:
Error Handling:
- Created MCPErrorFormatter for consistent error responses across all tools
- Provides structured errors with type, message, details, and actionable suggestions
- Helps clients (like Claude Code) understand and handle failures gracefully
- Categorizes errors (connection_timeout, validation_error, etc.) for better debugging
Timeout Configuration:
- Centralized timeout config with environment variable support
- Different timeouts for regular operations vs polling operations
- Configurable via MCP_REQUEST_TIMEOUT, MCP_CONNECT_TIMEOUT, etc.
- Prevents indefinite hangs when services are unavailable
Module Registration:
- Distinguishes between ImportError (acceptable) and code errors (must fix)
- SyntaxError/NameError/AttributeError now halt execution immediately
- Prevents broken code from silently failing in production
Polling Safety:
- Fixed project creation polling with exponential backoff
- Handles API unavailability with proper error messages
- Maximum attempts configurable via MCP_MAX_POLLING_ATTEMPTS
Response Normalization:
- Fixed inconsistent response handling in list_tasks
- Validates and normalizes different API response formats
- Clear error messages when response format is unexpected
These changes address critical issues from PR review while maintaining
backward compatibility. All 20 existing tests pass.
* refactor(mcp): Apply consistent error handling to all MCP tools
Comprehensive update to MCP server error handling:
Error Handling Improvements:
- Applied MCPErrorFormatter to all remaining MCP tool files
- Replaced all hardcoded timeout values with configurable timeout system
- Converted all simple string errors to structured error format
- Added proper httpx exception handling with detailed context
Tools Updated:
- document_tools.py: All 5 document management tools
- version_tools.py: All 4 version management tools
- feature_tools.py: Project features tool
- project_tools.py: Remaining 3 project tools (get, list, delete)
- task_tools.py: Remaining 4 task tools (get, list, update, delete)
Test Improvements:
- Removed backward compatibility checks from all tests
- Tests now enforce structured error format (dict not string)
- Any string error response is now considered a bug
- All 20 tests passing with new strict validation
This completes the error handling refactor for all MCP tools,
ensuring consistent client experience and better debugging.
* fix(mcp): Address all priority actions from PR review
Based on latest PR #306 review feedback:
Fixed Issues:
- Replaced last remaining basic error handling with MCPErrorFormatter
in version_tools.py get_version function
- Added proper error handling for invalid env vars in get_max_polling_attempts
- Improved type hints with TaskUpdateFields TypedDict for better validation
- All tools now consistently use get_default_timeout() (verified with grep)
Test Improvements:
- Added comprehensive tests for MCPErrorFormatter utility (10 tests)
- Added tests for timeout_config utility (13 tests)
- All 43 MCP tests passing with new utilities
- Tests verify structured error format and timeout configuration
Type Safety:
- Created TaskUpdateFields TypedDict to specify exact allowed fields
- Documents valid statuses and assignees in type comments
- Improves IDE support and catches type errors at development time
This completes all priority actions from the review:
✅ Fixed inconsistent timeout usage (was already done)
✅ Fixed error handling inconsistency
✅ Improved type hints for update_fields
✅ Added tests for utility modules
* style: Apply linting fixes and formatting
Applied automated linting and formatting:
- Fixed missing newlines at end of files
- Adjusted line wrapping for better readability
- Fixed multi-line string formatting in tests
- No functional changes, only style improvements
All 43 tests still passing after formatting changes.
* Update docker-compose.yml
Adding host.docker.internal:host-gateway to Docker Compose for the server and agents.
* Update README.md (#410)
added star history at end of readme
* Important updates to CONTRIBUTING.md and README
* fix: Allow HTTP for all private network ranges in Supabase URLs (#417)
* fix: Allow HTTP for all private network ranges in Supabase URLs
- Extend HTTP support to all RFC 1918 private IP ranges
- Class A: 10.0.0.0 to 10.255.255.255 (10.0.0.0/8)
- Class B: 172.16.0.0 to 172.31.255.255 (172.16.0.0/12)
- Class C: 192.168.0.0 to 192.168.255.255 (192.168.0.0/16)
- Also includes link-local (169.254.0.0/16) addresses
- Uses Python's ipaddress module for robust IP validation
- Maintains HTTPS requirement for public/production URLs
- Backwards compatible with existing localhost exceptions
* security: Fix URL validation vulnerabilities
- Replace substring matching with exact hostname matching to prevent bypass attacks
- Exclude unspecified address (0.0.0.0) from allowed HTTP hosts
- Add support for .localhost domains per RFC 6761
- Improve error messages with hostname context for better debugging
Addresses security concerns raised in PR review regarding:
- Malicious domains like 'localhost.attacker.com' bypassing HTTPS requirements
- Unspecified address being incorrectly allowed as valid connection target
---------
Co-authored-by: tazmon95 <tazmon95@users.noreply.github.com>
Co-authored-by: root <root@supatest2.jtpa.net>
* Fix business document categorization bug
- Fixed missing knowledge_type and tags parameters in DocumentStorageService.upload_document()
- Added source_type='file' to document chunk metadata for proper categorization
- Enhanced source metadata creation to include source_type based on source_id pattern
- Fixed metadata spread order in knowledge_item_service to prevent source_type override
- Business documents now correctly show pink color theme and appear in Business Documents section
Fixes issue where business documents were incorrectly stored as technical knowledge
and appeared with blue color theme instead of pink.
* feat: Add Gemini CLI support to MCPPage and IDEGlobalRules
* fix(mcp): Fix update_task signature and MCP instructions
Resolves #420 - Tasks being duplicated instead of updated
Changes:
1. Fixed update_task function signature to use individual optional parameters
- Changed from TypedDict to explicit parameters (title, status, etc.)
- Consistent with update_project and update_document patterns
- Builds update_fields dict internally from provided parameters
2. Updated MCP instructions with correct function names
- Replaced non-existent manage_task with actual functions
- Added complete function signatures for all tools
- Improved workflow documentation with concrete examples
This fixes the issue where AI agents were confused by:
- Wrong function names in instructions (manage_task vs update_task)
- Inconsistent parameter patterns across update functions
- TypedDict magic that wasn't clearly documented
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* test(mcp): Update tests for new update_task signature
- Fixed test_update_task_status to use individual parameters
- Added test_update_task_no_fields for validation testing
- All MCP tests passing (44 tests)
* style(mcp): Clean up whitespace in MCP instructions
- Remove trailing whitespace
- Consistent formatting in instruction text
* Fix crawler timeout for JavaScript-heavy documentation sites
Remove wait_for='body' selector from documentation site crawling config.
The body element exists immediately in HTML, causing unnecessary timeouts
for JavaScript-rendered content. Now relies on domcontentloaded event
and delay_before_return_html for proper JavaScript execution.
* chore: Remove unused imports and fix exception chaining
- Remove unused asyncio imports from batch.py and recursive.py
- Add proper exception chaining with 'from e' to preserve stack traces
* fix: Apply URL transformation before crawling in recursive strategy
- Transform URLs to raw content (e.g., GitHub blob -> raw) before sending to crawler
- Maintain mapping dictionary to preserve original URLs in results
- Align progress callback signatures between batch and recursive strategies
- Add safety guards for missing links attribute
- Remove unused loop counter in batch strategy
- Optimize binary file checks to avoid duplicate calls
This ensures GitHub files are crawled as raw content instead of HTML pages,
fixing the issue where content extraction was degraded due to HTML wrapping.
* Improve development environment with Docker Compose profiles (#435)
* Add improved development environment with backend in Docker and frontend locally
- Created dev.bat script to run backend services in Docker and frontend locally
- Added docker-compose.backend.yml for backend-only Docker setup
- Updated package.json to run frontend on port 3737
- Fixed api.ts to use default port 8181 instead of throwing error
- Script automatically stops production containers to avoid port conflicts
- Provides instant HMR for frontend development
* Refactor development environment setup: replace dev.bat with Makefile for cross-platform support and enhanced commands
* Enhance development environment: add environment variable checks and update test commands for frontend and backend
* Improve development environment with Docker Compose profiles
This commit enhances the development workflow by replacing the separate
docker-compose.backend.yml file with Docker Compose profiles, fixing
critical service discovery issues, and adding comprehensive developer
tooling through an improved Makefile system.
Key improvements:
- Replace docker-compose.backend.yml with cleaner profile approach
- Fix service discovery by maintaining consistent container names
- Fix port mappings (3737:3737 instead of 3737:5173)
- Add make doctor for environment validation
- Fix port configuration and frontend HMR
- Improve error handling with .SHELLFLAGS in Makefile
- Add comprehensive port configuration via environment variables
- Simplify make dev-local to only run essential services
- Add logging directory creation for local development
- Document profile strategy in docker-compose.yml
These changes provide three flexible development modes:
- Hybrid mode (default): Backend in Docker, frontend local with HMR
- Docker mode: Everything in Docker for production-like testing
- Local mode: API server and UI run locally
Co-authored-by: Zak Stam <zaksnet@users.noreply.github.com>
* Fix make stop command to properly handle Docker Compose profiles
The stop command now explicitly specifies all profiles to ensure
all containers are stopped regardless of how they were started.
* Fix README to document correct make commands
- Changed 'make lint' to 'make lint-frontend' and 'make lint-backend'
- Removed non-existent 'make logs-server' command
- Added 'make watch-mcp' and 'make watch-agents' commands
- All documented make commands now match what's available in Makefile
* fix: Address critical issues from code review #435
- Create robust environment validation script (check-env.js) that properly parses .env files
- Fix Docker healthcheck port mismatch (5173 -> 3737)
- Remove hard-coded port flags from package.json to allow environment configuration
- Fix Docker detection logic using /.dockerenv instead of HOSTNAME
- Normalize container names to lowercase (archon-server, archon-mcp, etc.)
- Improve stop-local command with port-based fallback for process killing
- Fix API configuration fallback chain to include VITE_PORT
- Fix Makefile shell variable expansion using runtime evaluation
- Update .PHONY targets with comprehensive list
- Add --profile flags to Docker Compose commands in README
- Add VITE_ARCHON_SERVER_PORT to docker-compose.yml
- Add Node.js 18+ to prerequisites
- Use dynamic ports in Makefile help messages
- Add lint alias combining frontend and backend linting
- Update .env.example documentation
- Scope .gitignore logs entry to /logs/
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix container name resolution for MCP server
- Add dynamic container name resolution with three-tier strategy
- Support environment variables for custom container names
- Add service discovery labels to docker-compose services
- Update BackendStartupError with correct container name references
* Fix frontend test failures in API configuration tests
- Update environment variable names to use VITE_ prefix that matches production code
- Fix MCP client service tests to use singleton instance export
- Update default behavior tests to expect fallback to port 8181
- All 77 frontend tests now pass
* Fix make stop-local to avoid Docker daemon interference
Replace aggressive kill -9 with targeted process termination:
- Filter processes by command name (node/vite/python/uvicorn) before killing
- Use graceful SIGTERM instead of SIGKILL
- Add process verification to avoid killing Docker-related processes
- Improve logging with descriptive step messages
* refactor: Simplify development workflow based on comprehensive review
- Reduced Makefile from 344 lines (43 targets) to 83 lines (8 essential targets)
- Removed unnecessary environment variables (*_CONTAINER_NAME variables)
- Fixed Windows compatibility by removing Unix-specific commands
- Added security fixes to check-env.js (path validation)
- Simplified MCP container discovery to use fixed container names
- Fixed 'make stop' to properly handle Docker Compose profiles
- Updated documentation to reflect simplified workflow
- Restored original .env.example with comprehensive Supabase key documentation
This addresses all critical issues from code review:
- Cross-platform compatibility ✅
- Security vulnerabilities fixed ✅
- 81% reduction in complexity ✅
- Maintains all essential functionality ✅
All tests pass: Frontend (77/77), Backend (267/267)
* feat: Add granular test and lint commands to Makefile
- Split test command into test-fe and test-be for targeted testing
- Split lint command into lint-fe and lint-be for targeted linting
- Keep original test and lint commands that run both
- Update help text with new commands for better developer experience
* feat: Improve Docker Compose detection and prefer modern syntax
- Prefer 'docker compose' (plugin) over 'docker-compose' (standalone)
- Add better error handling in Makefile with proper exit on failures
- Add Node.js check before running environment scripts
- Pass environment variables correctly to frontend in hybrid mode
- Update all documentation to use modern 'docker compose' syntax
- Auto-detect which Docker Compose version is available
* docs: Update CONTRIBUTING.md to reflect simplified development workflow
- Add Node.js 18+ as prerequisite for hybrid development
- Mark Make as optional throughout the documentation
- Update all docker-compose commands to modern 'docker compose' syntax
- Add Make command alternatives for testing (make test, test-fe, test-be)
- Document make dev for hybrid development mode
- Remove linting requirements until codebase errors are resolved
* fix: Rename frontend service to archon-frontend for consistency
Aligns frontend service naming with other services (archon-server, archon-mcp, archon-agents) for better consistency in Docker image naming patterns.
---------
Co-authored-by: Zak Stam <zakscomputers@hotmail.com>
Co-authored-by: Zak Stam <zaksnet@users.noreply.github.com>
* fixed the creation and saving of docs in the project management area
* Fix logging error in threading service
Fixed TypeError when passing custom fields to Python logger by using the 'extra' parameter instead of direct keyword arguments. This resolves embedding creation failures during crawl operations.
* Apply linting fixes for better code formatting
- Added trailing commas for multi-line function calls
- Improved line breaks for better readability
* Complete logging fixes for all statements in threading service
Applied the extra parameter pattern to all remaining logging statements (11 more) to ensure consistency and prevent runtime errors when any code path is executed. This completes the fix for the entire file.
* Remove deprecated PRP testing scripts and dead code
- Removed python/src/server/testing/ folder containing deprecated test utilities
- These PRP viewer testing tools were used during initial development
- No longer needed as functionality has been integrated into main codebase
- No dependencies or references found in production code
* Remove original_archon folder from main branch
- Removed the original_archon/ directory containing the legacy Archon v1-v6 iterations
- This was the original AI agent builder system before the pivot to the current architecture
- The folder has been preserved in the 'preserve-original-archon' branch for historical reference
- Reduces repository size by ~5.2MB and removes confusion about which codebase is active
* Fix proxy when specifying PROD=true
* Add a test
* Parse before passing to vite
* PR Feedback
* Add titles to action buttons in DocumentCard, DraggableTaskCard, and TaskTableView components.
* feat(ui): add accessibility attributes to action buttons
Add type, aria-label, and aria-hidden attributes to action and icon buttons across task and document components to improve accessibility and assistive technology support.
* Fix Docker Compose default profile and error documentation
- Add 'default' profile to all services so 'docker compose up --build -d' works without --profile flag
- Update BackendStartupError.tsx to include '--profile full' in Docker command examples
- Update docker-compose.yml comments to document the new default behavior
This allows users to run either:
- docker compose up --build -d (uses default profile, starts all services)
- docker compose --profile full up --build -d (explicit profile, same result)
- docker compose --profile backend up --build -d (backend services only)
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Use generic YOUR_PROFILE placeholder instead of hardcoded 'full' profile
This allows users to choose their preferred profile (backend, full, etc.) rather than
assuming they always want the full profile for error recovery.
* Remove profiles from all services to enable default startup
- Remove profile restrictions from all services so they start with 'docker compose up'
- All services now run by default without requiring --profile flags
- Profile functionality removed - users now use default behavior only
- This enables the requested 'docker compose up --build -d' workflow
* Update error modal to use 'full' profile with helpful note
- Change from generic YOUR_PROFILE to specific 'full' profile
- Add note explaining users can replace 'full' if needed
- Maintains clarity while providing flexibility for different profiles
* Merge UX improvements from PR #443
- Update error modal to show default 'docker compose up --build -d' command
- Add better organized note structure with bullet points
- Include profile-specific fallback example for existing users
- Update README Quick Start to show default command first
- Maintain backward compatibility guidance for profile users
* Fix critical token consumption issue in list endpoints (#488)
- Add include_content parameter to ProjectService.list_projects()
- Add exclude_large_fields parameter to TaskService.list_tasks()
- Add include_content parameter to DocumentService.list_documents()
- Update all MCP tools to use lightweight responses by default
- Fix critical N+1 query problem in ProjectService (was making separate query per project)
- Add response size monitoring and logging for validation
- Add comprehensive unit and integration tests
Results:
- Projects endpoint: 99.3% token reduction (27,055 -> 194 tokens)
- Tasks endpoint: 98.2% token reduction (12,750 -> 226 tokens)
- Documents endpoint: Returns metadata with content_size instead of full content
- Maintains full backward compatibility with default parameters
- Single query optimization eliminates N+1 performance issue
* fix: include_archived flag now works correctly in task listing
- Add include_archived parameter to TaskService.list_tasks()
- Service now conditionally applies archived filter based on parameter
- Add 'archived' field to task DTO for client visibility
- Update API endpoints to pass include_archived down to service
- Remove redundant client-side filtering in API layer
- Fix type hints in integration tests (dict[str, Any] | None)
- Use pytest.skip() instead of return for proper test reporting
These fixes address the functional bug identified by CodeRabbit where
archived tasks couldn't be retrieved even when explicitly requested.
* feat: disable agents service by default using Docker profiles
- Add 'agents' profile to archon-agents service
- Remove archon-agents as dependency from archon-mcp service
- Service now only starts with --profile agents flag
- Prevents startup issues while agents service is under development
- All core functionality continues to work without agents
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: address PR review feedback for agents service disabling
- Fix misleading profile documentation at top of docker-compose.yml
- Add AGENTS_ENABLED flag for cleaner agent service handling
- Make AGENTS_SERVICE_URL configurable via environment variable
- Prevent noisy connection errors when agents service isn't running
This provides a cleaner way to disable the agents service and allows
the application to skip agent wiring when AGENTS_ENABLED=false.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Add PRP story task template and reorganize PRP commands (#508)
* Reorganize PRP commands and add story task template
- Move PRP commands to dedicated subdirectories
- Add new agent definitions for codebase analysis and library research
- Create story task PRP template for user story implementation
- Rename prp-base.md to prp_base.md for consistency
* Update .claude/agents/library-researcher.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update .claude/commands/prp-claude-code/prp-story-task-create.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update .claude/commands/prp-claude-code/prp-story-task-create.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update PRPs/templates/prp_story_task.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update PRPs/templates/prp_story_task.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* fix: resolve container startup race condition in agents service (#451) (#503)
* depends on and env var added
Update Vite configuration to enable allowed hosts
- Uncommented the allowedHosts configuration to allow for dynamic host settings based on environment variables.
- This change enhances flexibility for different deployment environments while maintaining the default localhost and specific domain access.
Needs testing to confirm proper functionality with various host configurations.
rm my domain
* Enhance Vite configuration with dynamic allowed hosts support
- Added VITE_ALLOWED_HOSTS environment variable to .env.example and docker-compose.yml for flexible host configuration.
- Updated Vite config to dynamically set allowed hosts, incorporating defaults and custom values from the environment variable.
- This change improves deployment flexibility while maintaining security by defaulting to localhost and specific domains.
Needs testing to confirm proper functionality with various host configurations.
* refactor: remove unnecessary dependency on archon-agents in docker-compose.yml
- Removed the dependency condition for archon-agents from the archon-mcp service to streamline the startup process.
- This change simplifies the service configuration and reduces potential startup issues related to agent service health checks.
Needs testing to ensure that the application functions correctly without the archon-agents dependency.
---------
Co-authored-by: Julian Gegenhuber <office@salzkammercode.at>
* Fix race condition in concurrent crawling with unique source IDs (#472)
* Fix race condition in concurrent crawling with unique source IDs
- Add unique hash-based source_id generation to prevent conflicts
- Separate source identification from display with three fields:
- source_id: 16-char SHA256 hash for unique identification
- source_url: Original URL for tracking
- source_display_name: Human-friendly name for UI
- Add comprehensive test suite validating the fix
- Migrate existing data with backward compatibility
* Fix title generation to use source_display_name for better AI context
- Pass source_display_name to title generation function
- Use display name in AI prompt instead of hash-based source_id
- Results in more specific, meaningful titles for each source
* Skip AI title generation when display name is available
- Use source_display_name directly as title to avoid unnecessary AI calls
- More efficient and predictable than AI-generated titles
- Keep AI generation only as fallback for backward compatibility
* Fix critical issues from code review
- Add missing os import to prevent NameError crash
- Remove unused imports (pytest, Mock, patch, hashlib, urlparse, etc.)
- Fix GitHub API capitalization consistency
- Reuse existing DocumentStorageService instance
- Update test expectations to match corrected capitalization
Addresses CodeRabbit review feedback on PR #472
* Add safety improvements from code review
- Truncate display names to 100 chars when used as titles
- Document hash collision probability (negligible for <1M sources)
Simple, pragmatic fixes per KISS principle
* Fix code extraction to use hash-based source_ids and improve display names
- Fixed critical bug where code extraction was using old domain-based source_ids
- Updated code extraction service to accept source_id as parameter instead of extracting from URL
- Added special handling for llms.txt and sitemap.xml files in display names
- Added comprehensive tests for source_id handling in code extraction
- Removed unused urlparse import from code_extraction_service.py
This fixes the foreign key constraint errors that were preventing code examples
from being stored after the source_id architecture refactor.
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix critical variable shadowing and source_type determination issues
- Fixed variable shadowing in document_storage_operations.py where source_url parameter
was being overwritten by document URLs, causing incorrect source_url in database
- Fixed source_type determination to use actual URLs instead of hash-based source_id
- Added comprehensive tests for source URL preservation
- Ensure source_type is correctly set to "file" for file uploads, "url" for web crawls
The variable shadowing bug was causing sitemap sources to have the wrong source_url
(last crawled page instead of sitemap URL). The source_type bug would mark all
sources as "url" even for file uploads due to hash-based IDs not starting with "file_".
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix URL canonicalization and document metrics calculation
- Implement proper URL canonicalization to prevent duplicate sources
- Remove trailing slashes (except root)
- Remove URL fragments
- Remove tracking parameters (utm_*, gclid, fbclid, etc.)
- Sort query parameters for consistency
- Remove default ports (80 for HTTP, 443 for HTTPS)
- Normalize scheme and domain to lowercase
- Fix avg_chunks_per_doc calculation to avoid division by zero
- Track processed_docs count separately from total crawl_results
- Handle all-empty document sets gracefully
- Show processed/total in logs for better visibility
- Add comprehensive tests for both fixes
- 10 test cases for URL canonicalization edge cases
- 4 test cases for document metrics calculation
This prevents database constraint violations when crawling the same
content with URL variations and provides accurate metrics in logs.
* Fix synchronous extract_source_summary blocking async event loop
- Run extract_source_summary in thread pool using asyncio.to_thread
- Prevents blocking the async event loop during AI summary generation
- Preserves exact error handling and fallback behavior
- Variables (source_id, combined_content) properly passed to thread
Added comprehensive tests verifying:
- Function runs in thread without blocking
- Error handling works correctly with fallback
- Multiple sources can be processed
- Thread safety with variable passing
* Fix synchronous update_source_info blocking async event loop
- Run update_source_info in thread pool using asyncio.to_thread
- Prevents blocking the async event loop during database operations
- Preserves exact error handling and fallback behavior
- All kwargs properly passed to thread execution
Added comprehensive tests verifying:
- Function runs in thread without blocking
- Error handling triggers fallback correctly
- All kwargs are preserved when passed to thread
- Existing extract_source_summary tests still pass
* Fix race condition in source creation using upsert
- Replace INSERT with UPSERT for new sources to prevent PRIMARY KEY violations
- Handles concurrent crawls attempting to create the same source
- Maintains existing UPDATE behavior for sources that already exist
Added comprehensive tests verifying:
- Concurrent source creation doesn't fail
- Upsert is used for new sources (not insert)
- Update is still used for existing sources
- Async concurrent operations work correctly
- Race conditions with delays are handled
This prevents database constraint errors when multiple crawls target
the same URL simultaneously.
* Add migration detection UI components
Add MigrationBanner component with clear user instructions for database schema updates. Add useMigrationStatus hook for periodic health check monitoring with graceful error handling.
* Integrate migration banner into main app
Add migration status monitoring and banner display to App.tsx. Shows migration banner when database schema updates are required.
* Enhance backend startup error instructions
Add detailed Docker restart instructions and migration script guidance. Improves user experience when encountering startup failures.
* Add database schema caching to health endpoint
Implement smart caching for schema validation to prevent repeated database queries. Cache successful validations permanently and throttle failures to 30-second intervals. Replace debug prints with proper logging.
* Clean up knowledge API imports and logging
Remove duplicate import statements and redundant logging. Improves code clarity and reduces log noise.
* Remove unused instructions prop from MigrationBanner
Clean up component API by removing instructions prop that was accepted but never rendered. Simplifies the interface and eliminates dead code while keeping the functional hardcoded migration steps.
* Add schema_valid flag to migration_required health response
Add schema_valid: false flag to health endpoint response when database schema migration is required. Improves API consistency without changing existing behavior.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Hotfix - crawls hanging after embedding rate limiting
* Moving Dockerfiles to uv for package installation (#533)
* Moving Dockerfiles to uv for package installation
* Updating uv installation for CI
* Reduced the size of sentence-transformers by making it CPU only, including reranking by default now (#534)
* CI fails now when unit tests for backend fail (#536)
* CI fails now when unit tests for backend fail
* Fixing up a couple unit tests
* Documentation improvements for MCP and README (#540)
* Spacing updates for Make installation in README
* add PRPs/completed/ to gitignore
* add archon-coderabbit-helper slash command
* refactor: Remove Socket.IO and implement HTTP polling architecture (#514)
* refactor: Remove Socket.IO and consolidate task status naming
Major refactoring to simplify the architecture:
1. Socket.IO Removal:
- Removed all Socket.IO dependencies and code (~4,256 lines)
- Replaced with HTTP polling for real-time updates
- Added new polling hooks (usePolling, useDatabaseMutation, etc.)
- Removed socket services and handlers
2. Status Consolidation:
- Removed UI/DB status mapping layer
- Using database values directly (todo, doing, review, done)
- Removed obsolete status types and mapping functions
- Updated all components to use database status values
3. Simplified Architecture:
- Cleaner separation between frontend and backend
- Reduced complexity in state management
- More maintainable codebase
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* feat: Add loading states and error handling for UI operations
- Added loading overlay when dragging tasks between columns
- Added loading state when switching between projects
- Added proper error handling with toast notifications
- Removed remaining Socket.IO references
- Improved user feedback during async operations
* docs: Add comprehensive polling architecture documentation
Created developer guide explaining:
- Core polling components and hooks
- ETag caching implementation
- State management patterns
- Migration from Socket.IO
- Performance optimizations
- Developer guidelines and best practices
* fix: Correct method name for fetching tasks
- Fixed projectService.getTasks() to projectService.getTasksByProject()
- Ensures consistent naming throughout the codebase
- Resolves error when refreshing tasks after drag operations
* docs: Add comprehensive API naming conventions guide
Created naming standards documentation covering:
- Service method naming patterns
- API endpoint conventions
- Component and hook naming
- State variable naming
- Type definitions
- Common patterns and anti-patterns
- Migration notes from Socket.IO
* docs: Update CLAUDE.md with polling architecture and naming conventions
- Replaced Socket.IO references with HTTP polling architecture
- Added polling intervals and ETag caching documentation
- Added API naming conventions section
- Corrected task endpoint patterns (use getTasksByProject, not getTasks)
- Added state naming patterns and status values
* refactor: Remove Socket.IO and implement HTTP polling architecture
Complete removal of Socket.IO/WebSocket dependencies in favor of simple HTTP polling:
Frontend changes:
- Remove all WebSocket/Socket.IO references from KnowledgeBasePage
- Implement useCrawlProgressPolling hook for progress tracking
- Fix polling hook to prevent ERR_INSUFFICIENT_RESOURCES errors
- Add proper cleanup and state management for completed crawls
- Persist and restore active crawl progress across page refreshes
- Fix agent chat service to handle disabled agents gracefully
Backend changes:
- Remove python-socketio from requirements
- Convert ProgressTracker to in-memory state management
- Add /api/crawl-progress/{id} endpoint for polling
- Initialize ProgressTracker immediately when operations start
- Remove all Socket.IO event handlers and cleanup commented code
- Simplify agent_chat_api to basic REST endpoints
Bug fixes:
- Fix race condition where progress data wasn't available for polling
- Fix memory leaks from recreating polling callbacks
- Fix crawl progress URL mismatch between frontend and backend
- Add proper error filtering for expected 404s during initialization
- Stop polling when crawl operations complete
This change simplifies the architecture significantly and makes it more robust
by removing the complexity of WebSocket connections.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix data consistency issue in crawl completion
- Modify add_documents_to_supabase to return actual chunks stored count
- Update crawl orchestration to validate chunks were actually saved to database
- Throw exception when chunks are processed but none stored (e.g., API key failures)
- Ensure UI shows error state instead of false success when storage fails
- Add proper error field to progress updates for frontend display
This prevents misleading "crawl completed" status when backend fails to store data.
* Consolidate API key access to unified LLM provider service pattern
- Fix credential service to properly store encrypted OpenAI API key from environment
- Remove direct environment variable access pattern from source management service
- Update both extract_source_summary and generate_source_title_and_metadata to async
- Convert all LLM operations to use get_llm_client() for multi-provider support
- Fix callers in document_storage_operations.py and storage_services.py to use await
- Improve title generation prompt with better context and examples for user-readable titles
- Consolidate on single pattern that supports OpenAI, Google, Ollama providers
This fixes embedding service failures while maintaining compatibility for future providers.
* Fix async/await consistency in source management services
- Make update_source_info async and await it properly
- Fix generate_source_title_and_metadata async calls
- Improve source title generation with URL-based detection
- Remove unnecessary threading wrapper for async operations
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: correct API response handling in MCP project polling
- Fix polling logic to properly extract projects array from API response
- The API returns {projects: [...]} but polling was trying to iterate directly over response
- This caused 'str' object has no attribute 'get' errors during project creation
- Update both create_project polling and list_projects response handling
- Verified all MCP tools now work correctly including create_project
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: Optimize project switching performance and eliminate task jumping
- Replace race condition-prone polling refetch with direct API calls for immediate task loading (100-200ms vs 1.5-2s)
- Add polling suppression during direct API calls to prevent task jumping from double setTasks() calls
- Clear stale tasks immediately on project switch to prevent wrong data visibility
- Maintain polling for background updates from agents/MCP while optimizing user-initiated actions
Performance improvements:
- Project switches now load tasks in 100-200ms instead of 1.5-2 seconds
- Eliminated visual task jumping during project transitions
- Clean separation: direct calls for user actions, polling for external updates
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: Remove race condition anti-pattern and complete Socket.IO removal
Critical fixes addressing code review findings:
**Race Condition Resolution:**
- Remove fragile isLoadingDirectly flag that could permanently disable polling
- Remove competing polling onSuccess callback that caused task jumping
- Clean separation: direct API calls for user actions, polling for external updates only
**Socket.IO Removal:**
- Replace projectCreationProgressService with useProgressPolling HTTP polling
- Remove all Socket.IO dependencies and references
- Complete migration to HTTP-only architecture
**Performance Optimization:**
- Add ETag support to /projects/{project_id}/tasks endpoint for 70% bandwidth savings
- Remove competing TasksTab onRefresh system that caused multiple API calls
- Single source of truth: polling handles background updates, direct calls for immediate feedback
**Task Management Simplification:**
- Remove onRefresh calls from all TasksTab operations (create, update, delete, move)
- Operations now use optimistic updates with polling fallback
- Eliminates 3-way race condition between polling, direct calls, and onRefresh
Result: Fast project switching (100-200ms), no task jumping, clean polling architecture
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Remove remaining Socket.IO and WebSocket references
- Remove WebSocket URL configuration from api.ts
- Clean up WebSocket tests and mocks from test files
- Remove websocket parameter from embedding service
- Update MCP project tools tests to match new API response format
- Add example real test for usePolling hook
- Update vitest config to properly include test files
* Add comprehensive unit tests for polling architecture
- Add ETag utilities tests covering generation and checking logic
- Add progress API tests with 304 Not Modified support
- Add progress service tests for operation tracking
- Add projects API polling tests with ETag validation
- Fix projects API to properly handle ETag check independently of response object
- Test coverage for critical polling components following MCP test patterns
* Remove WebSocket functionality from service files
- Remove getWebSocketUrl imports that were causing runtime errors
- Replace WebSocket log streaming with deprecation warnings
- Remove unused WebSocket properties and methods
- Simplify disconnectLogs to no-op functions
These services now use HTTP polling exclusively as part of the
Socket.IO to polling migration.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix memory leaks in mutation hooks
- Add isMountedRef to track component mount status
- Guard all setState calls with mounted checks
- Prevent callbacks from firing after unmount
- Apply fix to useProjectMutation, useDatabaseMutation, and useAsyncMutation
Addresses Code Rabbit feedback about potential state updates after
component unmount. Simple pragmatic fix without over-engineering
request cancellation.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Document ETag implementation and limitations
- Add concise documentation explaining current ETag implementation
- Document that we use simple equality check, not full RFC 7232
- Clarify this works for our browser-to-API use case
- Note limitations for future CDN/proxy support
Addresses Code Rabbit feedback about RFC compliance by documenting
the known limitations of our simplified implementation.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Remove all WebSocket event schemas and functionality
- Remove WebSocket event schemas from projectSchemas.ts
- Remove WebSocket event types from types/project.ts
- Remove WebSocket initialization and subscription methods from projectService.ts
- Remove all broadcast event calls throughout the service
- Clean up imports to remove unused types
Complete removal of WebSocket infrastructure in favor of HTTP polling.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix progress field naming inconsistency
- Change backend API to return 'progress' instead of 'percentage'
- Remove unnecessary mapping in frontend
- Use consistent 'progress' field name throughout
- Update all progress initialization to use 'progress' field
Simple consolidation to one field name instead of mapping between two.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix tasks polling data not updating UI
- Update tasks state when polling returns new data
- Keep UI in sync with server changes for selected project
- Tasks now live-update from external changes without project switching
The polling was fetching fresh data but never updating the UI state.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix incorrect project title in pin/unpin toast messages
- Use API response data.title instead of selectedProject?.title
- Shows correct project name when pinning/unpinning any project card
- Toast now accurately reflects which project was actually modified
The issue was the toast would show the wrong project name when pinning
a project that wasn't the currently selected one.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
* Remove over-engineered tempProjects logic
Removed all temporary project tracking during creation:
- Removed tempProjects state and allProjects combining
- Removed handleProjectCreationProgress function
- Removed progress polling for project creation
- Removed ProjectCreationProgressCard rendering
- Simplified createProject to just create and let polling pick it up
This fixes false 'creation failed' errors and simplifies the code significantly.
Project creation now shows a simple toast and relies on polling for updates.
* Optimize task count loading with parallel fetching
Changed loadTaskCountsForAllProjects to use Promise.allSettled for parallel API calls:
- All project task counts now fetched simultaneously instead of sequentially
- Better error isolation - one project failing doesn't affect others
- Significant performance improvement for users with multiple projects
- If 5 projects: from 5×API_TIME to just 1×API_TIME total
* Fix TypeScript timer type for browser compatibility
Replace NodeJS.Timeout with ReturnType<typeof setInterval> in crawlProgressService.
This makes the timer type compatible across both Node.js and browser environments,
fixing TypeScript compilation errors in browser builds.
* Add explicit status mappings for crawl progress states
Map backend statuses to correct UI states:
- 'processing' → 'processing' (use existing UI state)
- 'queued' → 'starting' (pre-crawl state)
- 'cancelled' → 'cancelled' (use existing UI state)
This prevents incorrect UI states and gives users accurate feedback about
crawl operation status.
* Fix TypeScript timer types in pollingService for browser compatibility
Replace NodeJS.Timer with ReturnType<typeof setInterval> in both
TaskPollingService and ProjectPollingService classes. This ensures
compatibility across Node.js and browser environments.
* Remove unused pollingService.ts dead code
This file was created during Socket.IO removal but never actually used.
The application already uses usePolling hooks (useTaskPolling, useProjectPolling)
which have proper ETag support and visibility handling.
Removing dead code to reduce maintenance burden and confusion.
* Fix TypeScript timer type in progressService for browser compatibility
Replace NodeJS.Timer with ReturnType<typeof setInterval> to ensure
compatibility across Node.js and browser environments, consistent with
other timer type fixes throughout the codebase.
* Fix TypeScript timer type in projectCreationProgressService
Replace NodeJS.Timeout with ReturnType<typeof setInterval> in Map type
to ensure browser/DOM build compatibility.
* Add proper error handling to project creation progress polling
Stop infinite polling on fatal errors:
- 404 errors continue polling (resource might not exist yet)
- Other HTTP errors (500, 503, etc.) stop polling and report error
- Network/parsing errors stop polling and report error
- Clear feedback to callbacks on all error types
This prevents wasting resources polling forever on unrecoverable errors
and provides better user feedback when things go wrong.
* Fix documentation accuracy in API conventions and architecture docs
- Fix API_NAMING_CONVENTIONS.md: Changed 'documents' to 'docs' and used
distinct placeholders ({project_id} and {doc_id}) to match actual API routes
- Fix POLLING_ARCHITECTURE.md: Updated import path to use relative import
(from ..utils.etag_utils) to match actual code structure
- ARCHITECTURE.md: List formatting was already correct, no changes needed
These changes ensure documentation accurately reflects the actual codebase.
* Fix type annotations in recursive crawling strategy
- Changed max_concurrent from invalid 'int = None' to 'int | None = None'
- Made progress_callback explicitly async: 'Callable[..., Awaitable[None]] | None'
- Added Awaitable import from typing
- Uses modern Python 3.10+ union syntax (project requires Python 3.12)
* Improve error logging in sitemap parsing
- Use logger.exception() instead of logger.error() for automatic stack traces
- Include sitemap URL in all error messages for better debugging
- Remove unused traceback import and manual traceback logging
- Now all exceptions show which sitemap failed with full stack trace
* Remove all Socket.IO remnants from task_service.py
Removed:
- Duplicate broadcast_task_update function definitions
- _broadcast_available flag (always False)
- All Socket.IO broadcast blocks in create_task, update_task, and archive_task
- Socket.IO related logging and error handling
- Unnecessary traceback import within Socket.IO error handler
Task updates are now handled exclusively via HTTP polling as intended.
* Complete WebSocket/Socket.IO cleanup across frontend and backend
- Remove socket.io-client dependency and all related packages
- Remove WebSocket proxy configuration from vite.config.ts
- Clean up WebSocket state management and deprecated methods from services
- Remove VITE_ENABLE_WEBSOCKET environment variable checks
- Update all comments to remove WebSocket/Socket.IO references
- Fix user-facing error messages that mentioned Socket.IO
- Preserve legitimate FastAPI WebSocket endpoints for MCP/test streaming
This completes the refactoring to HTTP polling, removing all Socket.IO
infrastructure while keeping necessary WebSocket functionality.
* Remove MCP log display functionality following KISS principles
- Remove all log display UI from MCPPage (saved ~100 lines)
- Remove log-related API endpoints and WebSocket streaming
- Keep internal log tracking for Docker container monitoring
- Simplify MCPPage to focus on server control and configuration
- Remove unused LogEntry types and streaming methods
Following early beta KISS principles - MCP logs are debug info that
developers can check via terminal/Docker if needed. UI now focuses
on essential functionality only.
* Add Claude Code command for analyzing CodeRabbit suggestions
- Create structured command for CodeRabbit review analysis
- Provides clear format for assessing validity and priority
- Generates 2-5 practical options with tradeoffs
- Emphasizes early beta context and KISS principles
- Includes effort estimation for each option
This command helps quickly triage CodeRabbit suggestions and decide
whether to address them based on project priorities and tradeoffs.
* Add in-flight guard to prevent overlapping fetches in crawl progress polling
Prevents race condition where slow responses could cause multiple concurrent
fetches for the same progressId. Simple boolean flag skips new fetches while
one is active and properly cleans up on stop/disconnect.
Co-Authored-By: Claude <noreply@anthropic.com>
* Remove unused progressService.ts dead code
File was completely unused with no imports or references anywhere in the
codebase. Other services (crawlProgressService, projectCreationProgressService)
handle their specific progress polling needs directly.
Co-Authored-By: Claude <noreply@anthropic.com>
* Remove unused project creation progress components
Both ProjectCreationProgressCard.tsx and projectCreationProgressService.ts
were dead code with no references. The service duplicated existing usePolling
functionality unnecessarily. Removed per KISS principles.
Co-Authored-By: Claude <noreply@anthropic.com>
* Update POLLING_ARCHITECTURE.md to reflect current state
Removed references to deleted files (progressService.ts,
projectCreationProgressService.ts, ProjectCreationProgressCard.tsx).
Updated to document what exists now rather than migration history.
Co-Authored-By: Claude <noreply@anthropic.com>
* Update API_NAMING_CONVENTIONS.md to reflect current state
Updated progress endpoints to match actual implementation.
Removed migration/historical references and anti-patterns section.
Focused on current best practices and architecture patterns.
Co-Authored-By: Claude <noreply@anthropic.com>
* Remove unused optimistic updates code and references
Deleted unused useOptimisticUpdates.ts hook that was never imported.
Removed optimistic upda…
* Fix: PR body newline escaping (#423) Codex-created PRs render literal \\n sequences because templates pass multi-line bodies inline to gh pr create. Writing the body to a file and using --body-file preserves newlines. Changes: - Write PR bodies to pr-body.md before calling gh pr create/edit - Switch templates to use --body-file instead of inline --body Fixes #423 * Replace hardcoded artifact paths with \$ARTIFACTS_DIR in default commands The pr-body.md write and --body-file references in archon-create-pr, archon-finalize-pr, and archon-implement-issue were pointing to an absolute path on a specific developer's machine. Replace all 7 occurrences with \$ARTIFACTS_DIR/pr-body.md, which is pre-created by the workflow executor and documented as a supported variable.
* Fix: PR body newline escaping (coleam00#423) Codex-created PRs render literal \\n sequences because templates pass multi-line bodies inline to gh pr create. Writing the body to a file and using --body-file preserves newlines. Changes: - Write PR bodies to pr-body.md before calling gh pr create/edit - Switch templates to use --body-file instead of inline --body Fixes coleam00#423 * Replace hardcoded artifact paths with \$ARTIFACTS_DIR in default commands The pr-body.md write and --body-file references in archon-create-pr, archon-finalize-pr, and archon-implement-issue were pointing to an absolute path on a specific developer's machine. Replace all 7 occurrences with \$ARTIFACTS_DIR/pr-body.md, which is pre-created by the workflow executor and documented as a supported variable.
* Fix: PR body newline escaping (coleam00#423) Codex-created PRs render literal \\n sequences because templates pass multi-line bodies inline to gh pr create. Writing the body to a file and using --body-file preserves newlines. Changes: - Write PR bodies to pr-body.md before calling gh pr create/edit - Switch templates to use --body-file instead of inline --body Fixes coleam00#423 * Replace hardcoded artifact paths with \$ARTIFACTS_DIR in default commands The pr-body.md write and --body-file references in archon-create-pr, archon-finalize-pr, and archon-implement-issue were pointing to an absolute path on a specific developer's machine. Replace all 7 occurrences with \$ARTIFACTS_DIR/pr-body.md, which is pre-created by the workflow executor and documented as a supported variable.
3 participants
Pull Request
Summary
This PR enhances the development workflow by replacing the separate
docker-compose.backend.ymlfile with Docker Compose profiles, fixing critical service discovery issues, and adding comprehensive developer tooling through an improved Makefile system.Supersedes #330 with architectural improvements and critical bug fixes. Original work by @zaksnet, enhanced with fixes for service discovery, port configuration, and developer experience improvements.
Changes Made
docker-compose.backend.ymlwith Docker Compose profiles for cleaner architecturearchon-server, notbackend-server)make doctorcommand for environment validation.SHELLFLAGS = -ecin Makefilemake dev-localto only run essential services (API + UI)Type of Change
Affected Services
Testing
Test Evidence
Checklist
Breaking Changes
None - this PR maintains backward compatibility while improving the development experience.
Additional Notes
Three Development Modes
Migration for Existing Developers
make stopto stop any running containersmake installto ensure dependencies are up to datemake devto start the improved hybrid development environmentKey Fixes from Original PR #330
make doctorcommand helps diagnose setup issuesAttribution
Co-authored with @zaksnet who provided the initial Makefile implementation and development environment improvements in PR #330.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores