feat: add Create Spec button and fix Windows asyncio subprocess#78
feat: add Create Spec button and fix Windows asyncio subprocess#78leonvanzyl merged 2 commits intoAutoForgeAI:masterfrom
Conversation
UI Changes: - Add "Create Spec with AI" button in empty kanban when project has no spec - Button opens SpecCreationChat to guide users through spec creation - Shows in Pending column when has_spec=false and no features exist Windows Fixes: - Fix asyncio subprocess NotImplementedError on Windows - Set WindowsProactorEventLoopPolicy in server/__init__.py - Remove --reload from uvicorn (incompatible with Windows subprocess) - Add process cleanup on startup in start_ui.bat Spec Chat Improvements: - Enable full tool access (remove allowed_tools restriction) - Add "user" to setting_sources for global skills access - Use bypassPermissions mode for auto-approval - Add WebFetch/WebSearch auto-approve hook Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces automatic approval for web tools (WebFetch/WebSearch) via a new security hook, fixes Windows asyncio subprocess support across multiple entry points, broadens chat session tool/skill scope and permission handling, and adds a Spec Creation UI flow with integrated chat components and conditional Kanban rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PreToolUseMatcher as Matcher
participant SecurityHook as web_tools_auto_approve_hook
participant ToolExecutor as Tool (WebFetch/WebSearch)
Client->>Matcher: Request to use tool (tool_name)
Matcher->>SecurityHook: match for WebFetch/WebSearch
SecurityHook-->>Matcher: return {} (auto-approve)
Matcher->>ToolExecutor: invoke tool with approved permissions
ToolExecutor-->>Client: tool result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@start_ui.bat`:
- Around line 12-16: The script indiscriminately force-kills any PID listening
on port 8888 using the for loop and taskkill, which can terminate unrelated
services; update the block around the for /f (...) loop and taskkill usage to
(1) resolve the PID(s) for port 8888, (2) for each PID call tasklist /FI "PID eq
<pid>" or equivalent to retrieve the Image Name, (3) prompt the user with the
PID and process name and require confirmation before calling taskkill, and/or
add an explicit whitelist of expected process names to auto-kill; ensure the
flow handles multiple PIDs and aborts safely if the user declines or no
whitelist match is found.
In `@start_ui.py`:
- Around line 189-207: The docstring and printed message in
start_production_server are misleading because the code launches uvicorn without
--reload (so there is no hot reload) and the comment about PYTHONASYNCIODEBUG
doesn't match since the env copy never sets that variable; update the
start_production_server function to (1) change the docstring and the print() to
state that the server is started in production mode without hot reload, and (2)
either remove/adjust the PYTHONASYNCIODEBUG comment or actually set
env["PYTHONASYNCIODEBUG"] = "1" before passing env into subprocess.Popen
(referencing start_production_server, venv_python, env, and the subprocess.Popen
call to locate the code).
🧹 Nitpick comments (3)
ui/src/components/KanbanColumn.tsx (1)
88-97: Add explicit button type for safety.Optional, but avoids accidental form submission if this component ever renders inside a
<form>.♻️ Proposed tweak
- <button + <button + type="button" onClick={onCreateSpec} className="neo-btn neo-btn-primary inline-flex items-center gap-2" >ui/src/App.tsx (1)
452-466: Add modal accessibility attributes to the overlay container.This improves screen-reader handling for the full-screen modal.
♿ Proposed accessibility tweak
- {showSpecChat && selectedProject && ( - <div className="fixed inset-0 z-50 bg-[var(--color-neo-bg)]"> + {showSpecChat && selectedProject && ( + <div + className="fixed inset-0 z-50 bg-[var(--color-neo-bg)]" + role="dialog" + aria-modal="true" + aria-label="Spec creation" + >server/services/spec_chat_session.py (1)
182-185: Clarify the security and architectural intent for unrestricted tool access in spec_chat_session.The code intentionally enables full tool access via
setting_sources=["project", "user"]andpermission_mode="bypassPermissions"with noallowed_toolsconstraint, as documented in the code comment. However, there is no documented rationale for why spec generation requires broader access than the read-only assistant service (which explicitly restricts tools). Consider adding a comment explaining the business requirement for unrestricted access, or implement environment-based gating (as suggested in the original diff) if this scope should be toggleable.
| REM Kill any existing processes on port 8888 | ||
| echo Cleaning up old processes... | ||
| for /f "tokens=5" %%a in ('netstat -aon ^| findstr ":8888" ^| findstr "LISTENING"') do ( | ||
| taskkill /F /PID %%a >nul 2>&1 | ||
| ) |
There was a problem hiding this comment.
Avoid killing unrelated processes on port 8888.
The current block force-kills any listener on 8888, which can terminate unrelated services. Consider prompting the user (or filtering to expected process names) before killing.
🐛 Proposed safer prompt before killing
-REM Kill any existing processes on port 8888
-echo Cleaning up old processes...
-for /f "tokens=5" %%a in ('netstat -aon ^| findstr ":8888" ^| findstr "LISTENING"') do (
- taskkill /F /PID %%a >nul 2>&1
-)
+REM Kill any existing processes on port 8888 (with confirmation)
+echo Checking for processes on port 8888...
+for /f "tokens=5" %%a in ('netstat -aon ^| findstr ":8888" ^| findstr "LISTENING"') do (
+ echo Found PID %%a listening on 8888.
+ choice /M "Terminate PID %%a?"
+ if errorlevel 2 (
+ echo Skipping PID %%a
+ ) else (
+ taskkill /F /PID %%a >nul 2>&1
+ )
+)🤖 Prompt for AI Agents
In `@start_ui.bat` around lines 12 - 16, The script indiscriminately force-kills
any PID listening on port 8888 using the for loop and taskkill, which can
terminate unrelated services; update the block around the for /f (...) loop and
taskkill usage to (1) resolve the PID(s) for port 8888, (2) for each PID call
tasklist /FI "PID eq <pid>" or equivalent to retrieve the Image Name, (3) prompt
the user with the PID and process name and require confirmation before calling
taskkill, and/or add an explicit whitelist of expected process names to
auto-kill; ensure the flow handles multiple PIDs and aborts safely if the user
declines or no whitelist match is found.
| def start_production_server(port: int): | ||
| """Start FastAPI server in production mode.""" | ||
| """Start FastAPI server in production mode with hot reload.""" | ||
| venv_python = get_venv_python() | ||
|
|
||
| print(f"\n Starting server at http://127.0.0.1:{port}") | ||
| print(f"\n Starting server at http://127.0.0.1:{port} (with hot reload)") | ||
|
|
||
| # Set PYTHONASYNCIODEBUG to help with Windows subprocess issues | ||
| env = os.environ.copy() | ||
|
|
||
| # NOTE: --reload is NOT used because on Windows it breaks asyncio subprocess | ||
| # support (uvicorn's reload worker doesn't inherit the ProactorEventLoop policy). | ||
| # This affects Claude SDK which uses asyncio.create_subprocess_exec. | ||
| # For development with hot reload, use: python start_ui.py --dev | ||
| return subprocess.Popen([ | ||
| str(venv_python), "-m", "uvicorn", | ||
| "server.main:app", | ||
| "--host", "127.0.0.1", | ||
| "--port", str(port) | ||
| ], cwd=str(ROOT)) | ||
| "--port", str(port), | ||
| ], cwd=str(ROOT), env=env) |
There was a problem hiding this comment.
Fix misleading hot‑reload messaging and env comment.
--reload is explicitly not used, so “with hot reload” in the docstring/log is inaccurate, and the PYTHONASYNCIODEBUG comment doesn’t match the code.
📝 Suggested cleanup
-def start_production_server(port: int):
- """Start FastAPI server in production mode with hot reload."""
+def start_production_server(port: int):
+ """Start FastAPI server in production mode."""
@@
- print(f"\n Starting server at http://127.0.0.1:{port} (with hot reload)")
+ print(f"\n Starting server at http://127.0.0.1:{port}")
@@
- # Set PYTHONASYNCIODEBUG to help with Windows subprocess issues
env = os.environ.copy()
+ # Optional: enable asyncio debug when troubleshooting Windows subprocess issues
+ # env["PYTHONASYNCIODEBUG"] = "1"🤖 Prompt for AI Agents
In `@start_ui.py` around lines 189 - 207, The docstring and printed message in
start_production_server are misleading because the code launches uvicorn without
--reload (so there is no hot reload) and the comment about PYTHONASYNCIODEBUG
doesn't match since the env copy never sets that variable; update the
start_production_server function to (1) change the docstring and the print() to
state that the server is started in production mode without hot reload, and (2)
either remove/adjust the PYTHONASYNCIODEBUG comment or actually set
env["PYTHONASYNCIODEBUG"] = "1" before passing env into subprocess.Popen
(referencing start_production_server, venv_python, env, and the subprocess.Popen
call to locate the code).
- Verified kanban board renders 149 features successfully - Scroll performance: 0.10ms (well under 100ms threshold) - Tested smooth scrolling from top to bottom and back - No JavaScript errors (only non-critical favicon 404) - Screenshots captured at each verification step Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thank you! |
Security fixes to restore defense-in-depth after merging PR #78: **client.py:** - Revert permission mode from "bypassPermissions" to "acceptEdits" - Remove redundant web_tools_auto_approve_hook from PreToolUse hooks - Remove unused import of web_tools_auto_approve_hook **security.py:** - Remove web_tools_auto_approve_hook function (was redundant and returned {} for ALL tools, not just WebFetch/WebSearch) **server/services/spec_chat_session.py:** - Restore allowed_tools restriction: [Read, Write, Edit, Glob, WebFetch, WebSearch] - Revert permission mode from "bypassPermissions" to "acceptEdits" - Keeps setting_sources=["project", "user"] for global skills access **ui/src/components/AgentAvatar.tsx:** - Remove unused getMascotName export to fix React Fast Refresh warning - File now only exports AgentAvatar component as expected The bypassPermissions mode combined with unrestricted tool access in spec_chat_session.py created a security gap where Bash commands could execute without validation (sandbox disabled, no bash_security_hook). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds comprehensive validation of AgentSpecs before kernel execution: 1. Validates required fields (name, display_name, objective, task_type, tool_policy) 2. Validates tool_policy structure: - allowed_tools must be a non-empty list of strings - forbidden_patterns must be valid regexes - tool_hints must be a dict if present 3. Validates budget constraints: - max_turns: 1-500 - timeout_seconds: 60-7200 4. Returns detailed validation errors without creating AgentRun 5. New Pydantic schemas: ValidationErrorItem, SpecValidationErrorResponse 6. 97 comprehensive tests covering all validation scenarios Files changed: - api/__init__.py: Export spec_validator functions and constants - server/routers/agent_specs.py: Integrate validation in execute endpoint - server/schemas/agentspec.py: Add validation error response schemas - tests/test_feature_78_spec_validation.py: Unit tests for spec_validator - tests/test_feature_78_api_integration.py: API integration tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Changes
UI:
has_spec=falseand no featuresWindows Fixes:
WindowsProactorEventLoopPolicyinserver/__init__.py--reloadfrom uvicorn (breaks Windows subprocess)start_ui.batSpec Chat:
allowed_toolsrestriction for full tool access"user"tosetting_sourcesfor global skillsWebFetch/WebSearchauto-approve hookTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
UX
✏️ Tip: You can customize this high-level summary in your review settings.