Skip to content

Fix #609: Windows coding phase not starting after spec/planning #1347

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.

Sign up for GitHub

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

Merged
AndyMik90 merged 7 commits into from
Jan 20, 2026
Merged

Fix #609: Windows coding phase not starting after spec/planning #1347

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5f23e07
Fix #609: Windows coding phase not starting after spec/planning
TamerineSky Jan 6, 2026
4aa30de
Add exception handling for Windows subprocess.run()
TamerineSky Jan 6, 2026
77406c3
Fix linting: remove f-string without placeholders
TamerineSky Jan 6, 2026
d33993f
refactor: use is_windows() from core.platform for consistency
TamerineSky Jan 19, 2026
efafdca
Address review feedback for consistency
TamerineSky Jan 19, 2026
18bcb83
Fix Ruff formatting: wrap long lines
TamerineSky Jan 19, 2026
309433f
Merge branch 'develop' into fix/windows-coding-phase-609
TamerineSky Jan 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions apps/backend/runners/spec_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import asyncio
import io
import os
import subprocess
from pathlib import Path

# Configure safe encoding on Windows BEFORE any imports that might print
Expand Down Expand Up @@ -104,6 +105,7 @@

init_sentry(component="spec-runner")

from core.platform import is_windows
from debug import debug, debug_error, debug_section, debug_success
from phase_config import resolve_model_id
from review import ReviewState
Expand Down Expand Up @@ -369,8 +371,27 @@ def main():
print(f" {muted('Running:')} {' '.join(run_cmd)}")
print()

# Execute run.py - replace current process
os.execv(sys.executable, run_cmd)
# Execute run.py - use subprocess on Windows to maintain connection with Electron
# Fix for issue #609: os.execv() breaks connection on Windows
if is_windows():
try:
result = subprocess.run(run_cmd)
sys.exit(result.returncode)
except FileNotFoundError:
debug_error("spec_runner", "Could not start coding phase - executable not found")
print_status("Could not start coding phase - executable not found", "error")
sys.exit(1)
except OSError as e:
debug_error("spec_runner", f"Error starting coding phase: {e}")
print_status(f"Error starting coding phase: {e}", "error")
sys.exit(1)
except KeyboardInterrupt:
debug_error("spec_runner", "Coding phase interrupted by user")
print("\n\nCoding phase interrupted.")
sys.exit(1)
Comment on lines +393 to +396
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use print_status for KeyboardInterrupt to keep UI formatting consistent.
Minor UX consistency issue: other error paths here use print_status, but the interrupt path uses raw print.

Proposed fix 
-                except KeyboardInterrupt:
-                    debug_error("spec_runner", "Coding phase interrupted by user")
-                    print("\n\nCoding phase interrupted.")
-                    sys.exit(1)
+                except KeyboardInterrupt:
+                    debug_error("spec_runner", "Coding phase interrupted by user")
+                    print_status("Coding phase interrupted.", "warning")
+                    sys.exit(1)
🤖 Prompt for AI Agents 
In `@apps/backend/runners/spec_runner.py` around lines 393 - 396, The
KeyboardInterrupt handler in spec_runner currently calls print directly; change
it to use the existing print_status UI helper to keep formatting consistent:
inside the except KeyboardInterrupt block (where debug_error("spec_runner",
"Coding phase interrupted by user") and sys.exit(1) are used), replace the raw
print("\n\nCoding phase interrupted.") with a call to print_status(... ) that
outputs the same message (e.g., "Coding phase interrupted.") so the interrupt
path matches other error paths' UI formatting.

Copy link
Contributor Author

@TamerineSky TamerineSky Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing KeyboardInterrupt handler in the same file (line 403-406) also uses print() rather than print_status():

except KeyboardInterrupt:
    debug_error("spec_runner", "Spec creation interrupted by user")
    print("\n\nSpec creation interrupted.")
    print(...)

The new handler follows the same pattern for consistency with the existing code. Changing this one would create inconsistency - if we want to update to print_status(), both handlers should be updated together in a separate PR.

Copy link
Contributor

@coderabbitai coderabbitai bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TamerineSky, you're absolutely right! I verified the existing handler at lines 403-407 and it does use print() directly. Internal consistency within the file is more important here, and your new code correctly follows the established pattern.

If you'd like to update both KeyboardInterrupt handlers to use print_status() for better UI consistency across the codebase, that could be done as a separate cleanup PR, but it's definitely not necessary for this fix.

Thanks for catching that! 👍

🧠 Learnings used 
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.702Z
Learning: When fixing platform-specific bugs, ensure the fix doesn't break other platforms and rely on multi-platform CI to validate (Windows, macOS, Linux)

else:
# On Unix/macOS, os.execv() works correctly - replaces current process
os.execv(sys.executable, run_cmd)
Comment on lines 374 to 399
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Please verify Windows/macOS/Linux CI coverage for this platform split.
This change is OS-specific; confirm all three OS jobs pass to avoid regressions. Based on learnings, ...

🧰 Tools
🪛 GitHub Actions: Lint

[error] 378-385: ruff format check failed. 1 file would be reformatted (apps/backend/). Please run 'ruff format apps/backend/' to fix code style issues.

🤖 Prompt for AI Agents 
In `@apps/backend/runners/spec_runner.py` around lines 374 - 394, The PR
introduces an OS-specific code path in spec_runner (the is_windows() branch
using subprocess.run and the else branch using os.execv), so ensure CI
runs/tests cover Windows, macOS, and Linux: add or enable platform-specific CI
jobs that execute the spec_runner run path (invoking run_cmd) and validate exit
codes/behavior, and add tests or test-matrix entries that exercise
subprocess.run and os.execv paths (including FileNotFoundError, OSError and
KeyboardInterrupt handling) so regressions across platforms are caught; update
CI configuration and test harness to run these jobs and surface failures for the
spec_runner platform split.


sys.exit(0)

Expand Down
Loading