Remote setup#631
Conversation
…d-coded localhost - Modified get_service_url() to check VITE_MCP_USE_PROXY for external URL construction - Added support for HOST and VITE_MCP_PROTOCOL environment variables in both Docker and local modes - Updated get_mcp_url() to use VITE_MCP_PROTOCOL from environment - Fixes MCP dashboard showing localhost instead of proper LAN domain (archon.mcdonaldhomelab.com) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This environment variable is needed for the service discovery fix to properly detect when to use external URLs vs internal container names for MCP service. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…vironment variables The /api/mcp/config endpoint was bypassing the service discovery system and directly reading HOST environment variable, causing it to always return localhost regardless of VITE_MCP_USE_PROXY setting. Now uses get_mcp_url() to get the correct URL. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The credential service was loading HOST and PORT from the database settings table and overriding the environment variables set in docker-compose. This caused the MCP dashboard to show localhost instead of the configured domain. Removed HOST and PORT from infrastructure_credentials list so they remain sourced from environment variables only. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…on targets - Development stage: Live reloading with mounted volumes - Production stage: Optimized build with static file serving - Proper build args for Vite environment variables in production 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Changed DOCKER_SOCKET from /dev/null to /var/run/docker.sock to allow the archon-server to check MCP container status in production. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Added comprehensive documentation for LAN/Production deployment including: - Complete deployment steps in README.md - List of all critical fixes applied for LAN deployment - Common issues and their solutions - Updated CLAUDE.md with environment vs database settings guidance - Updated code review checklist with fixed issues Key fixes documented: 1. Service discovery hard-coded localhost fix 2. Credential service HOST/PORT override fix 3. MCP config endpoint service discovery integration 4. Frontend multi-stage Dockerfile 5. Docker Compose environment variable additions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (8)
WalkthroughAdds unified local/LAN deployment artifacts: new env templates and unified docker-compose, Traefik configs and LAN docs, deploy script, UI multi-stage Dockerfile, environment-driven MCP/API URL resolution with optional MCP proxy, configurable CORS, and active MCP health checks. Several docs were added/updated (some duplicates). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Browser
participant UI as Frontend
participant API as Archon Server
participant Disc as Service Discovery
participant MCP as MCP Service
Note over UI,API: UI uses relative /api/* endpoints
User->>UI: Open app
UI->>API: GET /api/health
API-->>UI: Health JSON
UI->>UI: getMCPConfig()
alt useProxy == true
UI->>UI: mcpUrl = protocol://host/mcp (external)
else
UI->>UI: mcpUrl = protocol://host:port/mcp (direct)
end
UI->>API: GET /api/mcp/config
API->>Disc: get_mcp_url(service="mcp", protocol)
Disc-->>API: protocol://host[:port]
API-->>UI: { host, port, protocol }
UI->>API: GET /api/mcp/health
API->>MCP: POST mcpUrl/mcp (JSON-RPC ping)
MCP-->>API: 200/202/4xx (allowed)
API-->>UI: Healthy/Unhealthy with url
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Poem
Tip You can disable sequence diagrams in the walkthrough.Disable the ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Update environment files for unified deployment - Remove deprecated LAN migration documentation - Add simplified deployment scripts and Traefik configuration - Update frontend services for improved MCP integration - Update Python dependencies - Add PRD documentation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Include local environment configuration for unified deployment - Required for deployment setup 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
python/src/server/services/credential_service.py (3)
463-476: Case mismatch breaks provider switching (llm_provider vs LLM_PROVIDER).
set_active_provider()storesllm_provider(lowercase) inrag_strategy, butget_active_provider()readsLLM_PROVIDER(uppercase). The UI will never see updates.Apply both diffs to normalize on
LLM_PROVIDERand remain backward compatible when reading:@@ - # Get the selected provider - provider = rag_settings.get("LLM_PROVIDER", "openai") + # Get the selected provider (support legacy lowercase key) + provider = rag_settings.get("LLM_PROVIDER") or rag_settings.get("llm_provider") or "openai" @@ - return await self.set_credential( - "llm_provider", + return await self.set_credential( + "LLM_PROVIDER", provider, category="rag_strategy", description=f"Active {service_type} provider", )Also applies to: 409-417
442-454: Don’t return sentinel string as an API key for Ollama.Returning
"ollama"as the API key can leak into callers expecting a secret. UseNone.- if key_name: - return await self.get_credential(key_name) - return "ollama" if provider == "ollama" else None + if key_name: + return await self.get_credential(key_name) + return None
463-476: Use uppercase LLM_PROVIDER key in set_active_provider
In python/src/server/services/credential_service.py within set_active_provider, replace"llm_provider"with"LLM_PROVIDER"to align with existing getters (e.g. rag_settings.get("LLM_PROVIDER", …) and os.getenv("LLM_PROVIDER", …)).python/src/server/main.py (2)
224-252: Return non-200 for not-ready and migration-required health states./health currently returns 200 even when initializing or migrations are needed, causing premature green healthchecks. Use 503 (Service Unavailable) for “initializing” and 428/503 for “migration_required”.
from fastapi import FastAPI +from fastapi.responses import JSONResponse @@ @app.get("/health") async def health_check(): @@ - if not _initialization_complete: - return { + if not _initialization_complete: + return JSONResponse(status_code=503, content={ "status": "initializing", "service": "archon-backend", "timestamp": datetime.now().isoformat(), "message": "Backend is starting up, credentials loading...", "ready": False, - } + }) @@ - if not schema_status["valid"]: - return { + if not schema_status["valid"]: + return JSONResponse(status_code=503, content={ "status": "migration_required", "service": "archon-backend", "timestamp": datetime.now().isoformat(), "ready": False, "migration_required": True, "message": schema_status["message"], "migration_instructions": "Open Supabase Dashboard → SQL Editor → Run: migration/add_source_url_display_name.sql", "schema_valid": False - } + })
273-336: Don’t swallow DB/connection errors in schema check; fail clearly.Catching Exception and returning “valid: True” hides real failures. Detect specific “undefined column”/migration errors; otherwise surface as readiness failures.
- except Exception as e: + except Exception as e: error_msg = str(e).lower() @@ - # Other errors don't necessarily mean migration needed - result = {"valid": True, "message": f"Schema check inconclusive: {str(e)}"} - # Don't cache inconclusive results - allow retry - return result + # Any other error likely indicates connectivity/config issues; report as not ready + _schema_check_cache["valid"] = False + _schema_check_cache["checked_at"] = current_time + _schema_check_cache["result"] = { + "valid": False, + "message": f"Database check failed: {type(e).__name__}: {str(e)}" + } + return _schema_check_cache["result"]python/src/server/config/service_discovery.py (1)
132-137: Fix default port inference for https.When no explicit port is present, default 443 for https (not 80).
- parsed = urlparse(url) - return parsed.hostname, parsed.port or 80 + parsed = urlparse(url) + default_port = 443 if parsed.scheme == "https" else 80 + return parsed.hostname, parsed.port or default_portarchon-ui-main/src/pages/MCPPage.tsx (1)
274-279: Use computed MCP URL in instructions (avoid hard-coded http://).Ensure the Claude Code CLI example matches proxy/protocol settings.
- `2. claude mcp add --transport http archon http://${config?.host}:${config?.port}/mcp`, + (() => { + const { useProxy } = getMCPConfig(); + const url = useProxy ? getExternalMCPUrl() : `${config?.protocol || 'http'}://${config?.host}:${config?.port}/mcp`; + const transport = 'http'; + return `2. claude mcp add --transport ${transport} archon ${url}`; + })(),python/src/server/api_routes/mcp_api.py (1)
762-819: Make health check more robust and consistent.Reuse discovered URL in error paths, use tuned httpx timeouts, and catch RequestError; include stack traces in logs.
Apply:
- try: - # Get the MCP service URL from service discovery - mcp_url = get_mcp_url() - - # Test the actual MCP endpoint with a simple JSON-RPC ping - async with httpx.AsyncClient() as client: - response = await client.post( - f"{mcp_url}/mcp", - json={"jsonrpc": "2.0", "method": "ping", "id": 1}, - timeout=2.0, - headers={"Content-Type": "application/json"} - ) + try: + # Get the MCP service URL from service discovery + mcp_url = get_mcp_url() + allowed_statuses = {200, 202, 400, 404, 405, 406} + timeout = httpx.Timeout(connect=1.0, read=2.0, write=2.0) + + # Test the actual MCP endpoint with a simple JSON-RPC ping + async with httpx.AsyncClient() as client: + response = await client.post( + f"{mcp_url}/mcp", + json={"jsonrpc": "2.0", "method": "ping", "id": 1}, + timeout=timeout, + headers={"Content-Type": "application/json"}, + ) @@ - if response.status_code in [200, 202, 400, 404, 405, 406]: + if response.status_code in allowed_statuses: @@ - except httpx.TimeoutException: + except httpx.TimeoutException: # Timeout - server not responding result = { "status": "unhealthy", "service": "mcp", "error": "Connection timeout", - "url": get_mcp_url() + "url": mcp_url } safe_set_attribute(span, "status", "unhealthy") safe_set_attribute(span, "error", "timeout") - api_logger.warning(f"MCP health check timeout: {get_mcp_url()}") + api_logger.warning(f"MCP health check timeout: {mcp_url}") return result - - except Exception as e: + except httpx.RequestError as e: + result = { + "status": "unhealthy", + "service": "mcp", + "error": str(e), + "url": mcp_url, + } + safe_set_attribute(span, "status", "unhealthy") + safe_set_attribute(span, "error", str(e)) + api_logger.error(f"MCP health check request error: {e}", exc_info=True) + return result + except Exception as e: # Other errors result = { "status": "unhealthy", "service": "mcp", "error": str(e), - "url": get_mcp_url() + "url": mcp_url } safe_set_attribute(span, "status", "unhealthy") safe_set_attribute(span, "error", str(e)) - api_logger.error(f"MCP health check error: {e}") + api_logger.error(f"MCP health check error: {e}", exc_info=True) return resultarchon-ui-main/Dockerfile (1)
1-17: Slim the production image; avoid shipping build tools and dev depsCurrent prod stage inherits apk build deps and node_modules. Use a dedicated builder + minimal runner.
-# Multi-stage Dockerfile for development and production -FROM node:18-alpine AS base - -WORKDIR /app - -# Install system dependencies needed for some npm packages -RUN apk add --no-cache python3 make g++ git curl - -# Copy package files -COPY package*.json ./ - -# Install dependencies including dev dependencies for testing -RUN npm ci - -# Copy source code -COPY . . +# Multi-stage Dockerfile for development and production +FROM node:18-alpine AS builder +WORKDIR /app +RUN apk add --no-cache python3 make g++ git curl +COPY package*.json ./ +RUN npm ci +COPY . . # Development stage (default for docker-compose.yml) -FROM base AS development +FROM builder AS development # Create coverage directory with proper permissions RUN mkdir -p /app/coverage && chmod 777 /app/coverage @@ -CMD ["npm", "run", "dev"] +CMD ["npm", "run", "dev"] -# Production stage (for docker-compose-lan.yml) -FROM base AS production +# Production stage (for unified production) +FROM node:18-alpine AS production +WORKDIR /app # Accept build arguments for Vite environment variables ARG VITE_MCP_HOST=localhost @@ -# Set environment variables for the build process +# Set environment variables for the build process ENV VITE_MCP_HOST=$VITE_MCP_HOST @@ -# Build the application -RUN npm run build +# Build the application +COPY package*.json ./ +RUN npm ci +COPY . . +RUN npm run build # Install serve to serve the production build RUN npm install -g serve # Expose port 3737 EXPOSE 3737 # Serve the production build -CMD ["serve", "-s", "dist", "-l", "3737"] +CMD ["serve", "-s", "dist", "-l", "3737"]Optional: For an even smaller image, replace the production stage with nginx:alpine and
COPY --from=builder /app/dist /usr/share/nginx/html.Also applies to: 30-60
docker-compose.yml (1)
39-39: Security: docker.sock mount is powerful—gate it for prod.Mounting /var/run/docker.sock grants container-level root on the host. Gate via an env flag or use a var like in unified compose.
- - /var/run/docker.sock:/var/run/docker.sock # Docker socket for MCP container control + - ${DOCKER_SOCKET:-/var/run/docker.sock}:/var/run/docker.sock # Disable by setting DOCKER_SOCKET="" in prod
🧹 Nitpick comments (58)
python/src/server/services/credential_service.py (4)
321-325: Surface DB errors instead of returning empty results.Per backend guidelines, avoid masking failures on critical dependencies. Raise with context; let API layer map to HTTP errors.
@@ - except Exception as e: - logger.error(f"Error getting credentials for category {category}: {e}") - return {} + except Exception as e: + logger.error(f"Error getting credentials for category {category}: {e}", exc_info=True) + raise @@ - except Exception as e: - logger.error(f"Error setting credential {key}: {e}") - return False + except Exception as e: + logger.error(f"Error setting credential {key}: {e}", exc_info=True) + raise @@ - except Exception as e: - logger.error(f"Error deleting credential {key}: {e}") - return False + except Exception as e: + logger.error(f"Error deleting credential {key}: {e}", exc_info=True) + raise @@ - except Exception as e: - logger.error(f"Error listing credentials: {e}") - return [] + except Exception as e: + logger.error(f"Error listing credentials: {e}", exc_info=True) + raiseAlso applies to: 247-250, 273-276, 371-373
76-79: Include stack traces on errors.Add
exc_info=Trueso operational logs capture traces.- logger.error(f"Error initializing Supabase client: {e}") + logger.error(f"Error initializing Supabase client: {e}", exc_info=True) @@ - logger.error(f"Error encrypting value: {e}") + logger.error(f"Error encrypting value: {e}", exc_info=True) @@ - logger.error(f"Error decrypting value: {e}") + logger.error(f"Error decrypting value: {e}", exc_info=True) @@ - logger.error(f"Error loading credentials: {e}") + logger.error(f"Error loading credentials: {e}", exc_info=True)Also applies to: 106-109, 120-123, 154-157
82-96: Harden encryption key derivation for production.Static salt and a fallback default key are risky. Make salt configurable and require
SUPABASE_SERVICE_KEY(or a dedicated secret) in non-dev.- service_key = os.getenv("SUPABASE_SERVICE_KEY", "default-key-for-development") + service_key = os.getenv("SUPABASE_SERVICE_KEY") + if not service_key: + raise RuntimeError("SUPABASE_SERVICE_KEY must be set for credential encryption") @@ - salt=b"static_salt_for_credentials", # In production, consider using a configurable salt + salt=os.getenv("CREDENTIAL_KDF_SALT", "archon-cred-salt").encode("utf-8"),
494-563: Env export loop: consider boolean/number normalization.If values are booleans or numbers in DB, exporting
str(value)may create surprises (“False” vs “false”). Normalize to strings expected by consumers.archon-ui-main/src/config/api.ts (2)
24-39: Guard for SSR/tests and avoid module-load evaluation.
windowaccess can crash in SSR/tests; alsoAPI_FULL_URL = getApiUrl()executes at import time. Use guards and lazy getters.@@ - // For development, construct from window location - const protocol = window.location.protocol; - const host = window.location.hostname; + // For development, construct from window location (SSR-safe) + if (typeof window === 'undefined') { + return import.meta.env.VITE_API_URL || ''; + } + const protocol = window.location.protocol; + const host = window.location.hostname; @@ export const API_BASE_URL = '/api'; // Always use relative URL for API calls -export const API_FULL_URL = getApiUrl(); +// Prefer lazy getters to avoid SSR/test crashes +export function getApiFullUrl(): string { + return getApiUrl(); +} +export function getWsUrl(): string { + const base = getApiUrl(); + const wsProto = (typeof window !== 'undefined' && window.location.protocol === 'https:') ? 'wss:' : 'ws:'; + const host = (typeof window !== 'undefined') ? window.location.host : ''; + if (!base) return `${wsProto}//${host}`; + return `${wsProto}//${base.replace(/^https?:\/\//, '')}`; +}
28-36: Port tri-state: handle whitespace and invalid values.Trim
portEnvand validate numeric; log when ignored.- const portEnv = import.meta.env.VITE_ARCHON_SERVER_PORT; + const portEnvRaw = import.meta.env.VITE_ARCHON_SERVER_PORT; + const portEnv = typeof portEnvRaw === 'string' ? portEnvRaw.trim() : portEnvRaw;docs/lan-migration/README.md (2)
43-48: Add a language to the fenced code block (markdownlint MD040).Use
textto satisfy linters.-``` +```text DEPLOYMENT_MODE=local → localhost:3737 (Direct HTTP) ↓ (instant switch via env file) DEPLOYMENT_MODE=lan → https://archon.mcdonaldhomelab.com (HTTPS via Traefik)--- `65-66`: **Reconsider “No authentication required on LAN.”** Document the rationale or scope-limit it; recommend optional auth and network ACLs to reduce lateral-movement risk. </blockquote></details> <details> <summary>docs/lan-migration/troubleshooting-traefik.md (1)</summary><blockquote> `150-156`: **Specify languages for log/code blocks (markdownlint MD040).** Mark these as `text` or `log` to satisfy linters. ```diff -``` +```text archon-ui logs: INFO Accepting connections at http://localhost:3737 HTTP GET / HTTP Returned 200 in X ms@@
-+text
traefik logs:
level=debug msg="Router archon-frontend@docker matched"
level=debug msg="Service archon-frontend@docker selected"Also applies to: 158-163
docs/lan-migration/code-review-checklist.md (1)
44-51: Add blank lines around tables (markdownlint MD058).Insert an empty line before and after each table.
-#### Primary Configuration Files -| File | What to Check | Required Changes | +#### Primary Configuration Files + +| File | What to Check | Required Changes | |------|--------------|------------------| @@ -| `src/server/main.py` | CORS configuration | Use `os.getenv("CORS_ORIGINS", "").split(",")` | + +| `src/server/main.py` | CORS configuration | Use `os.getenv("CORS_ORIGINS", "").split(",")` | | `src/server/config.py` | Service configuration | All service URLs from environment | @@ -#### Files to Review -| File | What to Check | Required Changes | +#### Files to Review + +| File | What to Check | Required Changes | |------|--------------|------------------|Also applies to: 74-82, 107-113
docs/lan-migration/brownfield-architecture.md (2)
21-29: Add language to fenced code block (markdownlint MD040).Mark stack summary as
text.-``` +```text Frontend: React 18.3.1, TypeScript 5.5.4, Vite 5.2.0, TailwindCSS 3.4.17 Backend: Python 3.12, FastAPI, Uvicorn, Pydantic 2.0+ Database: PostgreSQL (Supabase), pgvector for embeddings Infrastructure: Docker Compose, uv for Python dependency management Testing: Vitest (frontend), Pytest (backend) Linting: ESLint + TypeScript ESLint, Ruff + Mypy--- `992-1094`: **Convert emphasized section labels to proper headings (markdownlint MD036).** Replace bolded labels like “Objective:” with `### Objective`/`####` headings for lint compliance. </blockquote></details> <details> <summary>docs/lan-migration/brownfield-architecture-template.md (2)</summary><blockquote> `10-22`: **Add language to ASCII diagrams/trees (markdownlint MD040).** Use `text` for diagrams and source trees. ```diff -``` +```text ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐ @@ -``` +``` @@ -``` +```text C:\funstuff\Archon/ ├── python/ @@ -``` +```Also applies to: 55-73
192-206: Use headings instead of emphasis for section labels (markdownlint MD036).Promote “Phase …” labels to
###/####headings.docs/lan-migration/migration-checklist.md (1)
130-147: Small consistency nit: add language hints to fenced blocks.Blocks already use
bash; keep consistent across all CLI snippets (looks good here). No changes required.docs/lan-migration/traefik-configuration.md (4)
45-47: Set acme.json file permissions to 600 to avoid ACME failures.Traefik requires strict permissions on ACME storage.
Add to the doc after mounting acme.json:
touch ./data/acme.json && chmod 600 ./data/acme.json
226-231: Healthchecks assume curl is present; switch to CMD-SHELL + wget for busybox images.Many minimal images lack curl. Using wget avoids false negatives.
- healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:${ARCHON_SERVER_PORT:-8181}/health"] + healthcheck: + test: ["CMD-SHELL", "wget -qO- http://localhost:${ARCHON_SERVER_PORT:-8181}/health >/dev/null 2>&1 || exit 1"] @@ - healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:${ARCHON_MCP_PORT:-8051}/health"] + healthcheck: + test: ["CMD-SHELL", "wget -qO- http://localhost:${ARCHON_MCP_PORT:-8051}/health >/dev/null 2>&1 || exit 1"] @@ - healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:3737"] + healthcheck: + test: ["CMD-SHELL", "wget -qO- http://localhost:3737 >/dev/null 2>&1 || exit 1"]Also applies to: 270-274, 305-309
39-41: Cloudflare envs are unnecessary for httpChallenge; remove or document conditional usage.You only need CF_API_EMAIL/CF_DNS_API_TOKEN when dnsChallenge is enabled. Keeping them can confuse operators.
Propose adding a note: “Set CF_* only if dnsChallenge is enabled; they are ignored for httpChallenge.”
Also applies to: 100-114
9-14: Fix markdownlint MD040: add language hints to fenced blocks.Unlabeled fences trigger lint errors. Use text for directory trees and placeholder sections.
-``` +```text traefik/ ├── docker-compose.yml # Main Traefik container └── data/ ├── config.yml # Dynamic configuration └── traefik.yml # Static configuration@@
- routers:
- routers: # (no static routers)
@@
- services:
- services: # (no static services)
Also applies to: 161-166 </blockquote></details> <details> <summary>docs/lan-migration/product-requirements-document.md (2)</summary><blockquote> `54-57`: **Add language specifiers to fenced code blocks (markdownlint MD040).** Label user-story blocks as text to satisfy linters and improve readability. ```diff -``` +```text AS A knowledge worker on the local network I WANT TO access Archon from any device SO THAT I can use the knowledge base from my laptop, desktop, or tablet@@
-+text
AS A developer using MCP tools
I WANT TO connect to the MCP server remotely
SO THAT I can integrate Archon with my IDE from any workstation@@ -``` +```text AS A system administrator I WANT TO deploy Archon once on a server SO THAT all users can access it without individual installationsAlso applies to: 60-63, 66-70 --- `115-122`: **Clarify “LAN-only, zero-auth” scope and risks.** Spell out that anyone on the LAN can access the app; suggest optional IP allowlist or basic auth toggle for sensitive installations. Add a “Security Considerations” sub-bullet stating: “No authentication by design; consider enabling basic auth or IP allowlists if LAN trust is limited.” </blockquote></details> <details> <summary>python/src/server/main.py (3)</summary><blockquote> `169-175`: **Trim and validate CORS origins from env.** Guard against whitespace and empty entries to avoid silent misconfigurations. ```diff -config = get_config() -cors_origins = config.cors_origins.split(",") if config.cors_origins else ["http://localhost:3737"] +config = get_config() +cors_origins = ( + [o.strip() for o in config.cors_origins.split(",") if o.strip()] + if config.cors_origins else ["http://localhost:3737"] +)
182-195: Avoid toggling a global logger’s level per-request; race-prone.Changing uvicorn.access level inside middleware is not concurrency-safe. Use a logging Filter to drop health-check paths.
Example filter (add near top-level):
class HealthPathFilter(logging.Filter): def filter(self, record: logging.LogRecord) -> bool: msg = getattr(record, "args", None) if isinstance(msg, tuple) and len(msg) > 1: path = str(msg[1]) if path in ("/health", "/api/health"): return False return True uvicorn_logger = logging.getLogger("uvicorn.access") uvicorn_logger.setLevel(logging.INFO) uvicorn_logger.addFilter(HealthPathFilter())Then remove the skip_health_check_logs middleware entirely.
Also applies to: 56-58
76-79: Duplicate get_config import/use; import once and reuse.You import get_config twice and call it at import time and during startup. Retain a single import at module top; call get_config() only where needed.
- from .config.config import get_config - - get_config() # This will raise ConfigurationError if anon key detected + # Validate environment configuration; raises on invalid settings + get_config()Also applies to: 169-171
archon-ui-main/src/services/mcpServerService.ts (2)
142-145: Avoid “LEGACY” keyword in comments per repo guidelines.Rephrase to describe current functionality; keep deprecation note if needed.
- // LEGACY ARCHON TOOL ACCESS (For backward compatibility) + // Backward compatibility: direct Archon MCP tool access (prefer mcpClientService)
171-178: Add retry with exponential backoff around fetch to meet frontend service guidelines.External calls should retry then fail with a specific message including URL/context.
- const response = await fetch(mcpUrl, { + const response = await fetchWithRetry(mcpUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(mcpRequest) });Add this helper (outside class):
async function fetchWithRetry(input: RequestInfo, init: RequestInit, retries = 3, baseDelayMs = 250): Promise<Response> { let lastErr: unknown; for (let i = 0; i <= retries; i++) { try { const resp = await fetch(input, init); if (resp.ok || (resp.status >= 400 && resp.status < 500)) return resp; // don't retry client errors lastErr = new Error(`HTTP ${resp.status}`); } catch (e) { lastErr = e; } await new Promise(r => setTimeout(r, baseDelayMs * 2 ** i)); } const url = typeof input === 'string' ? input : (input as Request).url; throw new Error(`MCP request failed after ${retries + 1} attempts (${url}): ${lastErr instanceof Error ? lastErr.message : String(lastErr)}`); }README.md (2)
520-525: Use Compose v2 CLI consistently ("docker compose" not "docker-compose").The README mixes both forms. Prefer "docker compose" to match earlier sections and modern Compose v2.
- docker-compose -f docker-compose.unified.yml --profile prod build --no-cache - docker-compose -f docker-compose.unified.yml --profile prod up -d - docker-compose -f docker-compose.unified.yml ps + docker compose -f docker-compose.unified.yml --profile prod build --no-cache + docker compose -f docker-compose.unified.yml --profile prod up -d + docker compose -f docker-compose.unified.yml ps
347-353: Clarify proxy-mode MCP connection (no port, path is /mcp).When VITE_MCP_USE_PROXY=true, clients should use https://HOST/mcp (no explicit port). The current step 3 suggests updating “MCP port,” which can mislead.
-3. Update your AI client configuration with the new hostname and MCP port +3. Update your AI client configuration: + - Direct mode: use http(s)://HOST:ARCHON_MCP_PORT/mcp + - Proxy mode (VITE_MCP_USE_PROXY=true): use https://HOST/mcp (no port)docs/lan-migration/tech-stack-alignment.md (1)
30-36: Minor correction: “Built-in Python libs” vs netifaces.netifaces isn’t standard library; keep it under optional third-party explicitly to avoid confusion.
-| **Network Interface Detection** | Built-in Python libs | Interface enumeration | Detect available network interfaces for binding | Use standard library `socket`, `netifaces` if needed | +| **Network Interface Detection** | Standard lib + optional | Interface enumeration | Detect available network interfaces for binding | Prefer stdlib `socket`; optionally use third-party `netifaces` |.env.lan.example (1)
39-43: Apply dotenv style nits and add trailing newline.Purely cosmetic; aligns with dotenv-linter warnings.
-ARCHON_SERVER_PORT=8181 -ARCHON_MCP_PORT=8051 -ARCHON_AGENTS_PORT=8052 +ARCHON_MCP_PORT=8051 +ARCHON_AGENTS_PORT=8052 +ARCHON_SERVER_PORT=8181 ARCHON_UI_PORT=3737Also add a blank line at end of file.
python/src/server/config/service_discovery.py (1)
152-158: Avoid bare Exception; log with context.Catch httpx errors explicitly and log the failure; return False per contract.
- try: - async with httpx.AsyncClient(timeout=timeout) as client: - response = await client.get(health_endpoint) - return response.status_code == 200 - except Exception: - return False + try: + async with httpx.AsyncClient(timeout=timeout) as client: + response = await client.get(health_endpoint) + return response.status_code == 200 + except httpx.RequestError as e: + # TODO: integrate structlog/logfire here for consistency + # logger.warning("health_check failed", service=service, url=health_endpoint, error=str(e), exc_info=True) + return False.env.unified.lan (1)
29-33: Proxy-mode: external MCP URL usually doesn’t need a port.If using a reverse proxy, omit VITE_MCP_PORT in public docs/config or add a separate VITE_MCP_EXTERNAL_PORT to avoid leaking internal port assumptions.
archon-ui-main/src/pages/MCPPage.tsx (2)
112-118: Fallback config: use SSE transport to match server, and keep protocol from env.Minor consistency: transport is SSE over HTTP(S).
- setConfig({ - transport: 'http', + setConfig({ + transport: 'sse', host: config.host, port: config.port, protocol: config.protocol });
62-74: Replace manual setInterval with shared polling hook (per repo guidelines).Adopt usePolling to standardize retries/backoff and cleanup.
- useEffect(() => { - loadStatus(); - loadConfiguration(); - statusPollInterval.current = setInterval(loadStatus, 5000); - return () => { if (statusPollInterval.current) clearInterval(statusPollInterval.current); }; - }, []); + // Example: + // const { start, stop } = usePolling(loadStatus, 5000); + // useEffect(() => { loadConfiguration(); start(); return stop; }, []);python/src/server/api_routes/mcp_api.py (2)
442-449: Harden log decoding against binary/invalid UTF-8.Avoids exceptions on non-UTF8 bytes.
Apply:
- if isinstance(log_line, bytes): - log_line = log_line.decode("utf-8").strip() + if isinstance(log_line, bytes): + log_line = log_line.decode("utf-8", errors="replace").strip()
17-17: Restrict httpx version spec
httpx is already declared in python/pyproject.toml (lines 58, 80, 94, 126); pin to a fixed version or add an upper bound (e.g.,httpx>=0.24.0,<0.25.0).docs/lan-migration/executive-summary.md (1)
17-23: Fix markdownlint MD058: surround table with blank lines.Adds blank lines before/after the table.
Apply:
-### KEY BENEFITS -| Benefit | Current → Future | +### KEY BENEFITS + +| Benefit | Current → Future | @@ -| **URL** | localhost:3737 → archon.mcdonaldhomelab.com | +| **URL** | localhost:3737 → archon.mcdonaldhomelab.com | +archon-ui-main/src/config/mcp.ts (2)
28-33: Harden parsing and narrow protocol typeGuard against NaN ports and unexpected protocol values; also de-duplicate the "/mcp" path.
Apply:
+const MCP_BASE_PATH = '/mcp'; + export function getMCPConfig(): MCPConfig { const host = import.meta.env.VITE_MCP_HOST || 'localhost'; - const protocol = import.meta.env.VITE_MCP_PROTOCOL || 'http'; + const rawProtocol = (import.meta.env.VITE_MCP_PROTOCOL || 'http').toLowerCase(); + const protocol: 'http' | 'https' = rawProtocol === 'https' ? 'https' : 'http'; const useProxy = import.meta.env.VITE_MCP_USE_PROXY === 'true'; const port = import.meta.env.VITE_MCP_PORT || '8051'; - + const parsed = Number.parseInt(port, 10); + const safePort = Number.isFinite(parsed) ? parsed : 8051; + // If using proxy, use relative path that goes through Traefik // This allows the browser to automatically use the current domain - const url = useProxy - ? '/mcp' - : `${protocol}://${host}:${port}/mcp`; + const url = useProxy + ? MCP_BASE_PATH + : `${protocol}://${host}:${safePort}${MCP_BASE_PATH}`; return { host, - port: parseInt(port), + port: safePort, protocol, useProxy, url, transport: 'sse' }; }Also applies to: 40-47
56-66: Consider reusing MCP_BASE_PATH in external URL builderTiny consistency win and fewer string literals.
export function getExternalMCPUrl(): string { const config = getMCPConfig(); // If using proxy, construct the full external URL if (config.useProxy) { - return `${config.protocol}://${config.host}/mcp`; + return `${config.protocol}://${config.host}${MCP_BASE_PATH}`; } // Otherwise, use the direct connection URL - return `${config.protocol}://${config.host}:${config.port}/mcp`; + return `${config.protocol}://${config.host}:${config.port}${MCP_BASE_PATH}`; }docs/lan-migration/lan-deployment-guide.md (2)
49-58: Use Docker Compose v2 CLI consistentlyProject prerequisites call for Compose v2; prefer “docker compose …” (space) over the deprecated “docker-compose …”.
-docker-compose -f docker-compose-lan.yml up -d -docker-compose ps -docker-compose logs -f +docker compose -f docker-compose-lan.yml up -d +docker compose ps +docker compose logs -f @@ -docker-compose -f docker-compose-lan.yml logs -f +docker compose -f docker-compose-lan.yml logs -f @@ -docker-compose -f docker-compose-lan.yml up -d --build +docker compose -f docker-compose-lan.yml up -d --build @@ -docker-compose -f docker-compose-lan.yml down +docker compose -f docker-compose-lan.yml down @@ -docker-compose -f docker-compose-lan.yml down -v +docker compose -f docker-compose-lan.yml down -vAlso applies to: 121-126, 130-135, 139-144
79-85: Fix markdownlint issues (tables and headings)Add blank lines around the table and convert emphasized lines to real headings.
-### Service Routing -| Service | External URL | Internal Port | Traefik Labels | +### Service Routing + +| Service | External URL | Internal Port | Traefik Labels | |---------|--------------|---------------|----------------| @@ -**SSL Certificate Not Generated** +#### SSL Certificate Not Generated @@ -**API Requests Failing** +#### API Requests Failing @@ -**Frontend Not Loading** +#### Frontend Not Loading @@ -**Service Communication Issues** +#### Service Communication IssuesAlso applies to: 150-170
UNIFIED-DEPLOYMENT.md (1)
1-34: Unified flow reads clean; good quick-starts and MCP client notesLooks good. Consider a follow-up pass to ensure all docs use the same .env filenames (.env.unified.local/.lan vs .env.local/.lan) for consistency.
Also applies to: 135-141
docs/lan-migration/migration-roadmap.md (3)
5-7: Specify fenced-code languages (mermaid/text) to satisfy linters and renderers-```mermaid +```mermaid @@ -```mermaid +```mermaid @@ -``` +```text +PROJECT: Archon LAN Migration +━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +Overall Progress: [░░░░░░░░░░] 0% + +✅ Planning [██████████] 100% +⏳ Prerequisites [░░░░░░░░░░] 0% +⏳ Configuration [░░░░░░░░░░] 0% +⏳ Deployment [░░░░░░░░░░] 0% +⏳ Testing [░░░░░░░░░░] 0% +⏳ Documentation [░░░░░░░░░░] 0% + +Next Action: Run Day-1 Checklist +Blockers: None +Risk Level: LOW 🟢Also applies to: 99-101, 161-166 --- `182-191`: **Remove trailing punctuation in headings** Minor markdownlint cleanup. ```diff -### Must Complete in Order: +### Must Complete in Order @@ -### Can Run in Parallel: +### Can Run in Parallel
60-94: Add languages to non-mermaid blocksLabel ASCII “phase flow” and “parallel workstreams” blocks as text.
-``` +```text @@ -``` +```textAlso applies to: 124-144
docs/lan-migration/unified-deployment-strategy.md (1)
84-91: Make proxy network name configurableHelps when Traefik uses a non-default network name.
- proxy: - external: ${USE_PROXY_NETWORK:-false} + proxy: + name: ${PROXY_NETWORK:-proxy} + external: ${USE_PROXY_NETWORK:-false}docs/lan-migration/day1-checklist.md (2)
36-44: Add languages to fenced blocks (markdownlint MD040)Use “text” for checklists.
-``` +```text @@ -``` +```text @@ -``` +```textAlso applies to: 47-53, 237-247
60-64: Prefer Docker Compose v2 CLIKeep commands consistent with the v2 requirement.
-☐ docker-compose -f docker-compose-lan.yml logs -f +☐ docker compose -f docker-compose-lan.yml logs -f @@ -docker-compose ps > ~/archon-backup/current-containers.txt -docker network ls > ~/archon-backup/current-networks.txt +docker compose ps > ~/archon-backup/current-containers.txt +docker network ls > ~/archon-backup/current-networks.txtAlso applies to: 120-126, 128-132
archon-ui-main/Dockerfile (1)
30-31: Comment driftUpdate the comment to reference the unified compose rather than docker-compose-lan.yml.
-# Production stage (for docker-compose-lan.yml) +# Production stage (for docker-compose.unified.yml)docker-compose.yml (4)
31-35: Tighten defaults for CORS/API_BASE_URL for LAN/prod.
- Include 127.0.0.1 alongside localhost to avoid subtle CORS misses.
- Derive API_BASE_URL from HOST/ARCHON_SERVER_PORT to avoid drift when ports change.
Apply:
- - CORS_ORIGINS=${CORS_ORIGINS:-http://localhost:3737} - - API_BASE_URL=${API_BASE_URL:-http://localhost:8181} + - CORS_ORIGINS=${CORS_ORIGINS:-http://localhost:3737,http://127.0.0.1:3737} + - API_BASE_URL=${API_BASE_URL:-http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181}}
94-98: Mirror CORS/API_BASE_URL improvements in MCP.Same reasoning as server: broaden default CORS and derive API_BASE_URL.
- - CORS_ORIGINS=${CORS_ORIGINS:-http://localhost:3737} - - API_BASE_URL=${API_BASE_URL:-http://localhost:8181} + - CORS_ORIGINS=${CORS_ORIGINS:-http://localhost:3737,http://127.0.0.1:3737} + - API_BASE_URL=${API_BASE_URL:-http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181}}
142-146: Mirror CORS/API_BASE_URL improvements in agents.- - CORS_ORIGINS=${CORS_ORIGINS:-http://localhost:3737} - - API_BASE_URL=${API_BASE_URL:-http://localhost:8181} + - CORS_ORIGINS=${CORS_ORIGINS:-http://localhost:3737,http://127.0.0.1:3737} + - API_BASE_URL=${API_BASE_URL:-http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181}}
186-189: Healthcheck may fail if curl isn’t in the image.Node images often lack curl. Consider a zero-dep check or align with the Python-based probe used elsewhere.
- test: ["CMD", "curl", "-f", "http://localhost:3737"] + test: + ["CMD", "sh", "-c", "node -e \"fetch('http://localhost:3737').then(r=>process.exit(r.ok?0:1)).catch(()=>process.exit(1))\""]If your base image already installs curl, you can ignore this.
docker-compose.unified.yml (5)
171-171: Container name collisions (archon-ui).Both dev and prod services use container_name: archon-ui. Running both profiles or mixing stacks will conflict.
- container_name: archon-ui + container_name: archon-ui-dev @@ - container_name: archon-ui + container_name: archon-ui-prodAlso applies to: 221-221
175-176: Avoid drift: compute VITE_API_URL from HOST/ARCHON_SERVER_PORT by default.Current default hard-codes 8181; will be wrong if the server port changes.
- - VITE_API_URL=${VITE_API_URL:-http://localhost:8181} + - VITE_API_URL=${VITE_API_URL:-http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181}} @@ - - VITE_API_URL=${VITE_API_URL:-http://localhost:8181} + - VITE_API_URL=${VITE_API_URL:-http://${HOST:-localhost}:${ARCHON_SERVER_PORT:-8181}}Also applies to: 225-226
185-188: MCP vars named with VITE_ on server/backend may confuse.If the backend reads these, consider backend-specific names (e.g., MCP_PROTOCOL, MCP_USE_PROXY) to avoid coupling to frontend naming.
Also applies to: 235-238
193-197: Frontend healthchecks rely on curl—may not be present.Switch to a zero-dependency probe or confirm curl is installed in both dev/prod images.
- test: ["CMD", "curl", "-f", "http://localhost:3737"] + test: + ["CMD", "sh", "-c", "node -e \"fetch('http://localhost:3737').then(r=>process.exit(r.ok?0:1)).catch(()=>process.exit(1))\""]Also applies to: 243-247
254-260: YAML lint: missing trailing newline.Add a newline at EOF to satisfy linters and POSIX tools.
- name: ${PROXY_NETWORK:-proxy-internal} + name: ${PROXY_NETWORK:-proxy-internal} +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
.env.lan.example(1 hunks).env.unified.lan(1 hunks)CLAUDE.md(1 hunks)README.md(1 hunks)UNIFIED-DEPLOYMENT.md(1 hunks)archon-ui-main/Dockerfile(2 hunks)archon-ui-main/src/config/api.ts(1 hunks)archon-ui-main/src/config/mcp.ts(1 hunks)archon-ui-main/src/pages/MCPPage.tsx(4 hunks)archon-ui-main/src/services/mcpServerService.ts(4 hunks)docker-compose.unified.yml(1 hunks)docker-compose.yml(9 hunks)docs/lan-migration/README.md(1 hunks)docs/lan-migration/brownfield-architecture-template.md(1 hunks)docs/lan-migration/brownfield-architecture.md(1 hunks)docs/lan-migration/code-review-checklist.md(1 hunks)docs/lan-migration/day1-checklist.md(1 hunks)docs/lan-migration/executive-summary.md(1 hunks)docs/lan-migration/lan-deployment-guide.md(1 hunks)docs/lan-migration/migration-checklist.md(1 hunks)docs/lan-migration/migration-roadmap.md(1 hunks)docs/lan-migration/product-requirements-document.md(1 hunks)docs/lan-migration/project-brief.md(1 hunks)docs/lan-migration/tech-stack-alignment.md(1 hunks)docs/lan-migration/traefik-configuration.md(1 hunks)docs/lan-migration/troubleshooting-traefik.md(1 hunks)docs/lan-migration/unified-deployment-strategy.md(1 hunks)python/src/server/api_routes/mcp_api.py(3 hunks)python/src/server/config/config.py(3 hunks)python/src/server/config/service_discovery.py(2 hunks)python/src/server/main.py(2 hunks)python/src/server/services/credential_service.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
python/src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/src/**/*.py: Fail fast on service startup failures, missing configuration, database connection failures, authentication/authorization failures, critical dependencies unavailable, and invalid/corrupting data
Never accept or store corrupted data; on operation failure skip the item entirely rather than writing placeholders (e.g., zero embeddings)
For batch/background operations, continue processing but log each failure with details; track both successes and failures
Use specific exception types (avoid bare Exception), include context/IDs/URLs in messages, preserve full stack traces with logging (exc_info=True), and never return None/null to indicate failure—raise with details
Use database task status values directly: todo, doing, review, done
Target Python 3.12 style with 120-character line length; use Ruff for linting and Mypy for type checking
python/src/**/*.py: Target Python 3.12 and keep lines within 120 characters
Use Ruff for linting (errors, warnings, unused imports, code style) and keep code Ruff-clean
Use Mypy for type checking; maintain type-safe code across backend
Files:
python/src/server/config/config.pypython/src/server/config/service_discovery.pypython/src/server/services/credential_service.pypython/src/server/main.pypython/src/server/api_routes/mcp_api.py
python/src/server/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use FastAPI exception handlers to return rich error responses with appropriate HTTP status codes and typed error payloads
Files:
python/src/server/config/config.pypython/src/server/config/service_discovery.pypython/src/server/services/credential_service.pypython/src/server/main.pypython/src/server/api_routes/mcp_api.py
python/src/{server,agents,mcp}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
python/src/{server,agents,mcp}/**/*.py: Fail fast on service startup failures (credentials, DB, or service init) with clear errors
Treat missing configuration (env vars/invalid settings) as fatal and stop execution
Do not suppress database connection failures; bubble up immediately
Authentication/authorization failures must halt the operation and be clearly surfaced
Data corruption or validation errors should raise (use Pydantic to enforce), never silently accept
If a critical dependency is unavailable, fail immediately
Never persist invalid data that would corrupt state (e.g., zero embeddings, null FKs, malformed JSON)
Batch processing should continue, logging detailed errors per item; always return both successes and failures
Background tasks (e.g., embedding generation) should finish queues and log failures without crashing the whole process
Include context and IDs/URLs in error messages; preserve full stack traces with logging (exc_info=True)
Use specific exception types; avoid catching bare Exception
Never return None to signal failure; raise exceptions with details
For crawling/document processing: continue batches, skip failed items entirely, and log detailed errors; never store placeholders (e.g., zeroed embeddings)
Files:
python/src/server/config/config.pypython/src/server/config/service_discovery.pypython/src/server/services/credential_service.pypython/src/server/main.pypython/src/server/api_routes/mcp_api.py
python/src/server/services/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Backend service method naming mirrors CRUD patterns: get/create/update/delete with clear resource scoping
Files:
python/src/server/services/credential_service.py
archon-ui-main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use state naming conventions: is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures should be logged and not crash serving other clients
External API calls should retry with exponential backoff and ultimately fail with a clear, specific message
Include actionable context in frontend error logs/messages (what was attempted, relevant IDs/URLs)
Never return null to signal failure in async/data flows; throw errors with details
Use polling (HTTP) with provided hooks (usePolling, useDatabaseMutation, useProjectMutation); Socket.IO is removed
State naming: is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Persist theme choice in localStorage and respect Tailwind dark mode classes across components
Files:
archon-ui-main/src/config/mcp.tsarchon-ui-main/src/config/api.tsarchon-ui-main/src/pages/MCPPage.tsxarchon-ui-main/src/services/mcpServerService.ts
archon-ui-main/src/**/*.{ts,tsx,py}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid comment keywords like LEGACY, CHANGED, REMOVED; write comments that document current functionality only
Files:
archon-ui-main/src/config/mcp.tsarchon-ui-main/src/config/api.tsarchon-ui-main/src/pages/MCPPage.tsxarchon-ui-main/src/services/mcpServerService.ts
archon-ui-main/src/services/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
archon-ui-main/src/services/**/*.ts: Frontend service method naming: get[Resource]sByProject(projectId), getResource, create/update/delete[Resource]
Use GET /api/projects/{id}/tasks (not getTasks) for project tasks
Files:
archon-ui-main/src/services/mcpServerService.ts
🧠 Learnings (2)
📚 Learning: 2025-09-06T20:04:08.111Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T20:04:08.111Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in .env; optional LOGFIRE_TOKEN, LOG_LEVEL, ARCHON_*_PORT variables
Applied to files:
.env.lan.example.env.unified.landocker-compose.yml
📚 Learning: 2025-09-04T16:30:05.187Z
Learnt from: stevepresley
PR: coleam00/Archon#573
File: archon-ui-main/src/config/api.ts:15-25
Timestamp: 2025-09-04T16:30:05.187Z
Learning: Archon UI API config: Prefer lazy getters getApiFullUrl() and getWsUrl() over module-load constants to avoid SSR/test crashes. Avoid CommonJS exports patterns (Object.defineProperty(exports,…)) in ESM. Add typeof window guards with VITE_API_URL fallback inside getApiUrl()/getWebSocketUrl() when SSR safety is required.
Applied to files:
archon-ui-main/src/config/mcp.tsarchon-ui-main/src/config/api.tsarchon-ui-main/src/pages/MCPPage.tsx
🧬 Code graph analysis (4)
python/src/server/main.py (1)
python/src/server/config/config.py (1)
get_config(227-229)
archon-ui-main/src/pages/MCPPage.tsx (1)
archon-ui-main/src/config/mcp.ts (2)
getMCPConfig(28-48)getExternalMCPUrl(56-66)
python/src/server/api_routes/mcp_api.py (3)
python/src/server/config/logfire_config.py (2)
safe_set_attribute(193-206)safe_span(151-172)python/src/server/config/service_discovery.py (1)
get_mcp_url(230-233)archon-ui-main/src/services/mcpServerService.ts (1)
ServerConfig(16-22)
archon-ui-main/src/services/mcpServerService.ts (1)
archon-ui-main/src/config/mcp.ts (2)
getMCPConfig(28-48)getExternalMCPUrl(56-66)
🪛 markdownlint-cli2 (0.17.2)
docs/lan-migration/README.md
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/lan-migration/troubleshooting-traefik.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/lan-migration/brownfield-architecture-template.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
192-192: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
201-201: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
206-206: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/lan-migration/brownfield-architecture.md
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
992-992: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1008-1008: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1024-1024: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1041-1041: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1057-1057: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1073-1073: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/lan-migration/day1-checklist.md
36-36: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
237-237: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/lan-migration/lan-deployment-guide.md
79-79: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
150-150: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
155-155: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
160-160: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
165-165: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/lan-migration/traefik-configuration.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/lan-migration/executive-summary.md
17-17: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
docs/lan-migration/migration-roadmap.md
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
126-126: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
161-161: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
182-182: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
191-191: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
docs/lan-migration/code-review-checklist.md
44-44: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
74-74: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
107-107: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🪛 dotenv-linter (3.3.0)
.env.lan.example
[warning] 8-8: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
[warning] 40-40: [UnorderedKey] The ARCHON_MCP_PORT key should go before the ARCHON_SERVER_PORT key
(UnorderedKey)
[warning] 41-41: [UnorderedKey] The ARCHON_AGENTS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 50-50: [UnorderedKey] The VITE_MCP_PORT key should go before the VITE_MCP_PROTOCOL key
(UnorderedKey)
[warning] 55-55: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
.env.unified.lan
[warning] 8-8: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The BUILD_TARGET key should go before the DEPLOYMENT_MODE key
(UnorderedKey)
[warning] 21-21: [UnorderedKey] The BIND_IP key should go before the HOST key
(UnorderedKey)
[warning] 22-22: [UnorderedKey] The CORS_ORIGINS key should go before the HOST key
(UnorderedKey)
[warning] 23-23: [UnorderedKey] The API_BASE_URL key should go before the BIND_IP key
(UnorderedKey)
[warning] 32-32: [UnorderedKey] The VITE_MCP_PORT key should go before the VITE_MCP_PROTOCOL key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The ARCHON_MCP_PORT key should go before the ARCHON_SERVER_PORT key
(UnorderedKey)
[warning] 39-39: [UnorderedKey] The ARCHON_AGENTS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 49-49: [UnorderedKey] The COMPOSE_PROFILES key should go before the DOCKER_SOCKET key
(UnorderedKey)
[warning] 63-63: [UnorderedKey] The LOGFIRE_TOKEN key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 64-64: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 64-64: [UnorderedKey] The LOG_LEVEL key should go before the OPENAI_API_KEY key
(UnorderedKey)
🪛 Gitleaks (8.27.2)
.env.unified.lan
[high] 8-8: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 YAMLlint (1.37.1)
docker-compose.unified.yml
[error] 260-260: no new line character at the end of file
(new-line-at-end-of-file)
| SUPABASE_URL=https://ywqhytytvsugasmbpdfa.supabase.co | ||
| SUPABASE_SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6Inl3cWh5dHl0dnN1Z2FzbWJwZGZhIiwicm9sZSI6InNlcnZpY2Vfcm9sZSIsImlhdCI6MTc1NjQyMjIxNiwiZXhwIjoyMDcxOTk4MjE2fQ.3DnxBT2hVtQgDdrxq5L2YgElZBV2-vkhUFKnJ7txQGQ | ||
|
|
There was a problem hiding this comment.
Critical: Hardcoded Supabase service role key committed. Rotate immediately and purge history.
A real JWT service role key is present. This grants DB-wide privileges. Immediate action required:
- Revoke/rotate the Supabase service role key now (Dashboard → Project Settings → API → Regenerate).
- Force-push removal: purge from git history (filter-repo), invalidate caches.
- Do not commit real secrets. Use an example file with placeholders.
Apply this minimal diff to remove the secret from the repo:
-SUPABASE_URL=https://ywqhytytvsugasmbpdfa.supabase.co
-SUPABASE_SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6Inl3cWh5dHl0dnN1Z2FzbWJwZGZhIiwicm9sZSI6InNlcnZpY2Vfcm9sZSIsImlhdCI6MTc1NjQyMjIxNiwiZXhwIjoyMDcxOTk4MjE2fQ.3DnxBT2hVtQgDdrxq5L2YgElZBV2-vkhUFKnJ7txQGQ
+SUPABASE_URL=https://your-project.supabase.co
+SUPABASE_SERVICE_KEY=your-service-role-key-hereAdditionally, move this file to .env.unified.lan.example and add .env.unified.lan to .gitignore:
+ # .gitignore
+.env.unified.lanI can provide a sanitized commit plan and a filter-repo script if helpful.
📝 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.
| SUPABASE_URL=https://ywqhytytvsugasmbpdfa.supabase.co | |
| SUPABASE_SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6Inl3cWh5dHl0dnN1Z2FzbWJwZGZhIiwicm9sZSI6InNlcnZpY2Vfcm9sZSIsImlhdCI6MTc1NjQyMjIxNiwiZXhwIjoyMDcxOTk4MjE2fQ.3DnxBT2hVtQgDdrxq5L2YgElZBV2-vkhUFKnJ7txQGQ | |
| # .env.unified.lan (update to placeholders) | |
| -SUPABASE_URL=https://ywqhytytvsugasmbpdfa.supabase.co | |
| SUPABASE_URL=https://your-project.supabase.co | |
| SUPABASE_SERVICE_KEY=your-service-role-key-here |
| SUPABASE_URL=https://ywqhytytvsugasmbpdfa.supabase.co | |
| SUPABASE_SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6Inl3cWh5dHl0dnN1Z2FzbWJwZGZhIiwicm9sZSI6InNlcnZpY2Vfcm9sZSIsImlhdCI6MTc1NjQyMjIxNiwiZXhwIjoyMDcxOTk4MjE2fQ.3DnxBT2hVtQgDdrxq5L2YgElZBV2-vkhUFKnJ7txQGQ | |
| # .gitignore | |
| .env.unified.lan |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 8-8: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
🪛 Gitleaks (8.27.2)
[high] 8-8: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
In .env.unified.lan around lines 7 to 9 there is a committed real Supabase
service_role key; remove the secret from the file, replace it with a placeholder
(e.g. SUPABASE_SERVICE_KEY=REPLACE_ME), move the sanitized file to
.env.unified.lan.example and add .env.unified.lan to .gitignore, then
immediately rotate/revoke the exposed key in the Supabase dashboard (Project
Settings → API → Regenerate) and purge the leaked secret from repository history
using a tool like git filter-repo (or BFG) and force-push the rewritten history,
and finally notify stakeholders and invalidate any caches or CI secrets that may
have used the key.
| VITE_ARCHON_SERVER_PORT= | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Unset VITE_ARCHON_SERVER_PORT: verify consumers.
An empty value can override defaults in some loaders. Either remove the key or set an explicit value.
Run to check usages:
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts --type=py -C2 '\bVITE_ARCHON_SERVER_PORT\b|ARCHON_SERVER_PORT'Length of output: 12015
Remove or set explicit VITE_ARCHON_SERVER_PORT in .env.unified.lan. Blank entries become empty strings in import.meta.env and will override built-in defaults in api.ts/vite.config.ts; delete the key if it’s unused or assign a concrete port (e.g. 8181).
🤖 Prompt for AI Agents
In .env.unified.lan around lines 41-42 the VITE_ARCHON_SERVER_PORT key is set to
an empty value which will become an empty string in import.meta.env and override
built-in defaults; either remove the VITE_ARCHON_SERVER_PORT line entirely if
unused, or assign a concrete port number (for example
VITE_ARCHON_SERVER_PORT=8181) so the app reads the intended port rather than an
empty string.
| ports: | ||
| - "${BIND_IP:-127.0.0.1}:${ARCHON_SERVER_PORT:-8181}:8181" | ||
| environment: |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ports mismatch: container mapping conflicts with uvicorn port.
You map container port 8181, but start uvicorn on ${ARCHON_SERVER_PORT}. If ARCHON_SERVER_PORT ≠ 8181, the service won’t be reachable.
Fix by aligning both sides to the same var:
- - "${BIND_IP:-127.0.0.1}:${ARCHON_SERVER_PORT:-8181}:8181"
+ - "${BIND_IP:-127.0.0.1}:${ARCHON_SERVER_PORT:-8181}:${ARCHON_SERVER_PORT:-8181}"
@@
- "${ARCHON_SERVER_PORT:-8181}",
+ "${ARCHON_SERVER_PORT:-8181}",Alternatively, pin uvicorn to 8181 and vary only host port.
Run to spot similar issues:
Also applies to: 45-56
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'ports:\s*\n\s*-\s*"\$\{BIND_IP[^\n]+\}:\$\{([A-Z_]+)\}:(\d+)"' | sed -n 'h;$!H;${x;s/\n/\\n/g;p}'Length of output: 51
🏁 Script executed:
#!/bin/bash
set -e
FILE=$(find . -maxdepth 3 -type f -name docker-compose.unified.yml | head -n1)
echo "Inspecting $FILE"
rg -nC2 'ports:' "$FILE"
rg -nC2 'BIND_IP' "$FILE"
rg -nC2 'ARCHON_SERVER_PORT' "$FILE"Length of output: 4638
Align container port mappings with service‐listening ports
- archon-server (lines 13–15): host
${ARCHON_SERVER_PORT:-8181}→ container8181conflicts with uvicorn’s--port ${ARCHON_SERVER_PORT}. - Same mismatch in archon-mcp (79–80), archon-agents (130–131) and archon-ui (173–174).
Fix by mapping the container port to the same variable, e.g.:
- "${BIND_IP:-127.0.0.1}:${ARCHON_SERVER_PORT:-8181}:${ARCHON_SERVER_PORT:-8181}"
—or pin the service to the fixed port it actually listens on.
| - "${BIND_IP:-127.0.0.1}:${ARCHON_MCP_PORT:-8051}:8051" | ||
| environment: |
There was a problem hiding this comment.
MCP container port and healthcheck hard-coded to 8051—breaks if ARCHON_MCP_PORT changes.
Container mapping and probe should use the same var.
- - "${BIND_IP:-127.0.0.1}:${ARCHON_MCP_PORT:-8051}:8051"
+ - "${BIND_IP:-127.0.0.1}:${ARCHON_MCP_PORT:-8051}:${ARCHON_MCP_PORT:-8051}"
@@
- 'python -c "import socket; s=socket.socket(); s.connect((''localhost'', 8051)); s.close()"',
+ 'python -c "import socket,os; s=socket.socket(); s.connect((''localhost'', int(os.getenv(''ARCHON_MCP_PORT'', ''8051'')))); s.close()"',Also applies to: 107-118
🤖 Prompt for AI Agents
In docker-compose.unified.yml around lines 80-81 (and also review lines
107-118), the MCP container port mapping and healthcheck are hard-coded to 8051
which will break if ARCHON_MCP_PORT is changed; update the container port
mapping and the healthcheck/probe configuration to use the ARCHON_MCP_PORT
variable (e.g. reference ${ARCHON_MCP_PORT:-8051} for both host:container
mapping and the healthcheck target) so both mapping and probe always use the
same resolved port value.
| ARCHON_AGENTS_PORT: ${ARCHON_AGENTS_PORT:-8052} | ||
| container_name: archon-agents | ||
| ports: | ||
| - "${BIND_IP:-127.0.0.1}:${ARCHON_AGENTS_PORT:-8052}:8052" |
There was a problem hiding this comment.
Agents container/healthcheck hard-coded to 8052—same misalignment risk.
- - "${BIND_IP:-127.0.0.1}:${ARCHON_AGENTS_PORT:-8052}:8052"
+ - "${BIND_IP:-127.0.0.1}:${ARCHON_AGENTS_PORT:-8052}:${ARCHON_AGENTS_PORT:-8052}"
@@
- "import urllib.request; urllib.request.urlopen('http://localhost:8052/health').read()",
+ "import os,urllib.request; urllib.request.urlopen(f\"http://localhost:{os.getenv('ARCHON_AGENTS_PORT','8052')}/health\").read()",Also applies to: 152-158
🤖 Prompt for AI Agents
In docker-compose.unified.yml around line 131 (and similarly lines 152-158), the
agents container healthcheck/bind port is hard-coded to 8052 which can become
misaligned with the mapped host port; update the healthcheck and any port
references to use the ARCHON_AGENTS_PORT variable (or reference the same Compose
variable interpolation) instead of the literal 8052 so the container uses the
configured port consistently, e.g., replace hard-coded 8052 occurrences with
${ARCHON_AGENTS_PORT:-8052} and ensure any healthcheck CMD/endpoint targets the
interpolated value.
| ## Sample .env Structure | ||
|
|
||
| ```bash | ||
| # Master control | ||
| DEPLOYMENT_MODE=local|lan | ||
|
|
||
| # Computed values (set by deploy script) | ||
| if [ "$DEPLOYMENT_MODE" = "lan" ]; then | ||
| export HOST=archon.mcdonaldhomelab.com | ||
| export ENABLE_TRAEFIK=true | ||
| export USE_PROXY_NETWORK=true | ||
| export BIND_ADDRESS=0.0.0.0 | ||
| else | ||
| export HOST=localhost | ||
| export ENABLE_TRAEFIK=false | ||
| export USE_PROXY_NETWORK=false | ||
| export BIND_ADDRESS=127.0.0.1 | ||
| fi | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
.env “pseudocode” block is misleading; .env files can’t contain shell conditionals
Replace with explicit examples or move logic into deploy.sh.
-```bash
-# Master control
-DEPLOYMENT_MODE=local|lan
-...
-if [ "$DEPLOYMENT_MODE" = "lan" ]; then
- export HOST=archon.mcdonaldhomelab.com
- ...
-fi
-```
+```env
+# .env.unified.local
+DEPLOYMENT_MODE=local
+HOST=localhost
+ENABLE_TRAEFIK=false
+USE_PROXY_NETWORK=false
+BIND_IP=127.0.0.1
+```
+
+```env
+# .env.unified.lan
+DEPLOYMENT_MODE=lan
+HOST=archon.mydomain.com
+ENABLE_TRAEFIK=true
+USE_PROXY_NETWORK=true
+BIND_IP=0.0.0.0
+```🤖 Prompt for AI Agents
In docs/lan-migration/unified-deployment-strategy.md around lines 301–319, the
current .env "pseudocode" block uses shell conditionals which is misleading
because .env files cannot contain shell logic; replace that block with two
explicit example .env files (e.g., .env.unified.local and .env.unified.lan)
showing concrete key=value pairs for DEPLOYMENT_MODE, HOST, ENABLE_TRAEFIK,
USE_PROXY_NETWORK, and BIND_IP/BIND_ADDRESS, and/or note that any conditional
logic must live in deploy.sh which reads DEPLOYMENT_MODE and exports the
computed values; update the examples and any variable names for consistency
(BIND_IP vs BIND_ADDRESS) and remove the shell if/fi block.
| # Get MCP configuration using service discovery | ||
| import os | ||
| from urllib.parse import urlparse | ||
|
|
||
| mcp_port = int(os.getenv("ARCHON_MCP_PORT", "8051")) | ||
|
|
||
| # Configuration for SSE-only mode with actual port | ||
| # Use service discovery to get the proper MCP URL | ||
| mcp_url = get_mcp_url() | ||
| parsed_url = urlparse(mcp_url) | ||
|
|
||
| config = { | ||
| "host": "localhost", | ||
| "port": mcp_port, | ||
| "host": parsed_url.hostname, | ||
| "port": parsed_url.port, | ||
| "transport": "sse", | ||
| "protocol": parsed_url.scheme, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against None host/port; remove unused import; include scheme-derived default port.
Service discovery URLs might omit explicit port; UI expects a number. Also os is unused.
Apply:
- # Get MCP configuration using service discovery
- import os
- from urllib.parse import urlparse
+ # Get MCP configuration using service discovery
+ from urllib.parse import urlparse
@@
- mcp_url = get_mcp_url()
- parsed_url = urlparse(mcp_url)
-
- config = {
- "host": parsed_url.hostname,
- "port": parsed_url.port,
- "transport": "sse",
- "protocol": parsed_url.scheme,
- }
+ mcp_url = get_mcp_url()
+ parsed_url = urlparse(mcp_url)
+ host = parsed_url.hostname or "localhost"
+ default_port = parsed_url.port or (443 if parsed_url.scheme == "https" else 80)
+
+ config = {
+ "host": host,
+ "port": default_port,
+ "transport": "sse",
+ "protocol": parsed_url.scheme,
+ }Additionally, align with frontend interface expecting model:
# after retrieving model_choice
config["model"] = model_choice🤖 Prompt for AI Agents
In python/src/server/api_routes/mcp_api.py around lines 574 to 587, remove the
unused "import os", and make the MCP URL parsing resilient: set host to
parsed_url.hostname or "localhost" if None, determine port as parsed_url.port or
default to 443 when parsed_url.scheme == "https" else 80, and ensure port is an
integer; keep "transport" and "protocol" as before. After you obtain
model_choice, add config["model"] = model_choice so the config matches the
frontend interface.
| @router.get("/health") | ||
| async def mcp_health(): | ||
| """Health check for MCP API.""" | ||
| """Health check for MCP API - tests actual MCP server connectivity.""" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Audit logger. misuse of keyword args 'error=' across this module.*
Python logging doesn't accept arbitrary kwargs (use exc_info/extra). There are calls like api_logger.error(..., error=str(e)) elsewhere in this file that will raise TypeError at runtime.
Run to locate offenders:
Fix by switching to f-strings or %s args and add exc_info=True where appropriate.
🏁 Script executed:
#!/bin/bash
rg -nP '(api_logger|mcp_logger)\.(error|warning|info)\([^)]*\berror\s*=' -C1Length of output: 3469
Remove invalid error= kwargs and use proper logging calls
Calls like api_logger.warning("…", error=str(e)) and api_logger.error("…", error=str(e)) will raise TypeError. Replace each with either:
- Message formatting plus
exc_info=True, e.g.api_logger.warning("Could not cleanup crawling context: %s", e, exc_info=True)
- Or f-string in the message plus
exc_info=True, e.g.api_logger.error(f"Failed to get MCP tools: {e}", exc_info=True)
Offenders:
- python/src/server/main.py lines 146, 153
- python/src/server/api_routes/mcp_api.py lines 625, 731, 742
🤖 Prompt for AI Agents
In python/src/server/main.py lines 146 and 153 and
python/src/server/api_routes/mcp_api.py lines 625, 731 and 742, remove the
invalid error= keyword args passed to api_logger.warning/error and replace them
with normal logging usage: either pass the exception as a format argument and
set exc_info=True (e.g. api_logger.warning("Could not ...: %s", e,
exc_info=True)) or include the exception in the message via an f-string and set
exc_info=True (e.g. api_logger.error(f"Failed to ...: {e}", exc_info=True));
ensure no error= kwargs remain and exc_info=True is used to capture the
traceback.
| # Phase 1 parameterization variables with localhost defaults | ||
| bind_ip = os.getenv("BIND_IP", "127.0.0.1") | ||
| cors_origins = os.getenv("CORS_ORIGINS", "http://localhost:3737") | ||
| api_base_url = os.getenv("API_BASE_URL", "http://localhost:8181") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate BIND_IP early.
Fail fast on invalid IPs to match your config rigor elsewhere.
Apply:
bind_ip = os.getenv("BIND_IP", "127.0.0.1")
cors_origins = os.getenv("CORS_ORIGINS", "http://localhost:3737")
api_base_url = os.getenv("API_BASE_URL", "http://localhost:8181")
+ # Basic validations
+ try:
+ ipaddress.ip_address(bind_ip)
+ except ValueError as e:
+ raise ConfigurationError(f"BIND_IP must be a valid IP address, got: {bind_ip}") from e📝 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.
| # Phase 1 parameterization variables with localhost defaults | |
| bind_ip = os.getenv("BIND_IP", "127.0.0.1") | |
| cors_origins = os.getenv("CORS_ORIGINS", "http://localhost:3737") | |
| api_base_url = os.getenv("API_BASE_URL", "http://localhost:8181") | |
| # Phase 1 parameterization variables with localhost defaults | |
| bind_ip = os.getenv("BIND_IP", "127.0.0.1") | |
| cors_origins = os.getenv("CORS_ORIGINS", "http://localhost:3737") | |
| api_base_url = os.getenv("API_BASE_URL", "http://localhost:8181") | |
| # Basic validations | |
| try: | |
| ipaddress.ip_address(bind_ip) | |
| except ValueError as e: | |
| raise ConfigurationError(f"BIND_IP must be a valid IP address, got: {bind_ip}") from e |
🤖 Prompt for AI Agents
In python/src/server/config/config.py around lines 203 to 207, the BIND_IP value
read from the environment should be validated immediately; after assigning
bind_ip = os.getenv("BIND_IP", "127.0.0.1"), use the standard library ipaddress
module to parse and validate it (e.g., ipaddress.ip_address(bind_ip)); on
failure raise a clear exception or call sys.exit with a descriptive error so the
process fails fast on invalid IPs and matches existing config validation
behavior.
| if self.environment == Environment.DOCKER_COMPOSE: | ||
| # Docker Compose uses service names directly | ||
| # Check for override via environment variable | ||
| host = os.getenv(f"{service_name.upper().replace('-', '_')}_HOST", service_name) | ||
| url = f"{protocol}://{host}:{port}" | ||
| # For MCP service, check if proxy mode is enabled for external access | ||
| if service == "mcp" and os.getenv("VITE_MCP_USE_PROXY", "false").lower() == "true": | ||
| # Use external host and protocol for proxy mode | ||
| external_host = os.getenv("HOST", "localhost") | ||
| external_protocol = os.getenv("VITE_MCP_PROTOCOL", protocol) | ||
| url = f"{external_protocol}://{external_host}:{port}" | ||
| else: | ||
| # Docker Compose uses service names directly | ||
| # Check for override via environment variable | ||
| host = os.getenv(f"{service_name.upper().replace('-', '_')}_HOST", service_name) | ||
| url = f"{protocol}://{host}:{port}" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Proxy mode should not append internal port; allow optional external port.
When behind a reverse proxy (Traefik/Nginx) the external URL should be https://HOST/mcp (no :8051). Including the internal port can break external connectivity.
- if service == "mcp" and os.getenv("VITE_MCP_USE_PROXY", "false").lower() == "true":
- # Use external host and protocol for proxy mode
- external_host = os.getenv("HOST", "localhost")
- external_protocol = os.getenv("VITE_MCP_PROTOCOL", protocol)
- url = f"{external_protocol}://{external_host}:{port}"
+ if service == "mcp" and os.getenv("VITE_MCP_USE_PROXY", "false").lower() == "true":
+ # External (proxied) URL: prefer no explicit port; optionally allow VITE_MCP_EXTERNAL_PORT
+ external_host = os.getenv("HOST", "localhost")
+ external_protocol = os.getenv("VITE_MCP_PROTOCOL", protocol)
+ external_port = os.getenv("VITE_MCP_EXTERNAL_PORT", "").strip()
+ url = (
+ f"{external_protocol}://{external_host}:{external_port}"
+ if external_port
+ else f"{external_protocol}://{external_host}"
+ )📝 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.
| if self.environment == Environment.DOCKER_COMPOSE: | |
| # Docker Compose uses service names directly | |
| # Check for override via environment variable | |
| host = os.getenv(f"{service_name.upper().replace('-', '_')}_HOST", service_name) | |
| url = f"{protocol}://{host}:{port}" | |
| # For MCP service, check if proxy mode is enabled for external access | |
| if service == "mcp" and os.getenv("VITE_MCP_USE_PROXY", "false").lower() == "true": | |
| # Use external host and protocol for proxy mode | |
| external_host = os.getenv("HOST", "localhost") | |
| external_protocol = os.getenv("VITE_MCP_PROTOCOL", protocol) | |
| url = f"{external_protocol}://{external_host}:{port}" | |
| else: | |
| # Docker Compose uses service names directly | |
| # Check for override via environment variable | |
| host = os.getenv(f"{service_name.upper().replace('-', '_')}_HOST", service_name) | |
| url = f"{protocol}://{host}:{port}" | |
| if self.environment == Environment.DOCKER_COMPOSE: | |
| # For MCP service, check if proxy mode is enabled for external access | |
| if service == "mcp" and os.getenv("VITE_MCP_USE_PROXY", "false").lower() == "true": | |
| # External (proxied) URL: prefer no explicit port; optionally allow VITE_MCP_EXTERNAL_PORT | |
| external_host = os.getenv("HOST", "localhost") | |
| external_protocol = os.getenv("VITE_MCP_PROTOCOL", protocol) | |
| external_port = os.getenv("VITE_MCP_EXTERNAL_PORT", "").strip() | |
| url = ( | |
| f"{external_protocol}://{external_host}:{external_port}" | |
| if external_port | |
| else f"{external_protocol}://{external_host}" | |
| ) | |
| else: | |
| # Docker Compose uses service names directly | |
| # Check for override via environment variable | |
| host = os.getenv(f"{service_name.upper().replace('-', '_')}_HOST", service_name) | |
| url = f"{protocol}://{host}:{port}" |
🤖 Prompt for AI Agents
In python/src/server/config/service_discovery.py around lines 107 to 119, the
proxy-mode branch for the "mcp" service currently always appends the internal
port to the external URL, which breaks setups behind a reverse proxy; change it
to read an optional external port environment variable (e.g. VITE_MCP_PORT) and
only append ":port" when that external port is set and non-empty, otherwise
build the URL as "{protocol}://{host}{path_optional}" (i.e. no ":port"), while
keeping external_protocol defaulting to protocol and using HOST fallback as
before.
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
archon-ui-main/src/services/credentialsService.ts (6)
88-90: URL-encode dynamic path segment to prevent breakage/injectionCategory may contain spaces or reserved chars. Encode before interpolating.
Apply:
- `${this.baseUrl}/api/credentials/categories/${category}`, + `${this.baseUrl}/api/credentials/categories/${encodeURIComponent(category)}`,
130-130: Encode credential key in URLKeys with slashes/quotes will 404 or be interpreted as path traversal.
- const response = await fetch(`${this.baseUrl}/api/credentials/${key}`); + const response = await fetch(`${this.baseUrl}/api/credentials/${encodeURIComponent(key)}`);
224-231: Encode credential key in update routeSame concern as get.
- `${this.baseUrl}/api/credentials/${credential.key}`, + `${this.baseUrl}/api/credentials/${encodeURIComponent(credential.key)}`,
274-276: Encode credential key in delete routeSame concern as above.
- const response = await fetch(`${this.baseUrl}/api/credentials/${key}`, { + const response = await fetch(`${this.baseUrl}/api/credentials/${encodeURIComponent(key)}`, {
189-206: Numeric parsing drops valid zero valuesUsing
parseInt(...) || defaulttreats0as falsy and silently restores defaults. This will ignore explicit zero configs.- (settings as any)[cred.key] = - parseInt(cred.value || "0", 10) || (settings as any)[cred.key]; + { + const parsed = Number.parseInt(cred.value ?? "", 10); + (settings as any)[cred.key] = Number.isNaN(parsed) + ? (settings as any)[cred.key] + : parsed; + }
331-341: Harden parsing for code extraction settingsGuard against NaN and preserve valid zeros.
- if (typeof settings[key] === "number") { - if (key === "MAX_PROSE_RATIO") { - settings[key] = parseFloat(cred.value || "0.15"); - } else { - settings[key] = parseInt( - cred.value || settings[key].toString(), - 10, - ); - } + if (typeof settings[key] === "number") { + if (key === "MAX_PROSE_RATIO") { + const parsed = Number.parseFloat(cred.value ?? ""); + if (!Number.isNaN(parsed)) settings[key] = parsed as any; + } else { + const parsed = Number.parseInt(cred.value ?? "", 10); + if (!Number.isNaN(parsed)) settings[key] = parsed as any; + }archon-ui-main/src/services/mcpService.ts (1)
432-444: Add timeout + retry with contextual errors for external callsService methods should use backoff and include actionable context (URL/method/ids). Wrap fetch in a small helper and use it here.
- try { - const response = await fetch(mcpUrl, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify(mcpRequest) - }); + try { + const response = await fetchWithRetry(mcpUrl, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(mcpRequest) + }, { retries: 3, baseDelayMs: 300, timeoutMs: 8000, label: `MCP ${method}` }); @@ - } catch (error) { - console.error('MCP call failed:', error); + } catch (error) { + console.error('MCP call failed', { method, mcpUrl, error }); throw error; }Add this helper near the top of the file (outside the class):
async function fetchWithRetry( url: string, init: RequestInit, opts: { retries: number; baseDelayMs: number; timeoutMs: number; label?: string } ): Promise<Response> { const { retries, baseDelayMs, timeoutMs, label } = opts; let lastErr: unknown; for (let attempt = 0; attempt <= retries; attempt++) { const ac = new AbortController(); const t = setTimeout(() => ac.abort(), timeoutMs); try { const res = await fetch(url, { ...init, signal: ac.signal }); clearTimeout(t); if (res.ok) return res; lastErr = new Error(`${label || 'request'} HTTP ${res.status}`); } catch (e) { lastErr = e; } finally { clearTimeout(t); } // Exponential backoff with jitter const delay = baseDelayMs * 2 ** attempt + Math.floor(Math.random() * 100); await new Promise(r => setTimeout(r, delay)); } throw lastErr instanceof Error ? lastErr : new Error(String(lastErr)); }I can apply this to other service methods if you want.
Also applies to: 455-458
♻️ Duplicate comments (2)
.env.unified.lan (2)
41-41: Do not leave empty env values; set or remove VITE_ARCHON_SERVER_PORTEmpty string overrides defaults in import.meta.env.
-VITE_ARCHON_SERVER_PORT= +VITE_ARCHON_SERVER_PORT=8181If unused, delete the line instead.
1-64: Ship this as a template and ignore real env filesPrefer .env.unified.lan.example in repo, with .env.unified.lan in .gitignore, to reduce risk of committing real secrets.
🧹 Nitpick comments (35)
python/pyproject.toml (1)
182-182: mypy ignore_missing_imports=true is fine; consider scoping laterNo action required now. Optionally, tighten with per-module overrides once httpx/jose types settle to catch more issues without noise.
archon-ui-main/src/services/credentialsService.ts (1)
79-85: Unify error handling with contextual messagesWrap with try/catch and use
handleCredentialErrorfor network clarity and actionable context.- async getAllCredentials(): Promise<Credential[]> { - const response = await fetch(`${this.baseUrl}/api/credentials`); - if (!response.ok) { - throw new Error("Failed to fetch credentials"); - } - return response.json(); - } + async getAllCredentials(): Promise<Credential[]> { + try { + const response = await fetch(`${this.baseUrl}/api/credentials`); + if (!response.ok) { + const errorText = await response.text().catch(() => ""); + throw new Error(`HTTP ${response.status}${errorText ? `: ${errorText}` : ""}`); + } + return response.json(); + } catch (error) { + throw this.handleCredentialError(error, "Fetching credentials"); + } + }archon-ui-main/src/components/settings/FeaturesSection.tsx (1)
25-26: Rename loading state to match repo conventionUse
isLoadingper the state naming guideline.Example minimal changes:
- const [loading, setLoading] = useState(true); + const [isLoading, setIsLoading] = useState(true); ... - if (loading) return; + if (isLoading) return; ... - setLoading(true); + setIsLoading(true); ... - setLoading(false); + setIsLoading(false); ... - disabled={loading || !projectsSchemaValid} + disabled={isLoading || !projectsSchemaValid} ... - disabled={loading} + disabled={isLoading}Also applies to: 109-113, 135-136, 141-145, 167-169, 176-181, 191-194, 311-313
archon-ui-main/src/components/layouts/MainLayout.tsx (1)
53-54: Ensure timeout is always clearedMove
clearTimeoutto afinallyto avoid timer leaks on exceptions.- clearTimeout(timeoutId); + // response handling... @@ - } catch (error) { + } catch (error) { // Handle AbortError separately for timeout const errorMessage = error instanceof Error ? (error.name === 'AbortError' ? 'Request timeout (5s)' : error.message) : 'Unknown error'; // Only log after first attempt to reduce noise during normal startup if (retryCount > 0) { console.log(`Backend not ready yet (attempt ${retryCount + 1}/${maxRetries}):`, errorMessage); } // Retry if we haven't exceeded max retries if (retryCount < maxRetries) { setTimeout(() => { checkBackendHealth(retryCount + 1); }, retryDelay * Math.pow(1.5, retryCount)); // Exponential backoff for connection errors } else { console.error('Backend startup failed after maximum retries - showing error message'); setBackendReady(false); setBackendStartupFailed(true); } - } + } finally { + clearTimeout(timeoutId); + }Also applies to: 82-103
archon-ui-main/src/services/mcpService.ts (2)
406-409: Avoid “LEGACY” wording in comments per guidelinesReplace “LEGACY”/“deprecated” phrasing with neutral, current-state documentation.
- // ======================================== - // LEGACY TOOL FUNCTIONALITY (Updated for multi-client) - // ======================================== + // ======================================== + // Backward-compatible tool functionality (updated for multi-client) + // ======================================== @@ -/** - * Legacy function - replaced by mcpService.getAvailableTools() - * @deprecated Use mcpService.getAvailableTools() instead - */ +/** + * Backward-compatible helper; prefer mcpService.getAvailableTools(). + */Also applies to: 575-581
466-486: Tune logs and messages; include context, avoid noisy console.logReplace console.log/warn with concise console.info/warn that include counts/URLs, and keep “broken backend” wording out of messages shown to users.
- console.log('Attempting direct MCP tools/list call...'); + console.info('MCP: calling tools/list via protocol'); @@ - console.log('Successfully retrieved tools via MCP protocol:', validatedResult.tools.length); + console.info('MCP: tools/list returned', { count: validatedResult.tools.length }); @@ - console.warn('Direct MCP call failed, falling back to backend endpoint:', mcpError); + console.warn('MCP: protocol call failed, falling back to backend endpoint', { error: mcpError }); @@ - console.log('Backend endpoint returned:', data); + console.info('MCP backend /api/mcp/tools responded', { count: Array.isArray(data?.tools) ? data.tools.length : 0 }); @@ - console.warn('Backend returned debug placeholder - MCP tool introspection is not working'); + console.warn('MCP backend returned placeholder; tool introspection likely disabled');.env.unified.lan (3)
7-8: Fix placeholders (no spaces) and quote if neededSpaces break dotenv parsing. Use clear placeholders.
-SUPABASE_URL=your database link -SUPABASE_SERVICE_KEY=Your Key +SUPABASE_URL=https://your-project.supabase.co +SUPABASE_SERVICE_KEY=your-service-role-key
20-25: Unify domain placeholders for clarityKeep one canonical placeholder (yourdomain.com) across HOST/CORS/API/MCP host.
-HOST=archon.YourDomainName.com +HOST=archon.yourdomain.com @@ -CORS_ORIGINS=https://archon.yourdomain.com -API_BASE_URL=https://archon.yourdomain.com/api -VITE_API_URL=https://archon.yourdomain.com +CORS_ORIGINS=https://archon.yourdomain.com +API_BASE_URL=https://archon.yourdomain.com/api +VITE_API_URL=https://archon.yourdomain.com @@ -VITE_MCP_HOST=archon.yourdomainname.com +VITE_MCP_HOST=archon.yourdomain.comAlso applies to: 29-33
64-64: Add a trailing newlineSome tools expect a newline at EOF; add one to appease linters.
.env.unified.local (1)
96-96: Add trailing newlineAppease linters/CI.
.env.example (1)
30-31: Trim extra blank linesKeep the template compact.
- -traefik/data/traefik.yml (1)
35-35: Add trailing newlineFixes YAML lint complaint.
traefik/data/config.yml (5)
5-15: Avoid redirect middleware on HTTPS-only routers; apply a single “secured” chainSince routers bind to entryPoint https, redirectScheme is redundant and can confuse. Apply your “secured” chain consistently.
archon-frontend: entryPoints: - "https" rule: "Host(`archon.mcdonaldhomelab.com`)" priority: 1 middlewares: - - default-headers - - https-redirectscheme + - secured tls: {} service: archon-frontend
17-27: Same for API routerarchon-api: entryPoints: - "https" rule: "Host(`archon.mcdonaldhomelab.com`) && PathPrefix(`/api`)" priority: 2 middlewares: - - default-headers - - https-redirectscheme + - secured tls: {} service: archon-api
29-39: Same for MCP routerarchon-mcp: entryPoints: - "https" rule: "Host(`archon.mcdonaldhomelab.com`) && PathPrefix(`/mcp`)" priority: 3 middlewares: - - default-headers - - https-redirectscheme + - secured tls: {} service: archon-mcp
62-70: proxmox-transport is unusedRemove or reference it to avoid dead config.
98-102: Indentation + newlineFix indentation warnings and add newline at EOF.
deploy-lan.sh (5)
18-24: Use docker compose v2 and the configurable compose file-echo "🔄 Stopping existing containers..." -docker-compose -f docker-compose-lan.yml down +echo "🔄 Stopping existing containers..." +docker compose -f "$COMPOSE_FILE" down @@ -echo "🏗️ Building and starting containers..." -docker-compose -f docker-compose-lan.yml up -d --build +echo "🏗️ Building and starting containers..." +docker compose -f "$COMPOSE_FILE" up -d --build
39-46: Avoid hard-coded user paths in guidanceUse a placeholder so others can copy-paste safely.
- echo " cp traefik/data/config.yml /home/dmcdonald/dockge/traefik-3/data/config.yml" + echo " cp traefik/data/config.yml /path/to/your/traefik/data/config.yml"
48-51: Compose v2 for health check-docker-compose -f docker-compose-lan.yml ps +docker compose -f "$COMPOSE_FILE" ps
56-59: Print target domain from envKeeps output consistent with HOST.
-echo "3. Access Archon at: https://archon.mcdonaldhomelab.com" +echo "3. Access Archon at: https://${HOST_DOMAIN}"
29-33: Parameterize proxy network and service names
Replace hardcodedproxy,archon-ui, andarchon-serverwith variables (e.g.$PROXY_NETWORK,$SERVICE_UI,$SERVICE_SERVER), and confirm they match the services listed bydocker compose -f "$COMPOSE_FILE" ps --format '{{.Name}}'README-LAN.md (2)
25-26: Fix markdown lint (no-bare-urls) and standardize Compose CLI.Wrap bare URLs in angle brackets and prefer “docker compose” (v2).
-# Access at: https://archon.mcdonaldhomelab.com +# Access at: <https://archon.mcdonaldhomelab.com> ... -- **Domain Access**: `https://archon.mcdonaldhomelab.com` +- **Domain Access**: `<https://archon.mcdonaldhomelab.com>` ... -| **Access** | `http://localhost:3737` | `https://archon.mcdonaldhomelab.com` | +| **Access** | `http://localhost:3737` | `<https://archon.mcdonaldhomelab.com>` | ... -- Review Docker Compose logs: `docker-compose logs -f` +- Review Docker Compose logs: `docker compose logs -f`Also applies to: 31-33, 71-75, 88-89
60-66: Surround tables with blank lines.Satisfies MD058 (blanks-around-tables).
-## Files Overview - +## Files Overview + | File | Purpose | |------|---------| | `docker-compose.yml` | Developer localhost deployment | | `docker-compose-lan.yml` | LAN server override with Traefik labels | | `.env.lan.example` | Environment template for LAN server | | `docs/lan-migration/` | Detailed deployment documentation | - -## Key Differences: Dev vs LAN + +## Key Differences: Dev vs LAN + | Aspect | Developer | LAN Server | |--------|-----------|------------| | **Access** | `http://localhost:3737` | `https://archon.mcdonaldhomelab.com` | | **Security** | HTTP, direct ports | HTTPS, Traefik proxy | | **Network** | Port mappings | External proxy network | | **SSL** | None | Let's Encrypt via Traefik | | **Command** | `docker-compose up` | `docker-compose -f docker-compose-lan.yml up -d` |Also applies to: 69-76
traefik/docker-compose.yml (3)
28-43: Harden dashboard exposure and align domains.
- Replace placeholder domains with env vars.
- Add IP allowlist middleware (optional).
- Ensure
TRAEFIK_DASHBOARD_CREDENTIALSis htpasswd (bcrypt) format.- - traefik.http.routers.traefik.rule=Host(`traefik-dashboard.yourdomainname.com`) + - traefik.http.routers.traefik.rule=Host(`traefik-dashboard.${BASE_DOMAIN}`) ... - - traefik.http.routers.traefik-secure.rule=Host(`traefik-dashboard.yourdomainname.com`) + - traefik.http.routers.traefik-secure.rule=Host(`traefik-dashboard.${BASE_DOMAIN}`) ... - - traefik.http.routers.traefik-secure.tls.domains[0].main=yourdomainname.com - - traefik.http.routers.traefik-secure.tls.domains[0].sans=*.yourdomainname.com + - traefik.http.routers.traefik-secure.tls.domains[0].main=${BASE_DOMAIN} + - traefik.http.routers.traefik-secure.tls.domains[0].sans=*.${BASE_DOMAIN} + # (optional) restrict dashboard by IP; define in data/config.yml as ipwhitelist@file + # - traefik.http.routers.traefik-secure.middlewares=traefik-auth,ipwhitelist@filePlease confirm:
- BASE_DOMAIN is defined in .env (e.g., archon.mcdonaldhomelab.com).
- TRAEFIK_DASHBOARD_CREDENTIALS uses bcrypt:
user:$$2y$$...(htpasswd).
22-27: Ensure acme.json permissions are 600 and persisted.Volume is correct; enforce chmod 600 at provisioning (README now includes this). Consider setting owner to the Traefik user if you run as non-root.
5-15: Add basic healthcheck and least-privilege caps.Improves operability and reduces container privileges.
restart: unless-stopped security_opt: - no-new-privileges:true + cap_drop: + - ALL + cap_add: + - NET_BIND_SERVICE + healthcheck: + test: ["CMD-SHELL", "wget -qO- http://127.0.0.1:8080/ping || exit 1"] + interval: 30s + timeout: 5s + retries: 3Note: ensure
--ping=trueis enabled in traefik.yml for the /ping endpoint.docs/prd.md (5)
36-38: Fix bare URLs per markdownlint (MD034).Wrap URLs in angle brackets.
-**FR5:** The system shall expose services via HTTPS at https://archon.mcdonaldhomelab.com/ (frontend), /api/* (backend), and /mcp/* (MCP server) when in LAN mode +**FR5:** The system shall expose services via HTTPS at <https://archon.mcdonaldhomelab.com/> (frontend), `/api/*` (backend), and `/mcp/*` (MCP server) when in LAN mode ... -2. Services are accessible at https://archon.mcdonaldhomelab.com (frontend), /api (backend), /mcp (MCP) +2. Services are accessible at <https://archon.mcdonaldhomelab.com> (frontend), `/api` (backend), `/mcp` (MCP)Also applies to: 184-188
20-23: Add blank lines around the Change Log table (MD058).Improves markdown conformance.
### Change Log + | Date | Version | Description | Author | |------|---------|-------------|---------| | 2025-01-09 | 1.0 | Initial PRD creation based on Mary's analysis | John (PM) | +
289-291: Avoid emphasis as a heading (MD036).Use a proper heading or plain text.
-### UX Expert Prompt -*Not applicable for this infrastructure migration project - no UI changes required* +### UX Expert Prompt +Not applicable for this infrastructure migration project — no UI changes required.
46-60: Clarify enforceable resource limits in Compose.Compose “deploy.resources” support varies; ensure limits are actually enforced in your target runtime.
- If using Docker Compose (non‑Swarm), prefer the Compose Spec
deploy.resources.limits.memory/cpussupported by your CLI version or usemem_limit/cpusas a fallback.- Add a validation step in deploy script to print effective cgroup limits for each container (
docker inspect-> HostConfig.Memory/ NanoCpus).
28-43: Resolver and network naming consistency.Align PRD terms with compose/config:
- ACME resolver name “cloudflare” matches labels in traefik compose.
- External network named “proxy”.
archon-ui-main/src/services/mcpClientService.ts (3)
2-3: Remove unused getApiUrl import (align with lazy-getter guidance).
getApiUrlis imported but never used; this will trigger noUnusedLocals in strict builds and adds noise. Either remove it or wire it in via a lazy getter if you later move away from hard-coded relative paths.Apply:
-import { getApiUrl } from '@/config/api';
401-402: Harden Archon MCP URL: ensure '/mcp' suffix defensively.
getMCPConfig()currently returns a URL that ends with/mcp, but a future config change could omit it. Make the client robust.Apply:
- connection_config: { - url: config.url - }, + connection_config: { + url: config.url.endsWith('/mcp') + ? config.url + : `${config.url.replace(/\/+$/, '')}/mcp` + },Also applies to: 408-408
98-99: MakebaseUrlreadonly and confirm subpath deploymentFile: archon-ui-main/src/services/mcpClientService.ts: line 98
Apply:- private baseUrl = ''; + private readonly baseUrl = '';vite.config.ts has no
baseoverride (defaults to/), so all/api/...calls target the root and will break if the app is ever served under a subpath. Confirm whether you need subpath support and, if so, set Vite’sbaseor adjustbaseUrlaccordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.env.example(1 hunks).env.unified.lan(1 hunks).env.unified.local(1 hunks).gitignore(1 hunks)README-LAN.md(1 hunks)archon-ui-main/src/components/layouts/MainLayout.tsx(1 hunks)archon-ui-main/src/components/settings/FeaturesSection.tsx(2 hunks)archon-ui-main/src/services/credentialsService.ts(1 hunks)archon-ui-main/src/services/mcpClientService.ts(3 hunks)archon-ui-main/src/services/mcpService.ts(3 hunks)deploy-lan.sh(1 hunks)docs/prd.md(1 hunks)python/pyproject.toml(2 hunks)traefik/data/config.yml(1 hunks)traefik/data/traefik.yml(1 hunks)traefik/docker-compose.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
archon-ui-main/src/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint (standard React rules) for legacy UI code in /src/components
Files:
archon-ui-main/src/components/layouts/MainLayout.tsxarchon-ui-main/src/components/settings/FeaturesSection.tsx
archon-ui-main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use state naming conventions: is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures should be logged and not crash serving other clients
External API calls should retry with exponential backoff and ultimately fail with a clear, specific message
Include actionable context in frontend error logs/messages (what was attempted, relevant IDs/URLs)
Never return null to signal failure in async/data flows; throw errors with details
Use polling (HTTP) with provided hooks (usePolling, useDatabaseMutation, useProjectMutation); Socket.IO is removed
State naming: is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Persist theme choice in localStorage and respect Tailwind dark mode classes across components
Files:
archon-ui-main/src/components/layouts/MainLayout.tsxarchon-ui-main/src/services/credentialsService.tsarchon-ui-main/src/components/settings/FeaturesSection.tsxarchon-ui-main/src/services/mcpService.tsarchon-ui-main/src/services/mcpClientService.ts
archon-ui-main/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Legacy components live under src/components and may be used by pages; add related tests under archon-ui-main/test
Files:
archon-ui-main/src/components/layouts/MainLayout.tsxarchon-ui-main/src/components/settings/FeaturesSection.tsx
archon-ui-main/src/**/*.{ts,tsx,py}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid comment keywords like LEGACY, CHANGED, REMOVED; write comments that document current functionality only
Files:
archon-ui-main/src/components/layouts/MainLayout.tsxarchon-ui-main/src/services/credentialsService.tsarchon-ui-main/src/components/settings/FeaturesSection.tsxarchon-ui-main/src/services/mcpService.tsarchon-ui-main/src/services/mcpClientService.ts
archon-ui-main/src/services/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
archon-ui-main/src/services/**/*.ts: Frontend service method naming: get[Resource]sByProject(projectId), getResource, create/update/delete[Resource]
Use GET /api/projects/{id}/tasks (not getTasks) for project tasks
Files:
archon-ui-main/src/services/credentialsService.tsarchon-ui-main/src/services/mcpService.tsarchon-ui-main/src/services/mcpClientService.ts
🧠 Learnings (3)
📚 Learning: 2025-09-04T16:30:05.227Z
Learnt from: stevepresley
PR: coleam00/Archon#573
File: archon-ui-main/src/config/api.ts:15-25
Timestamp: 2025-09-04T16:30:05.227Z
Learning: Archon UI API config: Prefer lazy getters getApiFullUrl() and getWsUrl() over module-load constants to avoid SSR/test crashes. Avoid CommonJS exports patterns (Object.defineProperty(exports,…)) in ESM. Add typeof window guards with VITE_API_URL fallback inside getApiUrl()/getWebSocketUrl() when SSR safety is required.
Applied to files:
archon-ui-main/src/components/layouts/MainLayout.tsxarchon-ui-main/src/services/credentialsService.tsarchon-ui-main/src/services/mcpClientService.ts
📚 Learning: 2025-09-06T20:05:27.092Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-06T20:05:27.092Z
Learning: Applies to archon-ui-main/src/components/**/*.{ts,tsx} : Legacy components live under src/components and may be used by pages; add related tests under archon-ui-main/test
Applied to files:
archon-ui-main/src/components/layouts/MainLayout.tsx
📚 Learning: 2025-09-06T20:04:08.138Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T20:04:08.138Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in .env; optional LOGFIRE_TOKEN, LOG_LEVEL, ARCHON_*_PORT variables
Applied to files:
.env.unified.local.env.example.env.unified.lan
🧬 Code graph analysis (2)
archon-ui-main/src/services/mcpService.ts (1)
archon-ui-main/src/config/mcp.ts (1)
getMCPConfig(28-48)
archon-ui-main/src/services/mcpClientService.ts (2)
archon-ui-main/src/config/mcp.ts (1)
getMCPConfig(28-48)archon-ui-main/src/services/mcpService.ts (1)
MCPClientConfig(30-37)
🪛 markdownlint-cli2 (0.17.2)
docs/prd.md
20-20: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
36-36: Bare URL used
(MD034, no-bare-urls)
36-36: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
186-186: Bare URL used
(MD034, no-bare-urls)
289-289: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README-LAN.md
20-20: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
36-36: Bare URL used
(MD034, no-bare-urls)
36-36: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🪛 YAMLlint (1.37.1)
traefik/data/traefik.yml
[error] 35-35: no new line character at the end of file
(new-line-at-end-of-file)
traefik/data/config.yml
[error] 2-2: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
[warning] 94-94: wrong indentation: expected 10 but found 8
(indentation)
[warning] 101-101: wrong indentation: expected 10 but found 8
(indentation)
[error] 102-102: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Gitleaks (8.27.2)
.env.unified.local
[high] 35-35: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 37-37: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 dotenv-linter (3.3.0)
.env.unified.local
[warning] 42-42: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 47-47: [UnorderedKey] The BUILD_TARGET key should go before the DEPLOYMENT_MODE key
(UnorderedKey)
[warning] 54-54: [UnorderedKey] The BIND_IP key should go before the HOST key
(UnorderedKey)
[warning] 55-55: [UnorderedKey] The CORS_ORIGINS key should go before the HOST key
(UnorderedKey)
[warning] 56-56: [UnorderedKey] The API_BASE_URL key should go before the BIND_IP key
(UnorderedKey)
[warning] 65-65: [UnorderedKey] The VITE_MCP_PORT key should go before the VITE_MCP_PROTOCOL key
(UnorderedKey)
[warning] 71-71: [UnorderedKey] The ARCHON_MCP_PORT key should go before the ARCHON_SERVER_PORT key
(UnorderedKey)
[warning] 72-72: [UnorderedKey] The ARCHON_AGENTS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 81-81: [UnorderedKey] The COMPOSE_PROFILES key should go before the DOCKER_SOCKET key
(UnorderedKey)
[warning] 95-95: [UnorderedKey] The LOGFIRE_TOKEN key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 96-96: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 96-96: [UnorderedKey] The LOG_LEVEL key should go before the OPENAI_API_KEY key
(UnorderedKey)
.env.example
[warning] 30-30: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 31-31: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 72-72: [UnorderedKey] The ARCHON_MCP_PORT key should go before the ARCHON_SERVER_PORT key
(UnorderedKey)
[warning] 73-73: [UnorderedKey] The ARCHON_AGENTS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 75-75: [UnorderedKey] The ARCHON_DOCS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 82-82: [UnorderedKey] The VITE_MCP_PORT key should go before the VITE_MCP_PROTOCOL key
(UnorderedKey)
.env.unified.lan
[warning] 7-7: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 8-8: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 8-8: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
[warning] 8-8: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 14-14: [UnorderedKey] The BUILD_TARGET key should go before the DEPLOYMENT_MODE key
(UnorderedKey)
[warning] 21-21: [UnorderedKey] The BIND_IP key should go before the HOST key
(UnorderedKey)
[warning] 22-22: [UnorderedKey] The CORS_ORIGINS key should go before the HOST key
(UnorderedKey)
[warning] 23-23: [UnorderedKey] The API_BASE_URL key should go before the BIND_IP key
(UnorderedKey)
[warning] 32-32: [UnorderedKey] The VITE_MCP_PORT key should go before the VITE_MCP_PROTOCOL key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The ARCHON_MCP_PORT key should go before the ARCHON_SERVER_PORT key
(UnorderedKey)
[warning] 39-39: [UnorderedKey] The ARCHON_AGENTS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 49-49: [UnorderedKey] The COMPOSE_PROFILES key should go before the DOCKER_SOCKET key
(UnorderedKey)
[warning] 63-63: [UnorderedKey] The LOGFIRE_TOKEN key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 64-64: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 64-64: [UnorderedKey] The LOG_LEVEL key should go before the OPENAI_API_KEY key
(UnorderedKey)
🔇 Additional comments (7)
archon-ui-main/src/services/credentialsService.ts (1)
57-58: Relative API base path: good move for proxy/LAN setupsUsing relative URLs eliminates SSR/test pitfalls and avoids brittle env wiring. No further action required.
archon-ui-main/src/components/layouts/MainLayout.tsx (1)
48-51: Relative health endpoint: aligned with proxy strategySwitching to
/api/healthavoids brittle base URL logic and works across LAN/proxy deployments.archon-ui-main/src/services/mcpService.ts (2)
160-166: Good fallback to environment-driven MCP configSwitching to getMCPConfig() removes hard-coded defaults and aligns with deploy-time config. LGTM.
419-421: Correct: resolve MCP URL from central configUsing config.url keeps proxy/direct modes consistent across environments. 👍
.env.unified.local (1)
40-41: Good: using placeholders for live valuesThese should be filled locally; do not commit real keys.
.env.example (1)
77-83: MCP defaults look good for localhostValues align with getMCPConfig() usage. 👍
traefik/docker-compose.yml (1)
25-27: Mount configs read-only; leave ACME writable.Already RO for traefik.yml and config.yml; good. ACME must remain writable.
| # Local Development Configuration | ||
| # Copy to .env for local development | ||
|
|
||
| # ======================================== | ||
| # REQUIRED: Database Connection | ||
| # ======================================== | ||
| # Get your SUPABASE_URL from the Data API section of your Supabase project settings - | ||
| # https://supabase.com/dashboard/project/<your project ID>/settings/api | ||
| #LOCAL | ||
| #SUPABASE_URL=http://10.11.9.234:54321 | ||
| #WWW | ||
| #SUPABASE_URL=https://supabase.com/dashboard/project/ywqhytytvsugasmbpdfa/settings/api | ||
| #SUPABASE_URL=https://supabase.com/dashboard/project/ywqhytytvsugasmbpdfa/settings/api | ||
| #http://127.0.0.1:54321 | ||
| #http://127.0.0.1:54321 | ||
| SUPABASE_URL=supabaseLink | ||
|
|
||
| # ⚠️ CRITICAL: You MUST use the SERVICE ROLE key, NOT the Anon key! ⚠️ | ||
| # | ||
| # COMMON MISTAKE: Using the anon (public) key will cause ALL saves to fail with "permission denied"! | ||
| # | ||
| # How to get the CORRECT key: | ||
| # 1. Go to: https://supabase.com/dashboard/project/<your project ID>/settings/api | ||
| # 2. In the Settings menu, click on "API keys" | ||
| # 3. Find "Project API keys" section | ||
| # 4. You will see TWO keys - choose carefully: | ||
| # ❌ anon (public): WRONG - This is shorter, starts with "eyJhbGc..." and contains "anon" in the JWT | ||
| # ✅ service_role (secret): CORRECT - This is longer and contains "service_role" in the JWT | ||
| # | ||
| # The service_role key is typically much longer than the anon key. | ||
| # If you see errors like "Failed to save" or "Permission denied", you're using the wrong key! | ||
| # | ||
| # On the Supabase dashboard, it's labeled as "service_role" under "Project API keys" | ||
| #Anan Key local | ||
| #SUPABASE_SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZS1kZW1vIiwicm9sZSI6ImFub24iLCJleHAiOjE5ODM4MTI5OTZ9.CRXP1A7WOeoJeXxjNni43kdQwgnWNReilDMblYTn_I0 | ||
| #LOCAL_ | ||
| #SUPABASE_SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZS1kZW1vIiwicm9sZSI6InNlcnZpY2Vfcm9sZSIsImV4cCI6MTk4MzgxMjk5Nn0.EGIM96RAZx35lJzdJsyH-qQwv8Hdp7fsn3W0YpN81IU | ||
|
|
||
| #www | ||
| SUPABASE_SERVICE_KEY=supabaseKey | ||
|
|
||
|
|
||
| # ======================================== | ||
| # Deployment Mode: LOCAL | ||
| # ======================================== | ||
| DEPLOYMENT_MODE=local | ||
| BUILD_TARGET=development | ||
| NODE_ENV=development | ||
|
|
||
| # ======================================== | ||
| # Network Configuration | ||
| # ======================================== | ||
| HOST=localhost | ||
| BIND_IP=127.0.0.1 | ||
| CORS_ORIGINS=http://localhost:3737 | ||
| API_BASE_URL=http://localhost:8181 | ||
| VITE_API_URL=http://localhost:8181 | ||
|
|
||
| # ======================================== | ||
| # MCP Configuration | ||
| # ======================================== | ||
| VITE_MCP_HOST=localhost | ||
| VITE_MCP_PROTOCOL=http | ||
| VITE_MCP_USE_PROXY=false | ||
| VITE_MCP_PORT=8051 | ||
|
|
||
| # ======================================== | ||
| # Service Ports | ||
| # ======================================== | ||
| ARCHON_SERVER_PORT=8181 | ||
| ARCHON_MCP_PORT=8051 | ||
| ARCHON_AGENTS_PORT=8052 | ||
| ARCHON_UI_PORT=3737 | ||
|
|
||
| # ======================================== | ||
| # Development Features | ||
| # ======================================== | ||
| DOCKER_SOCKET=/var/run/docker.sock | ||
| PROD=false | ||
| # Use dev profile for development with volume mounts | ||
| COMPOSE_PROFILES=dev | ||
|
|
||
| # ======================================== | ||
| # Network Configuration (Local) | ||
| # ======================================== | ||
| # No external proxy network needed for local development | ||
| PROXY_EXTERNAL=false | ||
| PROXY_NETWORK=proxy-internal | ||
|
|
||
| # ======================================== | ||
| # Optional Features | ||
| # ======================================== | ||
| AGENTS_ENABLED=false | ||
| OPENAI_API_KEY= | ||
| LOGFIRE_TOKEN= | ||
| LOG_LEVEL=INFO No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Treat this as a template; ignore real envs
Commit .env.unified.local.example and add .env.unified.local to .gitignore to avoid future key leaks.
I can generate a filter-repo cleanup script if any real keys landed earlier.
🧰 Tools
🪛 Gitleaks (8.27.2)
[high] 35-35: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 37-37: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 dotenv-linter (3.3.0)
[warning] 42-42: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 47-47: [UnorderedKey] The BUILD_TARGET key should go before the DEPLOYMENT_MODE key
(UnorderedKey)
[warning] 54-54: [UnorderedKey] The BIND_IP key should go before the HOST key
(UnorderedKey)
[warning] 55-55: [UnorderedKey] The CORS_ORIGINS key should go before the HOST key
(UnorderedKey)
[warning] 56-56: [UnorderedKey] The API_BASE_URL key should go before the BIND_IP key
(UnorderedKey)
[warning] 65-65: [UnorderedKey] The VITE_MCP_PORT key should go before the VITE_MCP_PROTOCOL key
(UnorderedKey)
[warning] 71-71: [UnorderedKey] The ARCHON_MCP_PORT key should go before the ARCHON_SERVER_PORT key
(UnorderedKey)
[warning] 72-72: [UnorderedKey] The ARCHON_AGENTS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 81-81: [UnorderedKey] The COMPOSE_PROFILES key should go before the DOCKER_SOCKET key
(UnorderedKey)
[warning] 95-95: [UnorderedKey] The LOGFIRE_TOKEN key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 96-96: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 96-96: [UnorderedKey] The LOG_LEVEL key should go before the OPENAI_API_KEY key
(UnorderedKey)
🤖 Prompt for AI Agents
.in the file .env.unified.local lines 1-96, this file contains real-looking
secrets and should not be committed; create a sanitized template named
.env.unified.local.example (with placeholder values like SUPABASE_URL=your_url
and SUPABASE_SERVICE_KEY=your_service_key), remove sensitive values from this
file in the repo, add .env.unified.local to .gitignore, and if any real secrets
were previously committed run a history rewrite (e.g., git filter-repo or
provide the cleanup script) to purge those keys from the repo history before
merging.
| .bmad-core | ||
| lan-migration-docs/ No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ignore real env files and Traefik ACME storage to prevent secret leaks
Add ignores for concrete envs (.env.unified.*) and acme.json. This avoids accidentally committing real keys and cert state.
/logs/
.bmad-core
lan-migration-docs/
+#
+# Secrets and deployment artifacts
+.env.unified.local
+.env.unified.lan
+traefik/data/acme.json📝 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.
| .bmad-core | |
| lan-migration-docs/ | |
| /logs/ | |
| .bmad-core | |
| lan-migration-docs/ | |
| # Secrets and deployment artifacts | |
| .env.unified.local | |
| .env.unified.lan | |
| traefik/data/acme.json |
🤖 Prompt for AI Agents
In .gitignore around lines 8 to 9, the ignore list is missing concrete
environment files and Traefik ACME storage entries; update the .gitignore to add
patterns for .env.unified.* (or specific concrete env files used), and add
acme.json so real env files and Traefik certificate state are not committed;
apply succinct, generic patterns (e.g., .env.unified.* and acme.json) to prevent
accidental secrets and cert storage from being checked into source control.
| fetch('/api/projects/health').catch(() => null), | ||
| credentialsService.getCredential('DISCONNECT_SCREEN_ENABLED').catch(() => ({ value: 'true' })) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using null as a failure sentinel; add timeout and context
Returning null from the fetch promise hides error details and violates the “never return null” rule. Use a small wrapper with timeout and return undefined (or throw and handle outside) while logging context.
- fetch('/api/projects/health').catch(() => null),
+ (async () => {
+ const controller = new AbortController();
+ const t = setTimeout(() => controller.abort(), 5000);
+ try {
+ return await fetch('/api/projects/health', { signal: controller.signal });
+ } catch (e) {
+ console.warn('PROJECTS_HEALTH_REQUEST_FAILED', { url: '/api/projects/health' , error: e instanceof Error ? e.message : String(e) });
+ return undefined;
+ } finally {
+ clearTimeout(t);
+ }
+ })(),
@@
- url: '/api/projects/health'
+ url: '/api/projects/health'Also applies to: 61-62
🤖 Prompt for AI Agents
In archon-ui-main/src/components/settings/FeaturesSection.tsx around lines 42-43
and 61-62, the current .catch(() => null) pattern hides errors and violates the
"never return null" rule; replace these with a small fetch wrapper that applies
a short timeout, logs context and error details, and returns undefined (or
rethrows) on failure so callers can handle it explicitly; implement the wrapper
once in this module (or a shared util) and use it for both fetch calls, ensuring
you log which endpoint/credential failed and why before returning undefined.
| // Use relative URLs to work with proxy in all environments | ||
| private baseUrl = ''; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add fetch-with-retry + per-try timeout and richer errors (per guidelines).
Service calls don’t retry or time out. Add a small wrapper (retry idempotent methods only) and include actionable context (method, URL, status, body snippet).
Example (add to the class and incrementally adopt):
private async request<T>(
path: string,
init: RequestInit & { retries?: number; timeoutMs?: number } = {}
): Promise<T> {
const method = (init.method || 'GET').toUpperCase();
const retries = init.retries ?? (method === 'GET' ? 2 : 0);
const timeoutMs = init.timeoutMs ?? 10000;
const url = `${this.baseUrl}${path}`;
let attempt = 0, lastErr: unknown;
while (attempt <= retries) {
const ac = new AbortController();
const to = setTimeout(() => ac.abort(), timeoutMs);
try {
const res = await fetch(url, { ...init, signal: ac.signal, headers: { 'Accept': 'application/json', ...(init.headers||{}) } });
clearTimeout(to);
if (res.ok) return (await res.json()) as T;
// Try to extract server detail without crashing on non-JSON
let detail: string | undefined;
try { const j = await res.clone().json(); detail = (j as any)?.detail; } catch {}
const bodyText = detail ?? (await res.text()).slice(0, 500);
throw new Error(`[${method}] ${url} -> ${res.status} ${res.statusText}${bodyText ? `: ${bodyText}` : ''}`);
} catch (e) {
clearTimeout(to);
lastErr = e;
if (attempt === retries || method !== 'GET') break;
await new Promise(r => setTimeout(r, Math.min(2000, 300 * 2 ** attempt)));
attempt++;
}
}
throw lastErr instanceof Error ? lastErr : new Error(String(lastErr));
}
// Example adoption:
async getClients(): Promise<MCPClient[]> {
return this.request<MCPClient[]>('/api/mcp/clients/');
}| ### Prerequisites | ||
| - Docker & Docker Compose installed on LAN server | ||
| - Traefik proxy running with `proxy` network | ||
| - DNS: `archon.mcdonaldhomelab.com` → LAN server IP | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing pre-reqs and first-run steps (proxy network, ACME perms).
Without these, Traefik won't provision certs and services won't attach to the proxy network.
### Prerequisites
- Docker & Docker Compose installed on LAN server
- Traefik proxy running with `proxy` network
+- Cloudflare DNS API token configured (for Let's Encrypt DNS challenge)
- DNS: `archon.mcdonaldhomelab.com` → LAN server IP
### Deploy
```bash
# Clone repository
git clone <repo-url>
cd Archon
# Configure environment
cp .env.lan.example .env
# Edit .env with your Supabase credentials
+# One-time setup for Traefik/ACME and proxy network
+docker network create proxy || true
+mkdir -p traefik/data
+touch traefik/data/acme.json
+chmod 600 traefik/data/acme.json
+
# Deploy with LAN configuration (standalone file)
-docker-compose -f docker-compose-lan.yml up -d
+docker compose -f docker-compose-lan.yml up -dAlso applies to: 22-26
🤖 Prompt for AI Agents
In README-LAN.md around lines 7 to 11 (and similarly apply to lines 22 to 26),
the prerequisites and first-run steps are incomplete: add explicit one-time
setup instructions to create the Docker `proxy` network if missing, create the
`traefik/data` directory and an `acme.json` file with restrictive permissions
(chmod 600) so Traefik can store ACME certificates, and update the deploy
command to use `docker compose -f docker-compose-lan.yml up -d`; also include
the initial clone and .env copy/edit steps shown in the comment so users
configure Supabase credentials before bringing services up.
| ### Deploy | ||
| ```bash | ||
| # Clone repository | ||
| git clone <repo-url> |
There was a problem hiding this comment.
Replace placeholder repo URL.
Use the canonical repo to prevent copy/paste failures.
-git clone <repo-url>
+git clone https://github.com/coleam00/Archon.git📝 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.
| git clone <repo-url> | |
| git clone https://github.com/coleam00/Archon.git |
🤖 Prompt for AI Agents
In README-LAN.md around line 15, the git clone command uses a placeholder
"<repo-url>"; replace it with the repository's canonical clone URL (e.g., the
HTTPS or SSH URL from the project's main repo) so users can copy/paste and
successfully clone the project.
| api: | ||
| dashboard: true | ||
| debug: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden Traefik defaults for production
Disable debug and TLS skip-verify at the global level. Use a per-service serversTransport only if you truly need to connect to self-signed HTTPS backends.
api:
- dashboard: true
- debug: true
+ dashboard: true
+ debug: false
@@
-serversTransport:
- insecureSkipVerify: true
+# Prefer per-service serversTransport if necessary; keep global strict
+serversTransport:
+ insecureSkipVerify: falseAlso applies to: 15-15
🤖 Prompt for AI Agents
In traefik/data/traefik.yml around lines 1 to 3, the global config enables
dashboard debug and may implicitly allow insecure TLS handling; change defaults
for production by setting debug to false and ensure TLS skip-verify is disabled
globally (do not set insecureSkipVerify: true globally); instead, remove or set
any global insecure TLS options and, if you must connect to backends with
self-signed certs, create a per-service serversTransport with insecureSkipVerify
true and reference that transport on the specific service only.
| image: traefik:latest | ||
| container_name: traefik | ||
| restart: unless-stopped |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin Traefik image to a stable version.
Avoid “latest” to prevent surprise upgrades.
- image: traefik:latest
+ image: ${TRAEFIK_IMAGE:-traefik:v3}📝 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.
| image: traefik:latest | |
| container_name: traefik | |
| restart: unless-stopped | |
| image: ${TRAEFIK_IMAGE:-traefik:v3} | |
| container_name: traefik | |
| restart: unless-stopped |
🤖 Prompt for AI Agents
In traefik/docker-compose.yml around lines 3 to 5, the compose uses image:
traefik:latest which can cause unexpected upgrades; replace the "latest" tag
with a specific stable Traefik release (e.g. traefik:2.xx.x) to pin the image,
or reference an IMAGE_TAG env variable and document/update the .env file so
deployments use a controlled, auditable version and can be safely upgraded when
desired.
| environment: | ||
| # CF_DNS_API_TOKEN_FILE: /run/secrets/cf_api_token # note using _FILE for docker secrets | ||
| CF_DNS_API_TOKEN: ${CF_DNS_API_TOKEN} # if using .env | ||
| TRAEFIK_DASHBOARD_CREDENTIALS: ${TRAEFIK_DASHBOARD_CREDENTIALS} | ||
| # secrets: | ||
| # - cf_api_token | ||
| # env_file: .env # use .env | ||
| volumes: | ||
| - /etc/localtime:/etc/localtime:ro |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use Docker secrets for Cloudflare token; switch to _FILE env.
Prevents leaking CF API token via env inspection.
- environment:
- # CF_DNS_API_TOKEN_FILE: /run/secrets/cf_api_token # note using _FILE for docker secrets
- CF_DNS_API_TOKEN: ${CF_DNS_API_TOKEN} # if using .env
- TRAEFIK_DASHBOARD_CREDENTIALS: ${TRAEFIK_DASHBOARD_CREDENTIALS}
- # secrets:
- # - cf_api_token
+ environment:
+ CF_DNS_API_TOKEN_FILE: /run/secrets/cf_api_token
+ TRAEFIK_DASHBOARD_CREDENTIALS: ${TRAEFIK_DASHBOARD_CREDENTIALS}
+ secrets:
+ - cf_api_token
...
-#secrets:
-# cf_api_token:
-# file: ./cf_api_token.txt
+secrets:
+ cf_api_token:
+ file: ./cf_api_token.txtAlso applies to: 44-49
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.env.unified.local (1)
1-3: Commit a sanitized template and ignore real env filesTreat this as a template. Commit
.env.unified.local.examplewith placeholders and add.env.unified.localto.gitignoreto prevent future key leaks.Apply outside-this-file changes:
.gitignore
# local envs .env.unified.localDo you want a filter-repo cleanup script in case any real secrets ever landed?
🧹 Nitpick comments (11)
.env.unified.local (11)
9-16: Replace confusing commented examples with correct Supabase formatsThe commented dashboard URLs can mislead folks into pasting the wrong value. Provide canonical formats and remove duplicates.
-#LOCAL -#SUPABASE_URL=http://10.11.9.234:54321 -#WWW -#SUPABASE_URL=https://supabase.com/dashboard/project/ywqhytytvsugasmbpdfa/settings/api -#SUPABASE_URL=https://supabase.com/dashboard/project/ywqhytytvsugasmbpdfa/settings/api -#http://127.0.0.1:54321 -#http://127.0.0.1:54321 -SUPABASE_URL=supabaseLink +# Examples (pick one and replace placeholders): +# - Local Supabase: http://127.0.0.1:54321 +# - Hosted Supabase: https://<project-ref>.supabase.co +SUPABASE_URL=<your_supabase_url>
18-34: Tighten guidance on service_role vs anonGood callout. Suggest relying on the Supabase UI labels (“service_role”) rather than JWT string patterns; tokens are base64 and visuals aren’t reliable. Also reiterate: never commit real keys.
38-39: Remove extra blank line (dotenv-linter)-
42-45: Order keys to satisfy dotenv-linter (build/deploy block)-DEPLOYMENT_MODE=local -BUILD_TARGET=development +BUILD_TARGET=development +DEPLOYMENT_MODE=local NODE_ENV=development
49-54: Order network keys and consider multiple local origins-HOST=localhost -BIND_IP=127.0.0.1 -CORS_ORIGINS=http://localhost:3737 -API_BASE_URL=http://localhost:8181 +API_BASE_URL=http://localhost:8181 +BIND_IP=127.0.0.1 +CORS_ORIGINS=http://localhost:3737 +HOST=localhost VITE_API_URL=http://localhost:8181If the server supports CSV CORS, consider adding http://127.0.0.1:3737 as well. Confirm server parsing supports comma-separated origins.
58-62: Order MCP vars (dotenv-linter)-VITE_MCP_PROTOCOL=http -VITE_MCP_USE_PROXY=false -VITE_MCP_PORT=8051 +VITE_MCP_PORT=8051 +VITE_MCP_PROTOCOL=http +VITE_MCP_USE_PROXY=false
66-70: Order service ports (dotenv-linter)-ARCHON_SERVER_PORT=8181 -ARCHON_MCP_PORT=8051 -ARCHON_AGENTS_PORT=8052 +ARCHON_AGENTS_PORT=8052 +ARCHON_MCP_PORT=8051 +ARCHON_SERVER_PORT=8181 ARCHON_UI_PORT=3737
74-78: Order dev features (dotenv-linter)-DOCKER_SOCKET=/var/run/docker.sock -PROD=false -# Use dev profile for development with volume mounts -COMPOSE_PROFILES=dev +COMPOSE_PROFILES=dev +# Use dev profile for development with volume mounts +DOCKER_SOCKET=/var/run/docker.sock +PROD=false
89-92: Order optional keys (dotenv-linter)-OPENAI_API_KEY= -LOGFIRE_TOKEN= -LOG_LEVEL=INFO +LOGFIRE_TOKEN= +LOG_LEVEL=INFO +OPENAI_API_KEY=
74-75: Docker socket exposure: double-check necessityMounting the Docker socket grants root-equivalent control over the host. If not required for local flows, avoid mounting or use a rootless shim. At minimum, document the risk and scope it to dev only.
16-16: Validate required envs at startupServer should hard-fail with a clear message if SUPABASE_URL or SUPABASE_SERVICE_KEY are missing/blank. Same for API_BASE_URL when applicable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env.unified.local(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-06T20:04:08.138Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T20:04:08.138Z
Learning: Applies to .env : Require SUPABASE_URL and SUPABASE_SERVICE_KEY in .env; optional LOGFIRE_TOKEN, LOG_LEVEL, ARCHON_*_PORT variables
Applied to files:
.env.unified.local
🪛 dotenv-linter (3.3.0)
.env.unified.local
[warning] 38-38: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 43-43: [UnorderedKey] The BUILD_TARGET key should go before the DEPLOYMENT_MODE key
(UnorderedKey)
[warning] 50-50: [UnorderedKey] The BIND_IP key should go before the HOST key
(UnorderedKey)
[warning] 51-51: [UnorderedKey] The CORS_ORIGINS key should go before the HOST key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The API_BASE_URL key should go before the BIND_IP key
(UnorderedKey)
[warning] 61-61: [UnorderedKey] The VITE_MCP_PORT key should go before the VITE_MCP_PROTOCOL key
(UnorderedKey)
[warning] 67-67: [UnorderedKey] The ARCHON_MCP_PORT key should go before the ARCHON_SERVER_PORT key
(UnorderedKey)
[warning] 68-68: [UnorderedKey] The ARCHON_AGENTS_PORT key should go before the ARCHON_MCP_PORT key
(UnorderedKey)
[warning] 77-77: [UnorderedKey] The COMPOSE_PROFILES key should go before the DOCKER_SOCKET key
(UnorderedKey)
[warning] 91-91: [UnorderedKey] The LOGFIRE_TOKEN key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 92-92: [UnorderedKey] The LOG_LEVEL key should go before the OPENAI_API_KEY key
(UnorderedKey)
🔇 Additional comments (2)
.env.unified.local (2)
2-2: Clarify “Copy to .env” instructionIf docker-compose expects
.env.unified.local, update this header to avoid confusion, or document the exact--env-fileusage.
49-54: CORS correctness across UI/API during LANEnsure CORS_ORIGINS includes the exact scheme/host you advertise for LAN (e.g., http://:3737). If LAN docs exist, align examples here to reduce setup friction.
- Include comprehensive Traefik setup and configuration guide - Document reverse proxy setup for Archon deployment 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
README-TRAEFIK.md (10)
95-97: Pin image to a patch version and digest.Avoid floating tags for supply-chain stability.
- image: traefik:v3.0 + image: traefik:v3.0.x@sha256:<digest> # pin exact patch + digest
27-38: Scope/rotate Cloudflare token; avoid “No expiry.”Prefer limited scopes (Zone.DNS:Edit is required; Zone:Read optional) and set an expiry with rotation reminders.
165-170: Prefer “docker compose” (plugin) over legacy “docker-compose.”-docker-compose up -d +docker compose up -d
205-210: Reset acme.json safely; preserve perms.-echo "" > data/acme.json +truncate -s 0 data/acme.json && chmod 600 data/acme.json
177-185: DNS record type: favor A/AAAA over CNAME for local hostnames.For LAN-only hosts, an A/AAAA to the Docker host IP is simpler and avoids chained local resolution issues.
329-333: Clarify resolver note.“External DNS resolvers are required” isn’t strictly true for DNS‑01. It’s sufficient that ACME validates public DNS; the
resolvershere mainly influence Traefik’s propagation checks.
338-338: Fix markdownlint MD034 (bare URL).-https://www.youtube.com/watch?v=n1vOfdz5Nm8&t=1571s +<https://www.youtube.com/watch?v=n1vOfdz5Nm8&t=1571s>
45-84: Production defaults: disable api.debug and set TLS minima.api: dashboard: true - debug: true + debug: false @@ certificatesResolvers: @@ +# Optional: enforce TLS options +# tls: +# options: +# default: +# minVersion: VersionTLS12 +# sniStrict: true
91-105: Add healthcheck and read-only FS for Traefik container (except acme.json).traefik: @@ restart: unless-stopped + healthcheck: + test: ["CMD", "traefik", "healthcheck", "--ping"] + interval: 30s + timeout: 5s + retries: 3 security_opt: - no-new-privileges:true + read_only: true networks: - proxy @@ - ./data/acme.json:/acme.json + tmpfs: + - /tmpNote: read-only FS is compatible because ACME writes only to
/acme.json.
121-123: Middleware naming consistency.In Traefik v2/v3, middleware type key is
redirectscheme(ok), but for readability prefer consistent casing across labels and reuse a single global redirect middleware name (e.g.,redirect-to-https) to avoid duplicates across stacks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README-TRAEFIK.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README-TRAEFIK.md
338-338: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
README-TRAEFIK.md (1)
105-117: Confirmed CF_DNS_API_TOKEN_FILE support and Compose secrets mount
Traefik v3’s ACME/lego integration accepts CF_DNS_API_TOKEN_FILE for Cloudflare tokens, and Docker Compose v2 (standalone, non-Swarm) bind-mounts declared secrets under /run/secrets.
| serversTransport: | ||
| insecureSkipVerify: true | ||
|
|
There was a problem hiding this comment.
Remove insecureSkipVerify=true (disables TLS verification globally).
This weakens upstream TLS for every service. Only disable per-service if absolutely required.
Apply:
-serversTransport:
- insecureSkipVerify: true
+# serversTransport:
+# # Avoid disabling TLS verification globally. If needed, set per-router/service only.📝 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.
| serversTransport: | |
| insecureSkipVerify: true | |
| # serversTransport: | |
| # # Avoid disabling TLS verification globally. If needed, set per-router/service only. |
🤖 Prompt for AI Agents
In README-TRAEFIK.md around lines 60 to 62, the serversTransport setting uses
insecureSkipVerify: true which disables TLS verification globally; remove this
setting or set it to false and instead configure per-service TLS verification
exceptions only where absolutely necessary. Update the examples and instructions
to recommend omitting insecureSkipVerify entirely, document how to configure
per-backend TLS options (e.g., per-service cert/CA or specific skip flags) and
add a note warning against global disabling of TLS verification.
| image: traefik:v3.0 | ||
| container_name: traefik | ||
| restart: unless-stopped | ||
| security_opt: | ||
| - no-new-privileges:true | ||
| networks: | ||
| - proxy | ||
| ports: | ||
| - "80:80" | ||
| - "443:443" | ||
| environment: |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Check Traefik version currency and CVEs before merging.
🌐 Web query:
What is the latest stable Traefik 3.x release as of September 2025, and are there known CVEs affecting v3.0 images?
💡 Result:
Latest stable 3.x (as of 2025‑09‑10): Traefik v3.5.1 (released 2025‑08‑27). [1]
Known CVEs affecting v3.0 images:
- CVE-2025-54386 (path‑traversal in plugin installation) affects 3.0.0 through 3.4.4 (fixed in 3.4.5 / 3.5.0). [2]
- CVE-2025-32431 (path /../ routing bypass) and earlier high‑severity advisories (e.g., CVE-2024-45410) also affected older 3.0.x releases — 3.0 was within the affected range for those advisories and is no longer on active support. [3][4]
Recommendation: upgrade off 3.0 to a fixed, supported 3.4.5+ / 3.5.x release; check your specific deployed tag and apply vendor advisories.
Sources: GitHub releases (v3.5.1), NVD CVE-2025-54386, Traefik/CERT advisories. [1][2][3][4]
Upgrade Traefik image to a patched 3.x release
- README-TRAEFIK.md (lines 95–105): change
image: traefik:v3.0to at leasttraefik:v3.4.5or preferablytraefik:v3.5.1to address CVE-2025-54386 and CVE-2025-32431.
🤖 Prompt for AI Agents
In README-TRAEFIK.md around lines 95 to 105, the Traefik image is pinned to
traefik:v3.0 which is vulnerable; update the image reference to at least
traefik:v3.4.5 (preferably traefik:v3.5.1) to include the security patches for
CVE-2025-54386 and CVE-2025-32431, then save the file and, if applicable, run
docker-compose pull && docker-compose up -d or note the updated image tag in any
deployment manifests so the upgraded image is deployed.
| - "traefik.enable=true" | ||
| - "traefik.http.routers.traefik.entrypoints=http" | ||
| - "traefik.http.routers.traefik.rule=Host(`traefik-dashboard.local.yourdomain.com`)" | ||
| - "traefik.http.middlewares.traefik-auth.basicauth.users=${TRAEFIK_DASHBOARD_CREDENTIALS}" | ||
| - "traefik.http.middlewares.traefik-https-redirect.redirectscheme.scheme=https" | ||
| - "traefik.http.middlewares.sslheader.headers.customrequestheaders.X-Forwarded-Proto=https" | ||
| - "traefik.http.routers.traefik.middlewares=traefik-https-redirect" | ||
| - "traefik.http.routers.traefik-secure.entrypoints=https" | ||
| - "traefik.http.routers.traefik-secure.rule=Host(`traefik-dashboard.local.yourdomain.com`)" | ||
| - "traefik.http.routers.traefik-secure.middlewares=traefik-auth" | ||
| - "traefik.http.routers.traefik-secure.tls=true" | ||
| - "traefik.http.routers.traefik-secure.tls.certresolver=cloudflare" | ||
| - "traefik.http.routers.traefik-secure.tls.domains[0].main=local.yourdomain.com" | ||
| - "traefik.http.routers.traefik-secure.tls.domains[0].sans=*.local.yourdomain.com" | ||
| - "traefik.http.routers.traefik-secure.service=api@internal" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden dashboard exposure (add IP allowlist + rate limit).
Basic auth alone is weak. Add IP allowlist (and optionally rate limit) to dashboard routers.
- "traefik.http.routers.traefik-secure.middlewares=traefik-auth"
+ - "traefik.http.middlewares.dashboard-ipwhitelist.ipwhitelist.sourcerange=10.0.0.0/8,192.168.0.0/16,172.16.0.0/12"
+ - "traefik.http.middlewares.dashboard-ratelimit.ratelimit.average=50"
+ - "traefik.http.middlewares.dashboard-ratelimit.burst=100"
- - "traefik.http.routers.traefik-secure.middlewares=traefik-auth"
+ - "traefik.http.routers.traefik-secure.middlewares=traefik-auth,dashboard-ipwhitelist,dashboard-ratelimit"Also consider setting api.debug: false in production.
📝 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.
| - "traefik.enable=true" | |
| - "traefik.http.routers.traefik.entrypoints=http" | |
| - "traefik.http.routers.traefik.rule=Host(`traefik-dashboard.local.yourdomain.com`)" | |
| - "traefik.http.middlewares.traefik-auth.basicauth.users=${TRAEFIK_DASHBOARD_CREDENTIALS}" | |
| - "traefik.http.middlewares.traefik-https-redirect.redirectscheme.scheme=https" | |
| - "traefik.http.middlewares.sslheader.headers.customrequestheaders.X-Forwarded-Proto=https" | |
| - "traefik.http.routers.traefik.middlewares=traefik-https-redirect" | |
| - "traefik.http.routers.traefik-secure.entrypoints=https" | |
| - "traefik.http.routers.traefik-secure.rule=Host(`traefik-dashboard.local.yourdomain.com`)" | |
| - "traefik.http.routers.traefik-secure.middlewares=traefik-auth" | |
| - "traefik.http.routers.traefik-secure.tls=true" | |
| - "traefik.http.routers.traefik-secure.tls.certresolver=cloudflare" | |
| - "traefik.http.routers.traefik-secure.tls.domains[0].main=local.yourdomain.com" | |
| - "traefik.http.routers.traefik-secure.tls.domains[0].sans=*.local.yourdomain.com" | |
| - "traefik.http.routers.traefik-secure.service=api@internal" | |
| - "traefik.enable=true" | |
| - "traefik.http.routers.traefik.entrypoints=http" | |
| - "traefik.http.routers.traefik.rule=Host(`traefik-dashboard.local.yourdomain.com`)" | |
| - "traefik.http.middlewares.traefik-auth.basicauth.users=${TRAEFIK_DASHBOARD_CREDENTIALS}" | |
| - "traefik.http.middlewares.traefik-https-redirect.redirectscheme.scheme=https" | |
| - "traefik.http.middlewares.sslheader.headers.customrequestheaders.X-Forwarded-Proto=https" | |
| - "traefik.http.routers.traefik.middlewares=traefik-https-redirect" | |
| - "traefik.http.routers.traefik-secure.entrypoints=https" | |
| - "traefik.http.routers.traefik-secure.rule=Host(`traefik-dashboard.local.yourdomain.com`)" | |
| - "traefik.http.middlewares.dashboard-ipwhitelist.ipwhitelist.sourcerange=10.0.0.0/8,192.168.0.0/16,172.16.0.0/12" | |
| - "traefik.http.middlewares.dashboard-ratelimit.ratelimit.average=50" | |
| - "traefik.http.middlewares.dashboard-ratelimit.burst=100" | |
| - "traefik.http.routers.traefik-secure.middlewares=traefik-auth,dashboard-ipwhitelist,dashboard-ratelimit" | |
| - "traefik.http.routers.traefik-secure.tls=true" | |
| - "traefik.http.routers.traefik-secure.tls.certresolver=cloudflare" | |
| - "traefik.http.routers.traefik-secure.tls.domains[0].main=local.yourdomain.com" | |
| - "traefik.http.routers.traefik-secure.tls.domains[0].sans=*.local.yourdomain.com" | |
| - "traefik.http.routers.traefik-secure.service=api@internal" |
🤖 Prompt for AI Agents
In README-TRAEFIK.md around lines 118 to 133, the Traefik dashboard is protected
only by basic auth which is weak; add an IP allowlist middleware and
(optionally) a rate-limit middleware and attach them to both the HTTP and HTTPS
dashboard routers, and set api.debug: false for production. Create a middleware
like traefik-dashboard-allowlist with ipAllowlist (or IPStrategy) containing
your trusted IPs, create a traefik-dashboard-ratelimit middleware with basic
rateLimit settings if desired, and include those middleware names in the
"traefik.http.routers.traefik.middlewares" and
"traefik.http.routers.traefik-secure.middlewares" lists (in addition to
traefik-auth); finally ensure the Traefik static config has api.debug: false for
production deployments.
| ```bash | ||
| # Install htpasswd (Ubuntu) | ||
| sudo apt update && sudo apt install apache2-utils | ||
|
|
||
| # Generate credentials (replace 'admin' with your username) | ||
| htpasswd -nb admin password | ||
|
|
||
| # Copy the output to .env file | ||
| echo "TRAEFIK_DASHBOARD_CREDENTIALS=admin:\$2y\$10\$..." > .env | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use strong password and keep secrets out of VCS.
- Don’t use “password” in examples.
- Add
.envandcf_api_token.txtto.gitignore. - Restrict token file perms.
Example:
openssl rand -base64 24 | tr -d '\n' # generate strong pass
htpasswd -nb admin '<generated>'
chmod 600 cf_api_token.txt
printf "TRAEFIK_DASHBOARD_CREDENTIALS=admin:\$2y\$...\n" > .env.gitignore additions (repo root):
+.env
+cf_api_token.txt
+docker-volumes/**/acme.json- Resolve merge conflicts between deployment configurations and beta features - Keep both deployment improvements and new beta architecture - Maintain compatibility with both local and remote deployment modes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update README.md with docker-compose.unified.yml instructions - Update README-LAN.md with simplified deployment steps - Clarify environment configuration and deployment commands 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Pull Request
Summary
Changes Made
Type of Change
Affected Services
Testing
Test Evidence
Checklist
Breaking Changes
Additional Notes
Summary by CodeRabbit
New Features
Documentation
Chores