-
Notifications
You must be signed in to change notification settings - Fork 309
fix: prevent agent subprocess blocking on Windows #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Add stdin=subprocess.DEVNULL to prevent blocking on stdin reads - Add CREATE_NO_WINDOW flag on Windows to prevent console pop-ups - Remove trailing pause from start_ui.bat Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Resolved conflicts by combining: - stdin=DEVNULL and CREATE_NO_WINDOW (blocking fix) - PYTHONUNBUFFERED env var (output buffering fix) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughReplaces direct subprocess Popen calls with explicit popen_kwargs, adds stdin=DEVNULL and Windows CREATE_NO_WINDOW handling, ensures unbuffered Python output, introduces child-process-tree cleanup and feature in_progress reset on spawn failure, and removes a final pause from the UI batch script. (50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
parallel_orchestrator.py (2)
519-527: Inconsistent: Missing Windows subprocess handling in testing agent spawner.This
Popencall lacksstdin=subprocess.DEVNULLand the WindowsCREATE_NO_WINDOWflag that were added to_spawn_coding_agent. Testing agents could still block on stdin or show console pop-ups on Windows.🔧 Proposed fix to align with _spawn_coding_agent
try: - proc = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - text=True, - cwd=str(AUTOCODER_ROOT), - env={**os.environ, "PYTHONUNBUFFERED": "1"}, - ) + popen_kwargs = { + "stdin": subprocess.DEVNULL, + "stdout": subprocess.PIPE, + "stderr": subprocess.STDOUT, + "text": True, + "cwd": str(AUTOCODER_ROOT), + "env": {**os.environ, "PYTHONUNBUFFERED": "1"}, + } + if sys.platform == "win32": + popen_kwargs["creationflags"] = subprocess.CREATE_NO_WINDOW + + proc = subprocess.Popen(cmd, **popen_kwargs)
570-577: Inconsistent: Missing Windows subprocess handling in initializer.Same issue as
_spawn_testing_agent— thisPopencall is missingstdin=subprocess.DEVNULLand the WindowsCREATE_NO_WINDOWflag.🔧 Proposed fix to align with _spawn_coding_agent
- proc = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - text=True, - cwd=str(AUTOCODER_ROOT), - env={**os.environ, "PYTHONUNBUFFERED": "1"}, - ) + popen_kwargs = { + "stdin": subprocess.DEVNULL, + "stdout": subprocess.PIPE, + "stderr": subprocess.STDOUT, + "text": True, + "cwd": str(AUTOCODER_ROOT), + "env": {**os.environ, "PYTHONUNBUFFERED": "1"}, + } + if sys.platform == "win32": + popen_kwargs["creationflags"] = subprocess.CREATE_NO_WINDOW + + proc = subprocess.Popen(cmd, **popen_kwargs)
Added _kill_process_tree call in _read_output finally block to ensure child processes (Claude CLI) are cleaned up when agents complete or fail. This prevents accumulation of zombie processes that was causing 78+ Python processes when max concurrency was set to 5. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… (Feature leonvanzyl#89) - Created test_validate_dependency_graph_missing_targets.py with 21 tests - Created verify_feature_89.py standalone verification script - All 4 verification steps pass: 1. Create feature with non-existent dependency 2. Call validate_dependency_graph() 3. Verify missing_targets dict contains {1: [999]} 4. Verify structured ValidationResult with all issue types Tests cover: - Single and multiple missing targets - Mixed valid and missing dependencies - Missing target issue structure and auto_fixable flag - Edge cases (empty list, None deps, negative IDs) - Combinations with self-references and cycles Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Feature leonvanzyl#89: Missing dependency target detection - PASSING - All 4 verification steps pass - 21 comprehensive tests pass - Current progress: 30/103 features (29.1%) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
SUMMARY:
Fixed TypeScript build error in ProjectSetupRequired.tsx where startAgent
was being called with a boolean instead of an options object.
DETAILS:
- The startAgent API function signature was updated (in previous PR merges)
to accept an options object: { yoloMode?, parallelMode?, maxConcurrency?, testingAgentRatio? }
- ProjectSetupRequired.tsx was still calling it with the old signature:
startAgent(projectName, yoloMode) - passing boolean directly
- Changed to: startAgent(projectName, { yoloMode }) - wrapping in options object
This was the only remaining build error after merging 13+ PRs from upstream:
- PR leonvanzyl#112: Security vulnerabilities and race conditions
- PR leonvanzyl#89: Windows subprocess blocking fix
- PR leonvanzyl#109: Rate limit handling with exponential backoff
- PR leonvanzyl#88: MCP server config for ExpandChatSession
- PR leonvanzyl#100: Diagnostic warnings for config loading
- PR leonvanzyl#110: Quality gates (quality_gates.py)
- PR leonvanzyl#113: Structured logging (structured_logging.py)
- PR leonvanzyl#48: Knowledge files support (API, schemas, prompts)
- PR leonvanzyl#29: Feature editing/deletion (MCP tools)
- PR leonvanzyl#45: Chat persistence
- PR leonvanzyl#52: Refactoring feature guidance
- PR leonvanzyl#4: Project reset functionality
- PR leonvanzyl#95: UI polish, health checks, cross-platform fixes
Build now passes: npm run build succeeds with all 2245 modules transformed.
Summary
stdin=subprocess.DEVNULLto prevent blocking on stdin readsCREATE_NO_WINDOWflag on Windows to prevent console pop-upspausefrom start_ui.batPYTHONUNBUFFEREDenv var for immediate outputProblem
On Windows, the agent subprocess would block with "Press any key to continue" message, requiring manual restart.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.