-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: handle empty/greenfield projects in spec creation (#1426) #1841
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: develop
Are you sure you want to change the base?
Changes from 2 commits
c393d60
09858dd
cdd8c31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| """ | ||
|
|
||
| import json | ||
| import types | ||
| from collections.abc import Callable | ||
| from pathlib import Path | ||
|
|
||
|
|
@@ -18,6 +19,7 @@ | |
| from task_logger import ( | ||
| LogEntryType, | ||
| LogPhase, | ||
| TaskLogger, | ||
| get_task_logger, | ||
| ) | ||
| from ui import ( | ||
|
|
@@ -238,6 +240,41 @@ async def run(self, interactive: bool = True, auto_approve: bool = False) -> boo | |
| task_logger.start_phase(LogPhase.PLANNING, "Starting spec creation process") | ||
| TaskEventEmitter.from_spec_dir(self.spec_dir).emit("PLANNING_STARTED") | ||
|
|
||
| # Track whether we've already ended the planning phase (to avoid double-end) | ||
| self._planning_phase_ended = False | ||
|
|
||
| try: | ||
| return await self._run_phases(interactive, auto_approve, task_logger, ui) | ||
| except Exception as e: | ||
| # Ensure planning phase is always properly ended on unexpected errors | ||
| # This prevents the task from being stuck in "active" planning state | ||
| if not self._planning_phase_ended: | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, | ||
| success=False, | ||
| message=f"Spec creation failed with unexpected error: {e}", | ||
| ) | ||
| raise | ||
|
Comment on lines
243
to
258
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The To fix this,
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| async def _run_phases( | ||
| self, | ||
| interactive: bool, | ||
| auto_approve: bool, | ||
| task_logger: TaskLogger, | ||
| ui: types.ModuleType, | ||
| ) -> bool: | ||
| """Execute all spec creation phases. | ||
|
|
||
| Args: | ||
| interactive: Whether to run in interactive mode | ||
| auto_approve: Whether to skip human review | ||
| task_logger: The task logger instance | ||
| ui: The UI module | ||
|
|
||
| Returns: | ||
| True if spec creation and review completed successfully | ||
|
Comment on lines
+271
to
+276
This comment was marked as outdated.
Sorry, something went wrong. |
||
| """ | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| print( | ||
| box( | ||
| f"Spec Directory: {self.spec_dir}\n" | ||
|
|
@@ -291,6 +328,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| results.append(result) | ||
| if not result.success: | ||
| print_status("Discovery failed", "error") | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, success=False, message="Discovery failed" | ||
| ) | ||
|
|
@@ -305,6 +343,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| results.append(result) | ||
| if not result.success: | ||
| print_status("Requirements gathering failed", "error") | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, | ||
| success=False, | ||
|
|
@@ -335,6 +374,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| results.append(result) | ||
| if not result.success: | ||
| print_status("Complexity assessment failed", "error") | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, success=False, message="Complexity assessment failed" | ||
| ) | ||
|
|
@@ -396,6 +436,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| f"Phase '{phase_name}' failed: {'; '.join(result.errors)}", | ||
| LogEntryType.ERROR, | ||
| ) | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, | ||
| success=False, | ||
|
|
@@ -407,6 +448,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| self._print_completion_summary(results, phases_executed) | ||
|
|
||
| # End planning phase successfully | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, success=True, message="Spec creation complete" | ||
| ) | ||
|
|
@@ -661,9 +703,8 @@ def _run_review_checkpoint(self, auto_approve: bool) -> bool: | |
| print_status("Build will not proceed without approval.", "warning") | ||
| return False | ||
|
|
||
| except SystemExit as e: | ||
| if e.code != 0: | ||
| return False | ||
| except SystemExit: | ||
| # Review checkpoint may call sys.exit(); treat any exit as unapproved | ||
| return False | ||
| except KeyboardInterrupt: | ||
| print() | ||
|
|
||
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.
🧹 Nitpick | 🔵 Trivial
Good fix for the false-positive greenfield detection on missing/corrupt index.
The
if not stats: return Falseguard correctly prevents misclassifying projects whenget_project_index_statsreturns{}due to a missing or unparsable index file.One remaining edge case: if
project_index.jsonexists and parses as valid JSON but uses an unrecognized format (no"files"or"services"key),get_project_index_statsreturns{"file_count": 0, ...}— a non-empty dict that passes the guard and yieldsfile_count == 0 → True. This would be a false positive on a valid-but-unrecognized index format. Consider additionally checking that"file_count"is actually present instatsas a key (rather than relying on the default):Proposed hardening
def _is_greenfield_project(spec_dir: Path) -> bool: """Check if the project is empty/greenfield (0 discovered files).""" stats = get_project_index_stats(spec_dir) if not stats: return False # Can't determine - don't assume greenfield - return stats.get("file_count", 0) == 0 + if "file_count" not in stats: + return False # Unrecognized format - don't assume greenfield + return stats["file_count"] == 0🤖 Prompt for AI Agents