feat: Enable LAN access for Web UI server #31
Conversation
Changes: - Update CORS to allow all origins for LAN accessibility - Disable localhost-only security middleware - Bind uvicorn to 0.0.0.0 instead of 127.0.0.1 in server/main.py - Update start_ui.py to use 0.0.0.0 for socket binding - Update server startup messages to reflect new binding address This allows the AutoCoder Web UI to be accessed from other machines on the local network, enabling remote development workflows. Note: This reduces security - only use on trusted networks. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughServer now binds to 0.0.0.0 with CORS allowing all origins. Added a feature DB repair tool and POST /repair endpoint for duplicate removal and ID compaction. Introduced safe_print with retry and an autonomous completion check. UI gained dark-mode theming and many style updates. Project registration CLI added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router as Features Router
participant DB as Database
participant Session as DB Session
Client->>Router: POST /repair (project_name)
Router->>Session: open/validate project DB session
Router->>DB: SELECT features GROUP BY name (identify duplicates)
Router->>Router: determine duplicates (keep lowest ID)
Router->>DB: DELETE duplicate rows
DB-->>Router: duplicates_removed
Router->>DB: SELECT max(id) and check sequence
alt IDs non-sequential
Router->>DB: execute raw SQL to remap IDs 1..N
Router->>DB: update feature priorities to match new IDs
DB-->>Router: ids_compacted, new_max_id
end
Router->>Session: commit changes
Session-->>Router: commit success
Router-->>Client: return summary JSON (duplicates_removed, ids_compacted, old_max_id, new_max_id, total_features)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @server/main.py:
- Around line 66-80: Reinstate the localhost middleware (the commented
require_localhost function) but gate its registration on a configuration flag
(e.g., LAN_ACCESS_ENABLED / enable_lan_access) so the default behavior remains
localhost-only; update startup logic to conditionally call
app.middleware("http")(require_localhost) or skip it when the flag is true, add
a runtime/UI-exposed warning flag (e.g., is_lan_mode) that the frontend can read
so it can show a warning banner when accessed from non-localhost, and add a
brief README note about the security implications and how to enable LAN access.
- Around line 56-63: The CORS config uses app.add_middleware with CORSMiddleware
and sets allow_origins=["*"] together with allow_credentials=True, which
violates the CORS spec; either remove allow_credentials=True if you don’t need
cookies/auth headers, or replace the wildcard by an explicit list of allowed
origins (set allow_origins to a list of origin strings, e.g., from config/env)
while keeping allow_credentials=True so browsers accept credentialed requests.
🧹 Nitpick comments (2)
start_ui.py (1)
161-162: Display actual IP address instead of0.0.0.0in user-facing messages.Printing
http://0.0.0.0:{port}is misleading for users—0.0.0.0is a bind address, not a valid URL to access the server. Consider displaying eitherlocalhostfor local access or the machine's actual LAN IP address for remote access.♻️ Suggested improvement
import socket def get_local_ip() -> str: """Get the machine's local IP address for LAN access.""" try: # Create a socket to determine the local IP s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) s.connect(("8.8.8.8", 80)) ip = s.getsockname()[0] s.close() return ip except Exception: return "127.0.0.1"Then update the print statements:
- print(f" - FastAPI backend: http://0.0.0.0:{port}") + print(f" - FastAPI backend: http://127.0.0.1:{port} (LAN: http://{get_local_ip()}:{port})")- print(f"\n Starting server at http://0.0.0.0:{port}") + local_ip = get_local_ip() + print(f"\n Starting server at http://127.0.0.1:{port}") + print(f" LAN access: http://{local_ip}:{port}")- print(f" Server running at http://0.0.0.0:{port}") + print(f" Server running at http://127.0.0.1:{port}") + print(f" LAN access: http://{get_local_ip()}:{port}")Also applies to: 188-188, 283-283
ui/tsconfig.tsbuildinfo (1)
1-1: Consider addingtsconfig.tsbuildinfoto.gitignore.This file is auto-generated by the TypeScript compiler for incremental builds. It's typically excluded from version control as it's machine-specific and can cause unnecessary merge conflicts.
Add to
.gitignore:ui/tsconfig.tsbuildinfo
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
server/main.pystart_ui.pyui/tsconfig.tsbuildinfo
🔇 Additional comments (3)
start_ui.py (2)
47-47: Consider the implications of binding port check to all interfaces.Binding to
0.0.0.0for port discovery checks if the port is available on all network interfaces. This is correct for LAN access, but note that this means a port already bound on any interface (not just localhost) will be skipped.
165-171: LAN-accessible server binding looks correct.The uvicorn host binding to
0.0.0.0in both development and production modes correctly enables LAN access. The browser still opens to127.0.0.1locally (lines 257, 280), which is appropriate behavior.Also applies to: 190-195
server/main.py (1)
174-181: Main entry point binding is consistent with LAN access goal.The host binding change to
0.0.0.0in the__main__block aligns with the PR objective. Note thatreload=Truein production could be a security concern on LAN—consider making this configurable or removing it for production use.
| # ============================================================================ | ||
| # Security Middleware | ||
| # Security Middleware (disabled for LAN access) | ||
| # ============================================================================ | ||
|
|
||
| @app.middleware("http") | ||
| async def require_localhost(request: Request, call_next): | ||
| """Only allow requests from localhost.""" | ||
| client_host = request.client.host if request.client else None | ||
|
|
||
| # Allow localhost connections | ||
| if client_host not in ("127.0.0.1", "::1", "localhost", None): | ||
| raise HTTPException(status_code=403, detail="Localhost access only") | ||
|
|
||
| return await call_next(request) | ||
| # NOTE: Localhost restriction removed to allow LAN access | ||
| # @app.middleware("http") | ||
| # async def require_localhost(request: Request, call_next): | ||
| # """Only allow requests from localhost.""" | ||
| # client_host = request.client.host if request.client else None | ||
| # | ||
| # # Allow localhost connections | ||
| # if client_host not in ("127.0.0.1", "::1", "localhost", None): | ||
| # raise HTTPException(status_code=403, detail="Localhost access only") | ||
| # | ||
| # return await call_next(request) |
There was a problem hiding this comment.
Acknowledge security implications of removing localhost restriction.
Commenting out the localhost middleware is intentional for LAN access, but this exposes all API endpoints and WebSocket connections to any machine on the network. Consider:
- Adding a configuration flag to toggle LAN access (defaulting to localhost-only)
- Adding a warning banner in the UI when accessed from non-localhost
- Documenting the security implications in the README
This is acceptable for trusted networks but should be an opt-in feature rather than the default.
♻️ Suggested: Make LAN access configurable
+import os
+
+# Configuration
+ALLOW_LAN_ACCESS = os.getenv("AUTOCODER_ALLOW_LAN", "false").lower() == "true"
+
# ============================================================================
-# Security Middleware (disabled for LAN access)
+# Security Middleware
# ============================================================================
-# NOTE: Localhost restriction removed to allow LAN access
-# @app.middleware("http")
-# async def require_localhost(request: Request, call_next):
-# ...
+if not ALLOW_LAN_ACCESS:
+ @app.middleware("http")
+ async def require_localhost(request: Request, call_next):
+ """Only allow requests from localhost."""
+ client_host = request.client.host if request.client else None
+ if client_host not in ("127.0.0.1", "::1", "localhost", None):
+ raise HTTPException(status_code=403, detail="Localhost access only. Set AUTOCODER_ALLOW_LAN=true to enable LAN access.")
+ return await call_next(request)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @server/main.py around lines 66 - 80, Reinstate the localhost middleware (the
commented require_localhost function) but gate its registration on a
configuration flag (e.g., LAN_ACCESS_ENABLED / enable_lan_access) so the default
behavior remains localhost-only; update startup logic to conditionally call
app.middleware("http")(require_localhost) or skip it when the flag is true, add
a runtime/UI-exposed warning flag (e.g., is_lan_mode) that the frontend can read
so it can show a warning banner when accessed from non-localhost, and add a
brief README note about the security implications and how to enable LAN access.
- Add safe_print() with retry logic to handle EAGAIN errors when stdout
pipe buffer is full, preventing session crashes
- Add duplicate detection to feature_create_bulk to skip existing features
- Add feature_db_repair MCP tool and /api/projects/{name}/features/repair
endpoint to remove duplicates and compact IDs
- Fix notification flicker in AgentThought component by stabilizing
visibility state transitions
- Fix React key warning in KanbanColumn by using composite key
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcp_server/feature_mcp.py (1)
442-518:feature_db_repair()contains critical ORM bug: accessing ORM objects after commit with ID rewrite will fail.
Critical correctness bug: The sessionmaker at api/database.py:93 uses
sessionmaker(autocommit=False, autoflush=False, bind=engine)with no explicitexpire_on_commitparameter. SQLAlchemy's default isexpire_on_commit=True, which meanssession.commit()at line 495 expires all loaded ORM objects. When you then accessfeature.idin the loop at line 500, SQLAlchemy automatically refreshes the object using its old primary key—which no longer exists (it's been negated). This will raise an error at runtime.SQL safety: Lines 474 and 500 use f-string interpolation to build SQL instead of parameterized queries. Use expanding bind params with
bindparam(..., expanding=True)instead.Operational risk: Rewriting
features.id(the primary key) can break external caches or FK relationships if they exist now or in the future.The suggested fix captures IDs as plain integers before the commit and uses parameterized SQL:
Safer approach: avoid ORM refresh issue + parameterize SQL
def feature_db_repair() -> str: session = get_session() try: from sqlalchemy import text + from sqlalchemy import bindparam result = session.execute(text(duplicates_query)) duplicate_ids = [row[0] for row in result.fetchall()] if duplicate_ids: - session.execute( - text(f"DELETE FROM features WHERE id IN ({','.join(map(str, duplicate_ids))})") - ) + session.execute( + text("DELETE FROM features WHERE id IN :ids") + .bindparams(bindparam("ids", expanding=True)), + {"ids": duplicate_ids}, + ) - all_features = session.query(Feature).order_by(Feature.priority.asc(), Feature.id.asc()).all() - old_max_id = max(f.id for f in all_features) if all_features else 0 - total_features = len(all_features) + ordered_ids = [ + row[0] + for row in session.execute( + text("SELECT id FROM features ORDER BY priority ASC, id ASC") + ).fetchall() + ] + old_max_id = max(ordered_ids) if ordered_ids else 0 + total_features = len(ordered_ids) - actual_ids = set(f.id for f in all_features) + actual_ids = set(ordered_ids) - if needs_compaction and all_features: + if needs_compaction and ordered_ids: session.execute(text("UPDATE features SET id = -id")) - for new_id, feature in enumerate(all_features, start=1): + for new_id, old_id in enumerate(ordered_ids, start=1): session.execute( - text(f"UPDATE features SET id = {new_id}, priority = {new_id} WHERE id = {-feature.id}") + text("UPDATE features SET id = :new_id, priority = :new_id WHERE id = :neg_old_id"), + {"new_id": new_id, "neg_old_id": -old_id}, )
🧹 Nitpick comments (4)
ui/src/components/KanbanColumn.tsx (1)
48-60: Avoidindexin React keys; prefer a stable unique feature identifier.
key={${feature.id}-${index}}will “work”, but it can cause remounts (lost internal state / janky animations) when cards move between columns or the list is reordered. Better to keep a stable key (e.g.,feature.id) and fix the underlying duplicate-id issue in the data layer (which this PR already seems to be addressing via dedupe/repair tooling).Proposed change
- <div - key={`${feature.id}-${index}`} + <div + key={feature.id} className="animate-slide-in" style={{ animationDelay: `${index * 50}ms` }} >ui/src/components/AgentThought.tsx (1)
52-60: Initial “thought” display is delayed ~150ms and timestamp parsing can hide content unexpectedly.
- On first render with a valid
thought,isVisiblecan become true but the component still returnsnulluntildisplayedThoughtis set after the 150ms timeout.new Date(logs[...].timestamp).getTime()can produceNaN, which makes the paused-mode check fail.A small improvement is to initialize
displayedThoughtfromthoughton mount (skip fade), and guardlastLogTimestampagainstNaN.Also applies to: 76-88, 105-106
agent.py (1)
23-46: Avoid blockingasynciowithtime.sleep()insafe_print()backoff path.Since
safe_print()is called fromasync def run_agent_session()(Line 60+), hitting the retry path will block the event loop. Consider either making anasync_safe_print()that usesawait asyncio.sleep(...)(and await it at call sites), or offloading printing to a thread.Proposed direction (introduce async variant for async call sites)
+async def async_safe_print(*args, **kwargs) -> None: + max_retries = 5 + for attempt in range(max_retries): + try: + print(*args, **kwargs) + return + except BlockingIOError: + if attempt < max_retries - 1: + await asyncio.sleep(0.1 * (attempt + 1)) + else: + kwargs = dict(kwargs) + kwargs.pop("flush", None) + try: + print(*args, **kwargs) + except Exception: + returnmcp_server/feature_mcp.py (1)
359-440: Duplicate detection by name is case-sensitive and incomplete.The Feature model has no database-level unique constraint on
name, and onlyfeature_create_bulk()performs duplicate detection. The single feature creation endpoint (server/routers/features.py:151) does not check for duplicates at all, so identical or near-identical names (e.g., "Feature", "feature", "Feature ") can be created with different casing or whitespace.Recommend normalizing names (e.g.,
name.strip().casefold()) consistently in both bulk and single creation paths to prevent accidental near-duplicates, and consider adding the same duplicate check to the single creation endpoint for consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
agent.pymcp_server/feature_mcp.pyserver/routers/features.pyui/src/components/AgentThought.tsxui/src/components/KanbanColumn.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
server/routers/features.py (4)
server/routers/spec_creation.py (2)
validate_project_name(45-47)_get_project_path(34-42)server/routers/projects.py (1)
validate_project_name(77-84)server/websocket.py (2)
validate_project_name(107-109)_get_project_path(26-34)api/database.py (1)
Feature(19-44)
mcp_server/feature_mcp.py (2)
ui/src/lib/types.ts (1)
Feature(60-67)api/database.py (1)
Feature(19-44)
🔇 Additional comments (1)
agent.py (1)
78-131: Streaming output path looks consistent after migrating tosafe_print().The flush/end handling for TextBlock streaming (Line 96) and tool/result summaries (Line 98-125) is coherent and should keep the terminal UX roughly unchanged.
| @router.post("/repair") | ||
| async def repair_database(project_name: str): | ||
| """ | ||
| Repair the feature database by removing duplicates and compacting IDs. | ||
| Performs the following repairs: | ||
| 1. Removes duplicate features (keeping the one with lowest ID) | ||
| 2. Compacts IDs to be sequential (1, 2, 3, ...) with no gaps | ||
| 3. Resets priorities to match the new sequential IDs | ||
| """ | ||
| from sqlalchemy import text | ||
|
|
||
| project_name = validate_project_name(project_name) | ||
| project_dir = _get_project_path(project_name) | ||
|
|
||
| if not project_dir: | ||
| raise HTTPException(status_code=404, detail=f"Project '{project_name}' not found in registry") | ||
|
|
||
| if not project_dir.exists(): | ||
| raise HTTPException(status_code=404, detail="Project directory not found") | ||
|
|
||
| db_file = project_dir / "features.db" | ||
| if not db_file.exists(): | ||
| return {"duplicates_removed": 0, "ids_compacted": False, "total_features": 0} | ||
|
|
||
| _, Feature = _get_db_classes() | ||
|
|
||
| try: | ||
| with get_db_session(project_dir) as session: | ||
| # Step 1: Find and remove duplicates (keep lowest ID for each name) | ||
| duplicates_query = """ | ||
| SELECT id FROM features | ||
| WHERE id NOT IN ( | ||
| SELECT MIN(id) FROM features GROUP BY name | ||
| ) | ||
| """ | ||
| result = session.execute(text(duplicates_query)) | ||
| duplicate_ids = [row[0] for row in result.fetchall()] | ||
| duplicates_removed = len(duplicate_ids) | ||
|
|
||
| if duplicate_ids: | ||
| session.execute( | ||
| text(f"DELETE FROM features WHERE id IN ({','.join(map(str, duplicate_ids))})") | ||
| ) | ||
| session.commit() | ||
|
|
||
| # Step 2: Get current state | ||
| all_features = session.query(Feature).order_by(Feature.priority.asc(), Feature.id.asc()).all() | ||
| old_max_id = max(f.id for f in all_features) if all_features else 0 | ||
| total_features = len(all_features) | ||
|
|
||
| # Step 3: Check if compaction is needed | ||
| expected_ids = set(range(1, total_features + 1)) | ||
| actual_ids = set(f.id for f in all_features) | ||
| needs_compaction = expected_ids != actual_ids | ||
|
|
||
| new_max_id = old_max_id | ||
| if needs_compaction and all_features: | ||
| # First, shift all IDs to negative to avoid conflicts | ||
| session.execute(text("UPDATE features SET id = -id")) | ||
| session.commit() | ||
|
|
||
| # Then assign new sequential IDs | ||
| for new_id, feature in enumerate(all_features, start=1): | ||
| session.execute( | ||
| text(f"UPDATE features SET id = {new_id}, priority = {new_id} WHERE id = {-feature.id}") | ||
| ) | ||
| session.commit() | ||
|
|
||
| new_max_id = total_features | ||
|
|
||
| return { | ||
| "success": True, | ||
| "duplicates_removed": duplicates_removed, | ||
| "ids_compacted": needs_compaction, | ||
| "old_max_id": old_max_id, | ||
| "new_max_id": new_max_id, | ||
| "total_features": total_features | ||
| } | ||
| except HTTPException: | ||
| raise | ||
| except Exception: | ||
| logger.exception("Failed to repair database") | ||
| raise HTTPException(status_code=500, detail="Failed to repair database") | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for auth/middleware protecting endpoints
echo "=== Checking for auth/middleware patterns ==="
rg -n "APIRouter\(|add_middleware\(|Depends\(|HTTPBearer|OAuth2|Authorization" server --type py 2>/dev/null | head -50
echo ""
echo "=== Checking for /repair endpoint usages ==="
rg -n "/repair\b|repair_database\b" -C3 . --type py 2>/dev/null | head -100
echo ""
echo "=== Checking for CORS configuration ==="
rg -n "CORSMiddleware|allow_origins|cors" . --type py 2>/dev/null | head -30Repository: leonvanzyl/autocoder
Length of output: 1598
🏁 Script executed:
#!/bin/bash
# Check the complete features.py file to understand the router setup and any auth patterns
wc -l server/routers/features.py
echo ""
echo "=== Check router initialization and imports at top of features.py ==="
head -50 server/routers/features.py
echo ""
echo "=== Check for any existing endpoint protections ==="
rg -n "@router\." server/routers/features.py --type py -A5 | head -100Repository: leonvanzyl/autocoder
Length of output: 2999
🏁 Script executed:
#!/bin/bash
# Look for main app setup to understand overall security patterns
echo "=== Checking main app/server setup for auth ==="
fd -n "main\\.py|app\\.py|server\\.py|__main__\\.py" . --type f | head -5
echo ""
echo "=== Check if there's any config for auth/security ==="
fd -n "config\\.py|settings\\.py|env" . --type f | head -5Repository: leonvanzyl/autocoder
Length of output: 645
🏁 Script executed:
#!/bin/bash
echo "=== Check main.py middleware and app setup (lines 55-70) ==="
sed -n '55,70p' server/main.py
echo ""
echo "=== Search for feature_db_repair or similar repair functions ==="
rg -n "feature_db_repair|def.*repair" . --type py
echo ""
echo "=== Check the full repair_database return statements ==="
sed -n '260,344p' server/routers/features.py | grep -n "return"Repository: leonvanzyl/autocoder
Length of output: 959
🏁 Script executed:
#!/bin/bash
# Check if there are different response schemas for repair endpoint
echo "=== Check if repair response has schema/model definition ==="
rg -n "repair|Repair" . --type py | grep -i "response\|schema\|model"
echo ""
echo "=== Verify the response fields at different returns in repair_database ==="
sed -n '280,290p' server/routers/features.py
echo "---"
sed -n '328,344p' server/routers/features.pyRepository: leonvanzyl/autocoder
Length of output: 1130
🏁 Script executed:
#!/bin/bash
echo "=== Check the feature_db_repair function in MCP (lines 443+) ==="
sed -n '443,520p' mcp_server/feature_mcp.py
echo ""
echo "=== Also check if repair endpoint has ORM objects loaded before commit ==="
sed -n '307,325p' server/routers/features.pyRepository: leonvanzyl/autocoder
Length of output: 4110
POST /repair endpoint needs authentication protection, ORM bug fix, and response schema consistency.
-
Security: This endpoint performs destructive operations (deletes rows, rewrites primary keys). With CORS set to
allow_origins=["*"]and security middleware disabled for LAN access (seeserver/main.pylines 58-59, 65), there is no authentication barrier. Cross-origin requests from other LAN devices can trigger repairs. Add auth protection (admin token, config flag, or localhost-only restriction). -
ORM expiration bug: Line 307 loads ORM objects (
all_features = session.query(Feature)...all()), then line 320-321 commits after theid = -idupdate, then line 323+ reads from those expired objects in the loop. After a commit that modifies rows, the loaded ORM instances become stale. Use raw SQL to map IDs (e.g.,SELECT id, -id FROM features) instead of relying on pre-loaded ORM objects. This pattern appears identically inmcp_server/feature_mcp.pylines 485-507. -
Response schema inconsistency: When
features.dbis missing (line 282-284), the endpoint returns{"duplicates_removed": 0, "ids_compacted": False, "total_features": 0}, but the success path (line 328-344) includes"success": True, "old_max_id": ..., "new_max_id": .... Align both responses to include all fields or document the difference in the docstring.
| // Determine if component should be visible | ||
| // Use displayedThought for visibility check to prevent flickering when | ||
| // new logs come in without a valid thought | ||
| const shouldShow = useMemo(() => { | ||
| if (!thought) return false | ||
| const hasContent = thought || displayedThought | ||
| if (!hasContent) return false | ||
| if (agentStatus === 'running') return true | ||
| if (agentStatus === 'paused') { | ||
| return Date.now() - lastLogTimestamp < IDLE_TIMEOUT | ||
| } | ||
| return false | ||
| }, [thought, agentStatus, lastLogTimestamp]) | ||
| }, [thought, displayedThought, agentStatus, lastLogTimestamp]) |
There was a problem hiding this comment.
Idle timeout likely won’t trigger while paused unless something else re-renders.
shouldShow uses Date.now() - lastLogTimestamp < IDLE_TIMEOUT, but it’s computed only when deps change; if agentStatus === 'paused' and logs stop, nothing updates after 30s to flip it false. Consider scheduling a timeout (or a lightweight ticking state) while paused to force the transition.
Add completion check to prevent wasting API credits when no work remains: - Add all_features_complete() function to progress.py - Check at start of each iteration if all features are passing - Print friendly message and exit gracefully when complete - Skip check on first run (initializer hasn't created features yet) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…itch Two fixes for UI project management: 1. Projects created via /create-spec now automatically register in the autocoder registry so they appear in the UI dropdown immediately. Added register_project.py script and updated create-spec command. 2. Fixed stale data showing when switching projects. React Query cache is now cleared for the previous project before switching, preventing progress bar and features from showing wrong project's data. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Configure Playwright MCP to use bundled Chromium with --headless and --no-sandbox - Auto-detect Playwright's Chromium path for server environments - Add pgrep, cd, jq to allowed bash commands - Allow pkill for playwright, chrome, chromium processes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.claude/commands/create-spec.md:
- Around line 521-539: The hardcoded path to register_project.py will break for
other users; update the example/command to derive the script location
dynamically instead of using /home/john/autocoder/register_project.py — e.g.,
locate the installed Autocoder scripts via which start_ui.py and use its dirname
to build the path to register_project.py, or reference an environment variable
like $AUTOCODER_HOME or a relative path so the command works across
installations.
In @server/main.py:
- Around line 75-82: The CORS setup using app.add_middleware(CORSMiddleware,
allow_origins=["*"], allow_credentials=True, ...) is invalid because
allow_credentials=True cannot be used with a wildcard origin; either set
allow_credentials=False on the CORSMiddleware invocation or replace
allow_origins=["*"] with an explicit list of allowed origins (e.g.,
["http://192.168.1.x"]) or use allow_origin_regex to match dynamic origins while
keeping allow_credentials=True; update the CORSMiddleware call accordingly
(adjust allow_origins, allow_credentials, or add allow_origin_regex) so the
configuration complies with the CORS spec and Starlette's enforcement.
🧹 Nitpick comments (9)
ui/src/components/AgentThought.tsx (1)
91-103: Consider guarding against race between visibility and thought-update effects.If
shouldShowbecomesfalsewhile the thought-update effect (lines 77-88) has a pending 150ms timeout, both effects may race to modifydisplayedThought. In most cases, cleanup handles this, but the timing overlap (150ms vs 300ms) could cause a brief flash if a new thought arrives just as the component hides.A minor defensive improvement would be to check
shouldShowbefore updating state in the thought-update effect, or consolidate both behaviors into a single effect.server/main.py (1)
86-100: Consider adding a startup warning about LAN exposure.The localhost restriction removal is intentional per the PR, but users should be aware of the security implications. A startup log message would help.
Suggested addition in lifespan startup
@asynccontextmanager async def lifespan(app: FastAPI): """Lifespan context manager for startup and shutdown.""" # Startup - clean up orphaned lock files from previous runs cleanup_orphaned_locks() import logging logging.warning( "⚠️ Server is accessible from the local network (0.0.0.0). " "Only run on trusted networks." ) yield # Shutdown...client.py (2)
216-225: Hardcoded Chromium version and Linux-specific path may break on updates or other platforms.The path
chromium-1200will become stale when Playwright updates its bundled Chromium. Additionally,chrome-linux64is Linux-specific and won't work on macOS (chrome-mac) or Windows (chrome-win).Consider dynamically discovering the Chromium path:
🔧 Suggested improvement
- chromium_path = os.path.expanduser("~/.cache/ms-playwright/chromium-1200/chrome-linux64/chrome") + # Dynamically find Playwright's Chromium + playwright_cache = Path(os.path.expanduser("~/.cache/ms-playwright")) + chromium_path = None + if playwright_cache.exists(): + # Find the latest chromium directory + chromium_dirs = sorted(playwright_cache.glob("chromium-*"), reverse=True) + for chromium_dir in chromium_dirs: + # Check platform-specific subdirectories + for subdir in ["chrome-linux64/chrome", "chrome-mac/Chromium.app/Contents/MacOS/Chromium", "chrome-win/chrome.exe"]: + candidate = chromium_dir / subdir + if candidate.exists(): + chromium_path = str(candidate) + break + if chromium_path: + break playwright_args = [ "@playwright/mcp@latest", "--viewport-size", "1280x720", "--headless", # Always headless for server environments "--no-sandbox", # Required for some Linux environments ] # Use Playwright's Chromium if available (works on headless servers) - if os.path.exists(chromium_path): + if chromium_path: playwright_args.extend(["--executable-path", chromium_path])
27-52: Unused code:DEFAULT_PLAYWRIGHT_HEADLESSandget_playwright_headless()are no longer referenced.After switching to always-headless mode (line 220), the
PLAYWRIGHT_HEADLESSenvironment variable handling is dead code. Either remove it or use the function to conditionally set--headless.mcp_server/feature_mcp.py (2)
482-485: Use parameterized queries instead of string interpolation for SQL.While
duplicate_idscome from the database (reducing exploitation risk), using f-strings to build SQL is an unsafe pattern that could be problematic if this code is adapted elsewhere. Use parameterized queries for consistency and safety.🔧 Suggested fix using parameterized query
if duplicate_ids: + placeholders = ','.join(['?' for _ in duplicate_ids]) session.execute( - text(f"DELETE FROM features WHERE id IN ({','.join(map(str, duplicate_ids))})") + text(f"DELETE FROM features WHERE id IN ({placeholders})"), + duplicate_ids ) session.commit()
508-512: Use parameterized queries for ID reassignment SQL.Same concern as above—use parameterized queries instead of f-string interpolation for the UPDATE statement.
🔧 Suggested fix
# Then assign new sequential IDs for new_id, feature in enumerate(all_features, start=1): session.execute( - text(f"UPDATE features SET id = {new_id}, priority = {new_id} WHERE id = {-feature.id}") + text("UPDATE features SET id = :new_id, priority = :priority WHERE id = :old_id"), + {"new_id": new_id, "priority": new_id, "old_id": -feature.id} ) session.commit()server/routers/features.py (2)
252-336: Code duplication withmcp_server/feature_mcp.py::feature_db_repair().This repair logic is nearly identical to the MCP tool implementation. Consider extracting the shared repair logic into a common utility function (e.g., in
api/database.py) to avoid maintaining duplicate code.
292-294: Use parameterized queries instead of string interpolation.Same SQL injection pattern as noted in
mcp_server/feature_mcp.py. While the values come from the database, parameterized queries are the safer pattern.🔧 Suggested fixes
if duplicate_ids: + placeholders = ','.join(['?' for _ in duplicate_ids]) session.execute( - text(f"DELETE FROM features WHERE id IN ({','.join(map(str, duplicate_ids))})") + text(f"DELETE FROM features WHERE id IN ({placeholders})"), + duplicate_ids ) session.commit()for new_id, feature in enumerate(all_features, start=1): session.execute( - text(f"UPDATE features SET id = {new_id}, priority = {new_id} WHERE id = {-feature.id}") + text("UPDATE features SET id = :new_id, priority = :priority WHERE id = :old_id"), + {"new_id": new_id, "priority": new_id, "old_id": -feature.id} ) session.commit()Also applies to: 314-318
agent.py (1)
152-164: Consider usingsafe_printconsistently throughout the file.Several
print()calls inrun_autonomous_agent(lines 152-164, 176-188, 198-199, 280-301, 305-320) still use the regularprint()function. IfBlockingIOErroris a concern, these could also benefit fromsafe_print.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.claude/commands/create-spec.mdagent.pyclient.pymcp_server/feature_mcp.pyregister_project.pysecurity.pyserver/main.pyserver/routers/features.pyui/src/App.tsxui/src/components/AgentThought.tsxui/src/components/KanbanColumn.tsxui/tsconfig.tsbuildinfo
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/src/components/KanbanColumn.tsx
🧰 Additional context used
📓 Path-based instructions (7)
*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python backend should use the ClaudeSDKClient configuration in client.py with security hooks and MCP servers
Files:
register_project.pyagent.pyclient.pysecurity.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use SQLite with SQLAlchemy ORM for database access and feature management
Files:
register_project.pyagent.pyclient.pysecurity.pyserver/routers/features.pyserver/main.pymcp_server/feature_mcp.py
ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
ui/src/**/*.{ts,tsx}: Use TypeScript for React components in the UI (React 18 with TypeScript)
Use React Query (TanStack Query) for API calls and data fetching in the UI
Use Radix UI for component primitives in the React UI
Run ESLint to lint React UI code
Files:
ui/src/components/AgentThought.tsxui/src/App.tsx
agent.py
📄 CodeRabbit inference engine (CLAUDE.md)
Agent sessions should auto-continue with a 3-second delay between sessions
Files:
agent.py
security.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement bash command validation using the ALLOWED_COMMANDS whitelist in security.py
Files:
security.py
server/routers/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use FastAPI for REST API endpoints in the server/routers/ directory
Files:
server/routers/features.py
mcp_server/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use MCP (Model Context Protocol) servers for agent tools: feature_mcp.py for feature management, with tools: feature_get_stats, feature_get_next, feature_get_for_regression, feature_mark_passing, feature_skip, feature_create_bulk
Files:
mcp_server/feature_mcp.py
🧠 Learnings (8)
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to autonomous_agent_demo.py : Use absolute paths or registered project names when running the agent via: python autonomous_agent_demo.py --project-dir <path_or_name>
Applied to files:
register_project.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to registry.py : Store project registry using SQLite with POSIX path format (forward slashes) for cross-platform compatibility
Applied to files:
register_project.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/**/*.{ts,tsx} : Use React Query (TanStack Query) for API calls and data fetching in the UI
Applied to files:
ui/src/App.tsx
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to autonomous_agent_demo.py : YOLO mode skips regression testing and browser automation (Playwright MCP server) for rapid prototyping, but still runs lint and type-check
Applied to files:
client.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to mcp_server/**/*.py : Use MCP (Model Context Protocol) servers for agent tools: feature_mcp.py for feature management, with tools: feature_get_stats, feature_get_next, feature_get_for_regression, feature_mark_passing, feature_skip, feature_create_bulk
Applied to files:
client.pymcp_server/feature_mcp.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to security.py : Implement bash command validation using the ALLOWED_COMMANDS whitelist in security.py
Applied to files:
security.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Defense-in-depth security model: OS-level sandbox for bash commands, filesystem restricted to project directory, bash commands validated against ALLOWED_COMMANDS whitelist
Applied to files:
security.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Two-agent pattern: Initializer Agent reads app spec and creates features in first session; Coding Agent implements features one by one in subsequent sessions
Applied to files:
mcp_server/feature_mcp.py
🧬 Code graph analysis (4)
register_project.py (1)
registry.py (4)
register_project(215-250)get_project_path(275-293)RegistryError(54-56)Project(81-87)
agent.py (1)
progress.py (4)
all_features_complete(20-32)has_features(35-70)print_progress_summary(226-238)print_session_header(216-223)
server/routers/features.py (3)
server/utils/validation.py (1)
validate_project_name(10-28)server/routers/expand_project.py (1)
_get_project_path(35-43)api/database.py (1)
Feature(19-44)
mcp_server/feature_mcp.py (2)
ui/src/lib/types.ts (1)
Feature(60-67)api/database.py (1)
Feature(19-44)
🔇 Additional comments (11)
ui/src/components/AgentThought.tsx (2)
76-88: LGTM!The fade-out/fade-in transition logic is well-structured. The effect correctly guards against re-triggering once
displayedThoughtmatchesthought, and cleanup properly clears the pending timeout.
105-106: LGTM!The render guard correctly prevents rendering when the component is hidden or has no content, avoiding unnecessary DOM nodes and potential layout shifts.
security.py (2)
45-50: LGTM! Appropriate additions to the command allowlist.The additions of
pgrep,cd, andjqare reasonable for development workflows. Note thatcdonly affects the current subprocess, so it's only useful in compound commands (e.g.,cd dir && npm install), which is the intended use case.
190-199: LGTM! Extended pkill validation for headless browser processes.Adding
playwright,chrome, andchromiumto the allowed process names aligns with the headless server support improvements mentioned in the commit messages. The validation remains properly scoped.ui/src/App.tsx (1)
57-74: LGTM! Proper cache invalidation on project switch.Using
removeQueriesinstead ofinvalidateQueriesis the correct choice here—you want to clear stale data for the previous project rather than trigger a refetch. The updated dependency array correctly includesselectedProjectandqueryClientto ensure the callback is recreated when needed. This follows React Query best practices.server/main.py (1)
197-204: LAN binding change aligns with PR objectives.The host change from
127.0.0.1to0.0.0.0enables LAN access as intended. This should be paired with the documentation noting the trusted-network requirement.ui/tsconfig.tsbuildinfo (1)
1-1: Auto-generated build artifact—no review needed.This is a TypeScript compiler-generated build info file. It correctly reflects the current source file structure.
register_project.py (1)
25-61: LGTM!The script is well-structured with proper argument validation, idempotency checks (allowing re-registration at the same path), and comprehensive error handling for both
RegistryErrorandValueErrorcases. The path validation ensures the directory exists before registration.mcp_server/feature_mcp.py (1)
396-423: LGTM!The duplicate detection logic correctly:
- Pre-loads existing names to avoid N+1 queries
- Tracks duplicates within the same batch via
existing_names.add()- Maintains correct priority sequencing using
created_countagent.py (2)
27-48: LGTM! Good resilience pattern for buffered stdout.The retry logic with exponential backoff handles
BlockingIOErrorgracefully when stdout is a pipe. The silent fallback on final failure is appropriate to avoid crashing the agent over a print statement.
202-211: LGTM!The completion check is well-placed to save API credits by stopping before starting a new session when all features pass. The
is_first_runguard correctly avoids checking before the initializer has created features.
| ## 4. Register the Project (REQUIRED) | ||
|
|
||
| **After writing the status file**, register the project so it appears in the AutoCoder UI. | ||
|
|
||
| Run this command using Bash: | ||
|
|
||
| ```bash | ||
| python /home/john/autocoder/register_project.py "<project_name>" "$ARGUMENTS" | ||
| ``` | ||
|
|
||
| Where `<project_name>` is derived from the project path (the last directory component, e.g., `my-app` from `~/projects/my-app`). | ||
|
|
||
| **Example:** | ||
| ```bash | ||
| python /home/john/autocoder/register_project.py "my-app" "/home/john/projects/my-app" | ||
| ``` | ||
|
|
||
| **Note:** If the project is already registered, this will succeed silently. This ensures projects created via `/create-spec` appear in the UI dropdown. | ||
|
|
There was a problem hiding this comment.
Hardcoded user path will break for other users.
The absolute path /home/john/autocoder/register_project.py is user-specific and will fail for anyone with a different username or installation directory. Use a relative path or derive the path dynamically.
Suggested fix
Run this command using Bash:
```bash
-python /home/john/autocoder/register_project.py "<project_name>" "$ARGUMENTS"
+python "$(dirname "$(which start_ui.py)")/register_project.py" "<project_name>" "$ARGUMENTS"Where <project_name> is derived from the project path (the last directory component, e.g., my-app from ~/projects/my-app).
Example:
-python /home/john/autocoder/register_project.py "my-app" "/home/john/projects/my-app"
+python register_project.py "my-app" "/path/to/projects/my-app"
</details>
Alternatively, if the script is installed to the system PATH or the autocoder directory is known via an environment variable (e.g., `$AUTOCODER_HOME`), reference that instead.
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @.claude/commands/create-spec.md around lines 521 - 539, The hardcoded path
to register_project.py will break for other users; update the example/command to
derive the script location dynamically instead of using
/home/john/autocoder/register_project.py — e.g., locate the installed Autocoder
scripts via which start_ui.py and use its dirname to build the path to
register_project.py, or reference an environment variable like $AUTOCODER_HOME
or a relative path so the command works across installations.
</details>
<!-- fingerprinting:phantom:poseidon:ocelot -->
<!-- This is an auto-generated comment by CodeRabbit -->
| const shouldShow = useMemo(() => { | ||
| if (!thought) return false | ||
| const hasContent = thought || displayedThought | ||
| if (!hasContent) return false | ||
| if (agentStatus === 'running') return true | ||
| if (agentStatus === 'paused') { | ||
| return Date.now() - lastLogTimestamp < IDLE_TIMEOUT | ||
| } | ||
| return false | ||
| }, [thought, agentStatus, lastLogTimestamp]) | ||
| }, [thought, displayedThought, agentStatus, lastLogTimestamp]) |
There was a problem hiding this comment.
Date.now() inside useMemo breaks idle timeout logic.
When agentStatus === 'paused', the Date.now() value is captured once when the memo computes. Since there's no dependency that triggers re-computation as time passes, the component won't hide after the 30-second idle timeout if no new logs arrive.
Consider using an interval or a state-based approach to track elapsed time:
🛠️ Suggested fix using interval-based idle tracking
+ const [now, setNow] = useState(Date.now())
+
+ // Update `now` periodically when paused to enable idle timeout
+ useEffect(() => {
+ if (agentStatus === 'paused') {
+ const interval = setInterval(() => setNow(Date.now()), 1000)
+ return () => clearInterval(interval)
+ }
+ }, [agentStatus])
+
const shouldShow = useMemo(() => {
const hasContent = thought || displayedThought
if (!hasContent) return false
if (agentStatus === 'running') return true
if (agentStatus === 'paused') {
- return Date.now() - lastLogTimestamp < IDLE_TIMEOUT
+ return now - lastLogTimestamp < IDLE_TIMEOUT
}
return false
- }, [thought, displayedThought, agentStatus, lastLogTimestamp])
+ }, [thought, displayedThought, agentStatus, lastLogTimestamp, now])📝 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.
| const shouldShow = useMemo(() => { | |
| if (!thought) return false | |
| const hasContent = thought || displayedThought | |
| if (!hasContent) return false | |
| if (agentStatus === 'running') return true | |
| if (agentStatus === 'paused') { | |
| return Date.now() - lastLogTimestamp < IDLE_TIMEOUT | |
| } | |
| return false | |
| }, [thought, agentStatus, lastLogTimestamp]) | |
| }, [thought, displayedThought, agentStatus, lastLogTimestamp]) | |
| const [now, setNow] = useState(Date.now()) | |
| // Update `now` periodically when paused to enable idle timeout | |
| useEffect(() => { | |
| if (agentStatus === 'paused') { | |
| const interval = setInterval(() => setNow(Date.now()), 1000) | |
| return () => clearInterval(interval) | |
| } | |
| }, [agentStatus]) | |
| const shouldShow = useMemo(() => { | |
| const hasContent = thought || displayedThought | |
| if (!hasContent) return false | |
| if (agentStatus === 'running') return true | |
| if (agentStatus === 'paused') { | |
| return now - lastLogTimestamp < IDLE_TIMEOUT | |
| } | |
| return false | |
| }, [thought, displayedThought, agentStatus, lastLogTimestamp, now]) |
- Add dark mode toggle in Settings with localStorage persistence - Support system color scheme preference detection - Update all neo-brutalism components to use CSS variables - Add sqlite3 to allowed bash commands - Enable Bash tool for expand chat to query features database Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
server/main.py (1)
202-209: Binding to0.0.0.0should be opt-in (env/flag), not the default.
Otherwise it’s easy to unintentionally expose the UI/API beyond the LAN (containers, port-forwarding, VPNs).Proposed fix (host derived from same LAN flag)
@@ if __name__ == "__main__": import uvicorn + LAN_ACCESS = os.getenv("AUTOCODER_LAN_ACCESS", "0") == "1" uvicorn.run( "server.main:app", - host="0.0.0.0", # LAN accessible + host="0.0.0.0" if LAN_ACCESS else "127.0.0.1", port=8888, reload=True, )client.py (1)
208-224: Pin Playwright MCP version; make Chromium discovery dynamic and allow non-interactive npx execution.The hardcoded
chromium-1200path is brittle and breaks if the installed Playwright version differs. Replace with dynamic discovery of available Chromium versions. Pin@playwright/mcp@latestto a specific version (configurable viaPLAYWRIGHT_MCP_VERSIONenv var) for reproducibility. Addnpx --yesto prevent prompts in non-interactive environments (CI, servers). Make--no-sandboxconfigurable viaPLAYWRIGHT_NO_SANDBOXenv var instead of forcing it unconditionally, reducing the security posture.Proposed fix (pin/version env + non-interactive npx + dynamic Chromium discovery)
@@ - chromium_path = os.path.expanduser("~/.cache/ms-playwright/chromium-1200/chrome-linux64/chrome") + playwright_mcp_version = os.getenv("PLAYWRIGHT_MCP_VERSION", "latest") + chromium_candidates = sorted( + Path("~/.cache/ms-playwright").expanduser().glob("chromium-*/chrome-linux64/chrome") + ) + chromium_path = str(chromium_candidates[-1]) if chromium_candidates else None + no_sandbox = os.getenv("PLAYWRIGHT_NO_SANDBOX", "1") == "1" @@ - playwright_args = [ - "@playwright/mcp@latest", + playwright_args = [ + f"@playwright/mcp@{playwright_mcp_version}", "--viewport-size", "1280x720", "--headless", # Always headless for server environments - "--no-sandbox", # Required for some Linux environments ] + if no_sandbox: + playwright_args.append("--no-sandbox") @@ - if os.path.exists(chromium_path): + if chromium_path and os.path.exists(chromium_path): playwright_args.extend(["--executable-path", chromium_path]) mcp_servers["playwright"] = { "command": "npx", - "args": playwright_args, + "args": ["--yes", *playwright_args], }server/services/expand_chat_session.py (2)
173-188: Use the repo'screate_client()function instead of instantiatingClaudeSDKClientdirectly to ensure security hooks are applied.This code bypasses the standard client configuration in
client.py. Thecreate_client()function properly wires thebash_security_hookviaHookMatcher, but the direct instantiation here (lines 173-188) enablesBashtool without any security validation. Replace with:from client import create_client # In the initialization/setup self.client = create_client( project_dir=self.project_dir, model="claude-opus-4-5-20251101", yolo_mode=False )Ensure the system prompt and other context are still passed appropriately through the client's configuration.
148-158:"Bash(*)"allows unnecessary shell access; remove or gate it behind explicit flag.While
Bash(*)grants broad shell execution, actual commands are validated viabash_security_hookagainstALLOWED_COMMANDSwhitelist insecurity.py. However, the stated reason—"for sqlite3 queries"—is inaccurate;_create_features_bulk()uses SQLAlchemy ORM exclusively and doesn't execute raw bash commands. Remove"Bash(*)"unless genuinely needed by the Claude CLI for the expand-project workflow. If required, gate it behind an explicitAUTOCODER_ENABLE_BASH_TOOLenvironment variable to prevent accidental exposure in production deployments.security.py (2)
14-62: Addsqlite3,bash, andshtoCOMMANDS_NEEDING_EXTRA_VALIDATIONwith strict command parsing.The allowlist is currently bypassable:
sqlite3supports.shell/.systemdot-commands that execute arbitrary OS commands via the CLI (confirmed in official documentation).bashandshallowbash -c "..."andsh -lc "..."patterns where the inner command string is not parsed or validated, only the outer shell wrapper is checked. For example,bash -c "rm -rf /"would pass the allowlist becauseextract_commands()skips flags and does not inspect the string argument.Both issues require additional validation. Add validation functions that:
- Block
bash/shwhen used with-cor-lcflags.- Block
sqlite3commands containing dot-commands (e.g.,.shell,.system), and enforce read-only mode and project-directory restrictions on database paths.Proposed fix
-COMMANDS_NEEDING_EXTRA_VALIDATION = {"pkill", "chmod", "init.sh"} +COMMANDS_NEEDING_EXTRA_VALIDATION = {"pkill", "chmod", "init.sh", "sqlite3", "bash", "sh"} + +def validate_shell_wrapper_command(command_string: str) -> tuple[bool, str]: + # Disallow executing arbitrary strings via -c / -lc + try: + tokens = shlex.split(command_string) + except ValueError: + return False, "Could not parse shell command" + if len(tokens) >= 2 and tokens[0] in ("bash", "sh") and tokens[1] in ("-c", "-lc"): + return False, f"{tokens[0]} {tokens[1]} is not allowed" + return True, "" + +def validate_sqlite3_command(command_string: str) -> tuple[bool, str]: + # Minimal baseline: forbid sqlite dot-commands in CLI args (best-effort). + # NOTE: Further tightening should restrict DB path + enforce -readonly, based on context/project_dir. + try: + tokens = shlex.split(command_string) + except ValueError: + return False, "Could not parse sqlite3 command" + # Block obvious dot-commands passed directly (e.g., sqlite3 db '.shell rm -rf /') + for t in tokens[1:]: + if t.startswith("."): + return False, "sqlite3 dot-commands are not allowed" + return True, ""And in
bash_security_hook:elif cmd == "init.sh": allowed, reason = validate_init_script(cmd_segment) if not allowed: return {"decision": "block", "reason": reason} + elif cmd in ("bash", "sh"): + allowed, reason = validate_shell_wrapper_command(cmd_segment) + if not allowed: + return {"decision": "block", "reason": reason} + elif cmd == "sqlite3": + allowed, reason = validate_sqlite3_command(cmd_segment) + if not allowed: + return {"decision": "block", "reason": reason}
191-231: Update pkill tests to cover chrome/chromium/playwright allowed patterns.The allowlist expansion is appropriately scoped to a dev-only autonomous agent and protected by strict process name whitelist validation. The concern about killing non-dev processes is mitigated by this allowlist design. However, test coverage is outdated:
test_security.pylists"pkill chrome"as a command that should be blocked, but the current implementation allows it. Add test cases confirming that"pkill chrome","pkill chromium", and"pkill playwright"are now permitted, or update the allowlist if this expansion was unintended.
🤖 Fix all issues with AI agents
In @server/main.py:
- Around line 86-100: Re-enable the commented-out HTTP middleware by registering
the require_localhost middleware with @app.middleware("http") and enforce
localhost-only by default inside require_localhost (checking request.client.host
against "127.0.0.1", "::1", "localhost", None); add a configuration check (e.g.,
ALLOW_LAN or ENABLE_LAN_ACCESS) that must be explicitly set to true to permit
non-local hosts and ensure that config is read before middleware registration;
when LAN access is enabled, log a clear warning that the server is accepting LAN
connections.
- Around line 75-82: The CORS setup is invalid:
app.add_middleware(CORSMiddleware, allow_origins=["*"], allow_credentials=True,
...) must not combine wildcard origins with credentials; update the logic to
read an environment flag (e.g., ENABLE_LAN or ALLOW_CORS_LAN) and, when LAN
access is enabled, set allow_origins to a restricted list from an env var (or
parse a comma-separated ALLOWED_ORIGINS) and keep allow_credentials=True;
otherwise set allow_origins=["*"] with allow_credentials=False. Also restore and
gate the localhost-only security middleware (the commented-out check) behind the
same configuration flag so host="0.0.0.0" and LAN exposure are opt-in via env,
and ensure any place that binds host uses that flag to avoid accidental 0.0.0.0
deployment.
In @ui/src/App.tsx:
- Around line 26-36: The initDarkMode function runs at module load and can throw
if window, localStorage, matchMedia, or document are unavailable; wrap the body
of initDarkMode in a defensive check/try-catch: first verify typeof window !==
'undefined' and typeof document !== 'undefined', then check 'localStorage' in
window and 'matchMedia' in window before using them, and fall back to a safe
default if unavailable; also catch any thrown errors and no-op (or log) so
document.documentElement.classList.add('dark') is only called when safe.
In @ui/src/components/SettingsModal.tsx:
- Around line 9-25: getInitialDarkMode and applyDarkMode can throw in restricted
environments because they access localStorage, document, and matchMedia without
guards; wrap all localStorage.getItem/setItem calls in try/catch and return
sensible fallbacks, and guard document/window usage (e.g., check typeof window
!== 'undefined' and if document exists) before touching classList or matchMedia.
Specifically, update getInitialDarkMode to try reading localStorage and fall
back to false or the result of window.matchMedia only when safe (catch errors
from localStorage and matchMedia), and update applyDarkMode to check for
document.documentElement before modifying classList and to catch errors from
localStorage.setItem so toggling dark mode never throws.
🧹 Nitpick comments (3)
ui/src/styles/globals.css (1)
209-233: Make.neo-inputshadow theme-aware (currently hard-coded for light backgrounds).
box-shadow: 3px 3px 0px rgba(0, 0, 0, 0.1)(Line 218) will likely be too subtle/odd in dark mode—consider switching to an existing--shadow-neo-*token or adding a dedicated input shadow token. Based on learnings, prefer CSS variables for theming.ui/src/components/SettingsModal.tsx (2)
34-43: Simplify: make the effect react todarkModeand remove the duplicateapplyDarkModecall in the click handler.This avoids the “uses
darkModebut deps are []” pitfall and centralizes the side-effect.Proposed refactor
const [darkMode, setDarkMode] = useState(getInitialDarkMode) - // Apply dark mode on initial load useEffect(() => { applyDarkMode(darkMode) - }, []) + }, [darkMode]) const handleDarkModeToggle = () => { - const newValue = !darkMode - setDarkMode(newValue) - applyDarkMode(newValue) + setDarkMode(v => !v) }
164-198: Dark mode toggle UI looks solid (role/aria wired correctly).Minor: consider adding
type="button"on the toggle<button>to prevent accidental form-submit behavior if this modal ever ends up inside a<form>.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
client.pysecurity.pyserver/main.pyserver/services/expand_chat_session.pyui/src/App.tsxui/src/components/SettingsModal.tsxui/src/styles/globals.css
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use SQLite with SQLAlchemy ORM for database access and feature management
Files:
server/services/expand_chat_session.pyserver/main.pyclient.pysecurity.py
ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
ui/src/**/*.{ts,tsx}: Use TypeScript for React components in the UI (React 18 with TypeScript)
Use React Query (TanStack Query) for API calls and data fetching in the UI
Use Radix UI for component primitives in the React UI
Run ESLint to lint React UI code
Files:
ui/src/components/SettingsModal.tsxui/src/App.tsx
ui/src/styles/**/*.css
📄 CodeRabbit inference engine (CLAUDE.md)
ui/src/styles/**/*.css: Use Tailwind CSS v4 for styling in React UI, with CSS variables defined in globals.css via @theme directive
React UI uses neobrutalism design system with CSS variables for color tokens: --color-neo-pending (yellow), --color-neo-progress (cyan), --color-neo-done (green)
React UI custom animations: animate-slide-in, animate-pulse-neo, animate-shimmer
Files:
ui/src/styles/globals.css
*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python backend should use the ClaudeSDKClient configuration in client.py with security hooks and MCP servers
Files:
client.pysecurity.py
security.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement bash command validation using the ALLOWED_COMMANDS whitelist in security.py
Files:
security.py
🧠 Learnings (8)
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to security.py : Implement bash command validation using the ALLOWED_COMMANDS whitelist in security.py
Applied to files:
server/services/expand_chat_session.pysecurity.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/styles/**/*.css : React UI uses neobrutalism design system with CSS variables for color tokens: --color-neo-pending (yellow), --color-neo-progress (cyan), --color-neo-done (green)
Applied to files:
ui/src/components/SettingsModal.tsxui/src/styles/globals.css
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/**/*.{ts,tsx} : Use React Query (TanStack Query) for API calls and data fetching in the UI
Applied to files:
ui/src/App.tsx
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/**/*.{ts,tsx} : Use TypeScript for React components in the UI (React 18 with TypeScript)
Applied to files:
ui/src/App.tsx
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/styles/**/*.css : Use Tailwind CSS v4 for styling in React UI, with CSS variables defined in globals.css via theme directive
Applied to files:
ui/src/styles/globals.css
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/styles/**/*.css : React UI custom animations: animate-slide-in, animate-pulse-neo, animate-shimmer
Applied to files:
ui/src/styles/globals.css
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to autonomous_agent_demo.py : YOLO mode skips regression testing and browser automation (Playwright MCP server) for rapid prototyping, but still runs lint and type-check
Applied to files:
client.py
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to mcp_server/**/*.py : Use MCP (Model Context Protocol) servers for agent tools: feature_mcp.py for feature management, with tools: feature_get_stats, feature_get_next, feature_get_for_regression, feature_mark_passing, feature_skip, feature_create_bulk
Applied to files:
client.py
🧬 Code graph analysis (1)
ui/src/components/SettingsModal.tsx (2)
ui/src/hooks/useProjects.ts (3)
useSettings(233-241)useAvailableModels(223-231)useUpdateSettings(243-274)ui/src/lib/api.ts (1)
updateSettings(301-306)
🔇 Additional comments (3)
ui/src/styles/globals.css (2)
62-78: Dark mode token override looks correct forhtml.dark.Using
:root.darkwill correctly apply whendocument.documentElement.classListtogglesdark.
126-129: Good: migrated component colors to shared CSS variables.This is consistent with the repo’s “globals.css defines tokens” approach and should make dark mode much easier to keep consistent. Based on learnings, use Tailwind v4 tokens in
globals.css.Also applies to: 173-180, 215-217, 223-224, 231-232, 243-245, 251-253, 259-260, 276-289, 297-308, 312-320, 499-511
ui/src/App.tsx (1)
71-89: Query keys match correctly—no action needed.The queryKey arrays in
removeQueries()calls match those used inuseFeatures()anduseAgentStatus()hooks exactly:['features', projectName]and['agent-status', projectName]respectively. The cache cleanup will work as intended.
| # CORS - allow all origins for LAN access | ||
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=[ | ||
| "http://localhost:5173", # Vite dev server | ||
| "http://127.0.0.1:5173", | ||
| "http://localhost:8888", # Production | ||
| "http://127.0.0.1:8888", | ||
| ], | ||
| allow_origins=["*"], | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "main.py" | head -20Repository: leonvanzyl/autocoder
Length of output: 80
🏁 Script executed:
fd -t f "main\.py" | grep -E "(server|app)"Repository: leonvanzyl/autocoder
Length of output: 78
🏁 Script executed:
git ls-files | grep -E "server.*main\.py"Repository: leonvanzyl/autocoder
Length of output: 78
🏁 Script executed:
wc -l server/main.pyRepository: leonvanzyl/autocoder
Length of output: 82
🏁 Script executed:
cat -n server/main.py | sed -n '70,110p'Repository: leonvanzyl/autocoder
Length of output: 1699
🏁 Script executed:
cat -n server/main.py | sed -n '195,215p'Repository: leonvanzyl/autocoder
Length of output: 582
🏁 Script executed:
rg -t py "cookie|session|auth|credential" server/main.pyRepository: leonvanzyl/autocoder
Length of output: 655
🏁 Script executed:
rg -t py "Cookie|Session|Auth|JWT" --max-count=20Repository: leonvanzyl/autocoder
Length of output: 8488
🏁 Script executed:
grep -r "import.*Cookie\|from.*cookie" --include="*.py" | head -20Repository: leonvanzyl/autocoder
Length of output: 46
🏁 Script executed:
rg -t py "require_auth|@.*auth|Authorization|Bearer" server/routers/ | head -30Repository: leonvanzyl/autocoder
Length of output: 46
🏁 Script executed:
grep -r "get_db\|Depends" server/routers/*.py | head -20Repository: leonvanzyl/autocoder
Length of output: 1243
🏁 Script executed:
cat -n server/routers/projects.py | head -50Repository: leonvanzyl/autocoder
Length of output: 1582
🏁 Script executed:
rg -t py "allow_credentials\|CORSMiddleware" server/ | head -30Repository: leonvanzyl/autocoder
Length of output: 46
🏁 Script executed:
cat server/main.py | head -30Repository: leonvanzyl/autocoder
Length of output: 790
🌐 Web query:
CORS wildcard allow_origins credentials=True specification RFC6454 invalid
💡 Result:
Short answer: You cannot use Access-Control-Allow-Origin: * together with Access-Control-Allow-Credentials: true — browsers reject that combination. The CORS rules (and browser implementations) disallow a wildcard origin when credentials are allowed because it would expose credentialed responses to every origin. [1][2]
Notes:
- RFC6454 defines what an "origin" is (the basis for these CORS checks) but does not permit or authorize using '*' with credentials — that restriction comes from the CORS/fetch behavior implemented by browsers. [3][1]
Sources:
- MDN: "Credential is not supported if the CORS header 'Access-Control-Allow-Origin' is '*'." [1]
- PortSwigger: explanation that Access-Control-Allow-Origin: * cannot be combined with Access-Control-Allow-Credentials: true for security reasons. [2]
- RFC 6454: The Web Origin Concept (defines origin semantics used by CORS). [3]
🌐 Web query:
FastAPI CORSMiddleware allow_origins ["*"] allow_credentials True behavior
💡 Result:
Short answer: don't use allow_origins=[""] together with allow_credentials=True — browsers require a specific origin (not ""), and Starlette/FastAPI documentation enforces/notes this restriction. If you need credentialed requests, return the request's Origin (or list explicit origins) and set Vary: Origin instead of using "*" . [1][2][3]
Details/references:
- Starlette / FastAPI middleware docs: when allow_credentials is True, allow_origins (and allow_methods/allow_headers) must not be ["*"] and must be explicit. [1]
- FastAPI docs / tutorial (same behavior explained). [2]
- Browser behavior / spec (MDN/W3C): credentialed responses must not use Access-Control-Allow-Origin: *; browsers will block credentialed responses if the origin header is wildcard. Use an explicit origin or reflect Origin and send Vary: Origin. [3]
If you want, I can show a short example that correctly supports credentials (reflect Origin + Vary header) or a secure config for common dev/prod setups.
Wildcard CORS with allow_credentials=True is invalid per CORS specification and FastAPI/Starlette middleware.
Browsers reject Access-Control-Allow-Origin: * combined with Access-Control-Allow-Credentials: true. Either restrict allow_origins to specific origins and enable credentials, or use wildcard with allow_credentials=False.
Additionally, the commented-out localhost security middleware should be gated by configuration (not removed) to prevent accidental insecure deployments, and host="0.0.0.0" enables unintended network exposure. Use environment variables to make LAN access opt-in.
Proposed fix (env-gated LAN + valid CORS configuration)
@@
-# CORS - allow all origins for LAN access
-app.add_middleware(
- CORSMiddleware,
- allow_origins=["*"],
- allow_credentials=True,
- allow_methods=["*"],
- allow_headers=["*"],
-)
+LAN_ACCESS = os.getenv("AUTOCODER_LAN_ACCESS", "0") == "1"
+CORS_ORIGINS = [o.strip() for o in os.getenv("AUTOCODER_CORS_ORIGINS", "").split(",") if o.strip()]
+if LAN_ACCESS and not CORS_ORIGINS:
+ # Explicitly configured by user; use wildcard but disable credentials
+ CORS_ORIGINS = ["*"]
+
+app.add_middleware(
+ CORSMiddleware,
+ allow_origins=CORS_ORIGINS or ["http://127.0.0.1:8888", "http://localhost:8888"],
+ allow_credentials=False if CORS_ORIGINS == ["*"] else True,
+ allow_methods=["*"],
+ allow_headers=["*"],
+)🤖 Prompt for AI Agents
In @server/main.py around lines 75 - 82, The CORS setup is invalid:
app.add_middleware(CORSMiddleware, allow_origins=["*"], allow_credentials=True,
...) must not combine wildcard origins with credentials; update the logic to
read an environment flag (e.g., ENABLE_LAN or ALLOW_CORS_LAN) and, when LAN
access is enabled, set allow_origins to a restricted list from an env var (or
parse a comma-separated ALLOWED_ORIGINS) and keep allow_credentials=True;
otherwise set allow_origins=["*"] with allow_credentials=False. Also restore and
gate the localhost-only security middleware (the commented-out check) behind the
same configuration flag so host="0.0.0.0" and LAN exposure are opt-in via env,
and ensure any place that binds host uses that flag to avoid accidental 0.0.0.0
deployment.
| # Security Middleware (disabled for LAN access) | ||
| # ============================================================================ | ||
|
|
||
| @app.middleware("http") | ||
| async def require_localhost(request: Request, call_next): | ||
| """Only allow requests from localhost.""" | ||
| client_host = request.client.host if request.client else None | ||
|
|
||
| # Allow localhost connections | ||
| if client_host not in ("127.0.0.1", "::1", "localhost", None): | ||
| raise HTTPException(status_code=403, detail="Localhost access only") | ||
|
|
||
| return await call_next(request) | ||
| # NOTE: Localhost restriction removed to allow LAN access | ||
| # @app.middleware("http") | ||
| # async def require_localhost(request: Request, call_next): | ||
| # """Only allow requests from localhost.""" | ||
| # client_host = request.client.host if request.client else None | ||
| # | ||
| # # Allow localhost connections | ||
| # if client_host not in ("127.0.0.1", "::1", "localhost", None): | ||
| # raise HTTPException(status_code=403, detail="Localhost access only") | ||
| # | ||
| # return await call_next(request) | ||
|
|
There was a problem hiding this comment.
Don’t comment out the localhost-only middleware; gate it behind config so “secure-by-default” stays intact.
This should default to localhost-only, with explicit opt-in for LAN access (especially since the PR target is master).
Proposed fix (conditional middleware)
@@
-# NOTE: Localhost restriction removed to allow LAN access
-# @app.middleware("http")
-# async def require_localhost(request: Request, call_next):
-# """Only allow requests from localhost."""
-# client_host = request.client.host if request.client else None
-#
-# # Allow localhost connections
-# if client_host not in ("127.0.0.1", "::1", "localhost", None):
-# raise HTTPException(status_code=403, detail="Localhost access only")
-#
-# return await call_next(request)
+LAN_ACCESS = os.getenv("AUTOCODER_LAN_ACCESS", "0") == "1"
+if not LAN_ACCESS:
+ @app.middleware("http")
+ async def require_localhost(request: Request, call_next):
+ client_host = request.client.host if request.client else None
+ if client_host not in ("127.0.0.1", "::1", "localhost", None):
+ raise HTTPException(status_code=403, detail="Localhost access only")
+ return await call_next(request)📝 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.
| # Security Middleware (disabled for LAN access) | |
| # ============================================================================ | |
| @app.middleware("http") | |
| async def require_localhost(request: Request, call_next): | |
| """Only allow requests from localhost.""" | |
| client_host = request.client.host if request.client else None | |
| # Allow localhost connections | |
| if client_host not in ("127.0.0.1", "::1", "localhost", None): | |
| raise HTTPException(status_code=403, detail="Localhost access only") | |
| return await call_next(request) | |
| # NOTE: Localhost restriction removed to allow LAN access | |
| # @app.middleware("http") | |
| # async def require_localhost(request: Request, call_next): | |
| # """Only allow requests from localhost.""" | |
| # client_host = request.client.host if request.client else None | |
| # | |
| # # Allow localhost connections | |
| # if client_host not in ("127.0.0.1", "::1", "localhost", None): | |
| # raise HTTPException(status_code=403, detail="Localhost access only") | |
| # | |
| # return await call_next(request) | |
| # Security Middleware (disabled for LAN access) | |
| # ============================================================================ | |
| LAN_ACCESS = os.getenv("AUTOCODER_LAN_ACCESS", "0") == "1" | |
| if not LAN_ACCESS: | |
| @app.middleware("http") | |
| async def require_localhost(request: Request, call_next): | |
| client_host = request.client.host if request.client else None | |
| if client_host not in ("127.0.0.1", "::1", "localhost", None): | |
| raise HTTPException(status_code=403, detail="Localhost access only") | |
| return await call_next(request) |
🤖 Prompt for AI Agents
In @server/main.py around lines 86 - 100, Re-enable the commented-out HTTP
middleware by registering the require_localhost middleware with
@app.middleware("http") and enforce localhost-only by default inside
require_localhost (checking request.client.host against "127.0.0.1", "::1",
"localhost", None); add a configuration check (e.g., ALLOW_LAN or
ENABLE_LAN_ACCESS) that must be explicitly set to true to permit non-local hosts
and ensure that config is read before middleware registration; when LAN access
is enabled, log a clear warning that the server is accepting LAN connections.
| // Apply dark mode on initial load (before React renders) | ||
| function initDarkMode() { | ||
| const saved = localStorage.getItem('darkMode') | ||
| const prefersDark = window.matchMedia('(prefers-color-scheme: dark)').matches | ||
| const isDark = saved !== null ? saved === 'true' : prefersDark | ||
| if (isDark) { | ||
| document.documentElement.classList.add('dark') | ||
| } | ||
| } | ||
| initDarkMode() | ||
|
|
There was a problem hiding this comment.
Guard initDarkMode() against missing window / localStorage exceptions (can crash app before render).
Because this runs at module evaluation time, any localStorage/matchMedia/document exception can prevent the UI from loading at all.
Proposed fix
// Apply dark mode on initial load (before React renders)
function initDarkMode() {
- const saved = localStorage.getItem('darkMode')
- const prefersDark = window.matchMedia('(prefers-color-scheme: dark)').matches
- const isDark = saved !== null ? saved === 'true' : prefersDark
- if (isDark) {
- document.documentElement.classList.add('dark')
- }
+ if (typeof window === 'undefined') return
+ try {
+ const saved = window.localStorage.getItem('darkMode')
+ const prefersDark =
+ window.matchMedia?.('(prefers-color-scheme: dark)')?.matches ?? false
+ const isDark = saved !== null ? saved === 'true' : prefersDark
+ document.documentElement.classList.toggle('dark', isDark)
+ } catch {
+ // ignore storage / matchMedia failures (keep default theme)
+ }
}
initDarkMode()🤖 Prompt for AI Agents
In @ui/src/App.tsx around lines 26 - 36, The initDarkMode function runs at
module load and can throw if window, localStorage, matchMedia, or document are
unavailable; wrap the body of initDarkMode in a defensive check/try-catch: first
verify typeof window !== 'undefined' and typeof document !== 'undefined', then
check 'localStorage' in window and 'matchMedia' in window before using them, and
fall back to a safe default if unavailable; also catch any thrown errors and
no-op (or log) so document.documentElement.classList.add('dark') is only called
when safe.
| // Dark mode helper functions | ||
| function getInitialDarkMode(): boolean { | ||
| if (typeof window === 'undefined') return false | ||
| const saved = localStorage.getItem('darkMode') | ||
| if (saved !== null) return saved === 'true' | ||
| return window.matchMedia('(prefers-color-scheme: dark)').matches | ||
| } | ||
|
|
||
| function applyDarkMode(isDark: boolean): void { | ||
| if (isDark) { | ||
| document.documentElement.classList.add('dark') | ||
| } else { | ||
| document.documentElement.classList.remove('dark') | ||
| } | ||
| localStorage.setItem('darkMode', String(isDark)) | ||
| } | ||
|
|
There was a problem hiding this comment.
Harden dark mode helpers: localStorage access can throw; document should be guarded.
This can currently crash when opening settings or toggling dark mode in restricted storage environments.
Proposed fix
function getInitialDarkMode(): boolean {
if (typeof window === 'undefined') return false
- const saved = localStorage.getItem('darkMode')
- if (saved !== null) return saved === 'true'
- return window.matchMedia('(prefers-color-scheme: dark)').matches
+ try {
+ const saved = window.localStorage.getItem('darkMode')
+ if (saved !== null) return saved === 'true'
+ } catch {
+ // ignore
+ }
+ return window.matchMedia?.('(prefers-color-scheme: dark)')?.matches ?? false
}
function applyDarkMode(isDark: boolean): void {
+ if (typeof window === 'undefined') return
if (isDark) {
document.documentElement.classList.add('dark')
} else {
document.documentElement.classList.remove('dark')
}
- localStorage.setItem('darkMode', String(isDark))
+ try {
+ window.localStorage.setItem('darkMode', String(isDark))
+ } catch {
+ // ignore
+ }
}🤖 Prompt for AI Agents
In @ui/src/components/SettingsModal.tsx around lines 9 - 25, getInitialDarkMode
and applyDarkMode can throw in restricted environments because they access
localStorage, document, and matchMedia without guards; wrap all
localStorage.getItem/setItem calls in try/catch and return sensible fallbacks,
and guard document/window usage (e.g., check typeof window !== 'undefined' and
if document exists) before touching classList or matchMedia. Specifically,
update getInitialDarkMode to try reading localStorage and fall back to false or
the result of window.matchMedia only when safe (catch errors from localStorage
and matchMedia), and update applyDarkMode to check for document.documentElement
before modifying classList and to catch errors from localStorage.setItem so
toggling dark mode never throws.
Dark mode improvements: - Replace hardcoded #1a1a1a colors with CSS variable --color-neo-text - Fix text readability in ExpandProjectChat, SpecCreationChat, ChatMessage - Fix FolderBrowser, ConfirmDialog, NewProjectModal text colors - Add explicit text color to TypingIndicator - Fix ProjectSelector selected item text (text-black on yellow) Debug panel: - Remove fixed 600px max height limit - Allow panel to expand up to header (window.innerHeight - 64px) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/styles/globals.css (1)
324-330: Hard-coded border color in.neo-empty-statewill not adapt to dark mode.The dashed border still uses
#1a1a1awhich won't be visible against the dark background.Suggested fix
.neo-empty-state { background-color: var(--color-neo-bg); - border: 4px dashed #1a1a1a; + border: 4px dashed var(--color-neo-border); padding: 2rem; text-align: center; }
🤖 Fix all issues with AI agents
In @ui/src/components/AssistantPanel.tsx:
- Around line 48-50: The main panel container in AssistantPanel.tsx still uses a
hard-coded background class (`bg-white`) which breaks dark mode; locate the
top-level panel div in the AssistantPanel component (the same wrapper that
contains the Bot icon container) and replace the `bg-white` class with
`bg-[var(--color-neo-card)]` so the entire panel uses the theme variable
consistently.
🧹 Nitpick comments (2)
ui/src/components/ProjectSelector.tsx (1)
109-122: Consider using a CSS variable for selected item text color.The hardcoded
text-blackworks well for contrast against the yellow--color-neo-pendingbackground. However, for full theming consistency, consider defining a--color-neo-pending-textvariable. This is a minor suggestion since the current approach ensures good readability.ui/src/styles/globals.css (1)
153-171: Consider using CSS variables for semantic button variants.The primary, success, warning, and danger button variants use hard-coded colors. While this is intentional for brand consistency, in dark mode these may need adjusted text colors for adequate contrast on dark backgrounds.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
ui/src/App.tsxui/src/components/AssistantChat.tsxui/src/components/AssistantPanel.tsxui/src/components/ChatMessage.tsxui/src/components/ConfirmDialog.tsxui/src/components/DebugLogViewer.tsxui/src/components/ExpandProjectChat.tsxui/src/components/FolderBrowser.tsxui/src/components/KanbanColumn.tsxui/src/components/NewProjectModal.tsxui/src/components/ProjectSelector.tsxui/src/components/QuestionOptions.tsxui/src/components/SpecCreationChat.tsxui/src/components/TypingIndicator.tsxui/src/styles/globals.cssui/tsconfig.tsbuildinfo
✅ Files skipped from review due to trivial changes (3)
- ui/src/components/NewProjectModal.tsx
- ui/src/components/FolderBrowser.tsx
- ui/tsconfig.tsbuildinfo
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/src/App.tsx
- ui/src/components/KanbanColumn.tsx
🧰 Additional context used
📓 Path-based instructions (2)
ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
ui/src/**/*.{ts,tsx}: Use TypeScript for React components in the UI (React 18 with TypeScript)
Use React Query (TanStack Query) for API calls and data fetching in the UI
Use Radix UI for component primitives in the React UI
Run ESLint to lint React UI code
Files:
ui/src/components/AssistantChat.tsxui/src/components/AssistantPanel.tsxui/src/components/ProjectSelector.tsxui/src/components/DebugLogViewer.tsxui/src/components/ExpandProjectChat.tsxui/src/components/ChatMessage.tsxui/src/components/QuestionOptions.tsxui/src/components/ConfirmDialog.tsxui/src/components/SpecCreationChat.tsxui/src/components/TypingIndicator.tsx
ui/src/styles/**/*.css
📄 CodeRabbit inference engine (CLAUDE.md)
ui/src/styles/**/*.css: Use Tailwind CSS v4 for styling in React UI, with CSS variables defined in globals.css via @theme directive
React UI uses neobrutalism design system with CSS variables for color tokens: --color-neo-pending (yellow), --color-neo-progress (cyan), --color-neo-done (green)
React UI custom animations: animate-slide-in, animate-pulse-neo, animate-shimmer
Files:
ui/src/styles/globals.css
🧠 Learnings (4)
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/styles/**/*.css : React UI uses neobrutalism design system with CSS variables for color tokens: --color-neo-pending (yellow), --color-neo-progress (cyan), --color-neo-done (green)
Applied to files:
ui/src/components/AssistantPanel.tsxui/src/components/ProjectSelector.tsxui/src/components/ExpandProjectChat.tsxui/src/components/ChatMessage.tsxui/src/components/QuestionOptions.tsxui/src/components/ConfirmDialog.tsxui/src/styles/globals.cssui/src/components/SpecCreationChat.tsxui/src/components/TypingIndicator.tsx
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/**/*.{ts,tsx} : Use TypeScript for React components in the UI (React 18 with TypeScript)
Applied to files:
ui/src/components/ProjectSelector.tsxui/src/components/ExpandProjectChat.tsxui/src/components/TypingIndicator.tsx
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/styles/**/*.css : Use Tailwind CSS v4 for styling in React UI, with CSS variables defined in globals.css via theme directive
Applied to files:
ui/src/components/ConfirmDialog.tsxui/src/styles/globals.css
📚 Learning: 2026-01-10T08:23:04.012Z
Learnt from: CR
Repo: leonvanzyl/autocoder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T08:23:04.012Z
Learning: Applies to ui/src/styles/**/*.css : React UI custom animations: animate-slide-in, animate-pulse-neo, animate-shimmer
Applied to files:
ui/src/styles/globals.cssui/src/components/TypingIndicator.tsx
🔇 Additional comments (22)
ui/src/components/DebugLogViewer.tsx (2)
16-20: LGTM!The constants are well-organized with clear naming. The
HEADER_HEIGHTconstant provides a sensible buffer to prevent the debug panel from overlapping the app header.
56-61: LGTM! Consistent responsive height logic.The clamping logic is correctly applied in both the initial state and the resize handler, ensuring the panel respects the header space. The SSR safety check in the initializer is appropriate.
One edge case to consider: if the browser window shrinks while the panel is open, the panel height won't automatically adjust until the user drags the resize handle. This is minor since it self-corrects on interaction, but you could optionally add a window resize listener to re-clamp
panelHeightif strict viewport compliance is needed.Also applies to: 188-193
ui/src/components/QuestionOptions.tsx (1)
96-96: Consistent theming updates for dark mode support.The CSS variable changes align well with the neobrutalism design system tokens. The pattern of using
--color-neo-cardfor backgrounds and--color-neo-donefor selected indicators is consistently applied across both regular options and the "Other" option.Also applies to: 130-130, 143-143, 173-173, 185-185
ui/src/components/TypingIndicator.tsx (1)
25-27: LGTM!Adding
text-[var(--color-neo-text)]ensures the "Claude is thinking..." text respects the theme, consistent with the dark mode updates across other components.ui/src/components/ConfirmDialog.tsx (1)
63-65: LGTM!The title text color now uses the theme variable, ensuring consistency with dark mode across the application.
ui/src/components/AssistantChat.tsx (1)
134-134: LGTM!The input area background now uses the themed card color variable, ensuring visual consistency with other card elements and proper dark mode support.
ui/src/components/ProjectSelector.tsx (1)
68-68: LGTM!The dropdown trigger button now uses themed colors for both background and text, aligning with the dark mode theming updates.
ui/src/components/ChatMessage.tsx (3)
36-40: LGTM!The assistant message styling correctly uses
bg-[var(--color-neo-card)]for theme-aware backgrounds.
107-107: LGTM!Text color properly uses
var(--color-neo-text)for theme consistency.
147-148: LGTM!Attachment container styling correctly updated to use themed card background.
ui/src/styles/globals.css (3)
63-80: Dark mode theme implementation looks solid.The dark mode color palette is well-designed with appropriate contrast inversions. Status colors (pending, progress, done) correctly remain unchanged for consistency, while background/text/border colors properly invert for readability.
128-134: LGTM!Button base styles correctly use CSS variables for text, background, and border colors, enabling proper dark mode support.
501-512: LGTM!Scrollbar styling properly uses CSS variables for theme-aware appearance.
ui/src/components/SpecCreationChat.tsx (4)
210-214: LGTM!Header styling correctly updated to use themed card background and text color variables.
253-256: LGTM!Error banner close button hover effect properly uses the themed card color with opacity.
307-309: LGTM!Input area container correctly uses themed card background.
435-439: LGTM!Retry button and YOLO toggle inactive state correctly use themed card background for consistency.
Also applies to: 446-448
ui/src/components/ExpandProjectChat.tsx (5)
187-191: LGTM!Header styling correctly updated to use themed card background and text color, consistent with SpecCreationChat.
226-229: LGTM!Error banner close button hover properly themed.
271-273: LGTM!Input area container uses themed background.
281-281: LGTM!Attachment preview container uses themed card background, matching the pattern in SpecCreationChat.
372-375: LGTM!Close button in completion footer correctly uses themed card background.
| <div className="bg-[var(--color-neo-card)] border-2 border-[var(--color-neo-border)] p-1.5 shadow-[2px_2px_0px_rgba(0,0,0,1)]"> | ||
| <Bot size={18} /> | ||
| </div> |
There was a problem hiding this comment.
Theming update looks good, but the panel background on line 34 is still hard-coded.
The icon container correctly uses bg-[var(--color-neo-card)] for theme support. However, the main panel container on line 34 still uses bg-white, which will break dark mode consistency.
Suggested fix for panel background
bg-white
+ bg-[var(--color-neo-card)]At line 34, replace bg-white with bg-[var(--color-neo-card)] to ensure the entire panel respects the theme.
🤖 Prompt for AI Agents
In @ui/src/components/AssistantPanel.tsx around lines 48 - 50, The main panel
container in AssistantPanel.tsx still uses a hard-coded background class
(`bg-white`) which breaks dark mode; locate the top-level panel div in the
AssistantPanel component (the same wrapper that contains the Bot icon container)
and replace the `bg-white` class with `bg-[var(--color-neo-card)]` so the entire
panel uses the theme variable consistently.
- Add isUnmountingRef to prevent WebSocket reconnection during React StrictMode unmount/remount cycle - Add deduplication for consecutive identical log lines - Reset dedup refs when clearing logs or switching projects Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ui/src/hooks/useWebSocket.ts (2)
84-95: Consecutive deduplication may suppress legitimate repeated messages.The deduplication logic only compares against the immediate previous line. While this prevents log spam from repeated identical messages, it could also suppress legitimate consecutive duplicates (e.g., repeated error messages or status updates that genuinely occur twice).
Consider whether this trade-off is acceptable for your use case, or if you need a more sophisticated approach (e.g., time-based throttling or allowing duplicates after a minimum interval).
163-206: Cleanup logic is thorough; minor redundancy in dependencies.The cleanup properly handles all resources: the ping interval, pending reconnection timeout, and the WebSocket itself. The unmount flag management is correctly sequenced.
Note: Including both
projectNameandconnectin the dependency array is slightly redundant sinceconnectalready depends onprojectName. This is harmless but could be simplified if you prefer minimal dependencies.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/src/hooks/useWebSocket.ts
🧰 Additional context used
📓 Path-based instructions (1)
ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
ui/src/**/*.{ts,tsx}: Use TypeScript for React components in the UI (React 18 with TypeScript)
Use React Query (TanStack Query) for API calls and data fetching in the UI
Use Radix UI for component primitives in the React UI
Run ESLint to lint React UI code
Files:
ui/src/hooks/useWebSocket.ts
🧬 Code graph analysis (1)
ui/src/hooks/useWebSocket.ts (1)
server/websocket.py (1)
connect(59-66)
🔇 Additional comments (4)
ui/src/hooks/useWebSocket.ts (4)
39-41: LGTM! Appropriate use of refs for tracking state that shouldn't trigger re-renders.Using refs for the unmounting flag and deduplication tracking is the correct approach since these values need to persist across renders but don't need to trigger UI updates.
101-112: Consistent with log deduplication pattern.The dev log deduplication follows the same pattern as regular logs. The implementation is consistent and maintains separate dedup state appropriately.
131-145: LGTM! Proper guard against reconnection during cleanup.The unmount guard correctly prevents unnecessary reconnection attempts when the component is cleaning up. The timing is also correct: the cleanup sets
isUnmountingRef.current = truebefore closing the WebSocket, so whenonclosefires, the reconnection is properly skipped.
209-218: LGTM! Correct reset of dedup refs when clearing logs.Resetting the deduplication refs alongside clearing the log arrays ensures that subsequent messages won't be incorrectly filtered against stale state.
- Make LAN access opt-in via AUTOCODER_LAN_ACCESS env var (default: disabled) - Fix CORS: use wildcard with credentials=False, or specific origins with credentials=True - Re-enable localhost middleware when LAN access is disabled (secure by default) - Fix AgentThought idle timeout: add interval to update time when paused - Fix hardcoded path in create-spec.md (use relative path) - Add try/catch to initDarkMode to prevent crash on localStorage errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Motivation
Allows remote development workflows where the AutoCoder server runs on one machine (e.g., a development server or container) and is accessed from other machines on the LAN.
Changes
server/main.py: CORS now allows all origins, localhost middleware commented out, host changed to 0.0.0.0start_ui.py: Socket binding and uvicorn host changed to 0.0.0.0 for both dev and production modesSecurity Note
This reduces security by allowing any machine on the network to access the UI. Should only be used on trusted networks.
Test plan
./start_ui.shorpython start_ui.pyhttp://<server-ip>:8888🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Security
Style
✏️ Tip: You can customize this high-level summary in your review settings.