Skip to content

Fix human_review status bugs (#1149, #509) #1425

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

Closed
elmateo487 wants to merge 2 commits into from
Closed

Fix human_review status bugs (#1149, #509) #1425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df2edfa
Fix human_review status bugs (#1149, #509)
Jan 22, 2026
3afbfab
Fix planning failure incorrectly setting human_review status
Jan 22, 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
18 changes: 18 additions & 0 deletions apps/backend/agents/coder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,24 @@ def _validate_and_fix_implementation_plan() -> tuple[bool, list[str]]:
print(f"Continuing build: {highlight(spec_dir.name)}")
print_progress_summary(spec_dir)

# Fix for #509: Transition from human_review to in_progress after approval
# When continuing a build that was approved, ensure we transition out of human_review
plan = load_implementation_plan(spec_dir)
if plan and plan.status == "human_review" and plan.planStatus == "review":
from review.state import ReviewState
review_state = ReviewState.load(spec_dir)
if review_state.is_approval_valid(spec_dir):
# User approved the plan - transition to in_progress now that execution begins
plan.status = "in_progress"
plan.planStatus = "in_progress"
plan_file = spec_dir / "implementation_plan.json"
plan.save(plan_file)
Comment on lines +195 to +204

This comment was marked as outdated.

Sign in to view

print_status("Transitioned from human_review to in_progress after approval", "success")
Comment on lines +195 to +205
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

plan.save() recomputes status and can undo the in_progress transition.

ImplementationPlan.save() calls update_status_from_subtasks(), which sets backlog/pending when all subtasks are still pending (common right after approval). That means this transition likely won’t persist to disk. Consider preserving explicit in_progress in the “all pending” branch or persisting without recomputing status.

🐛 One possible fix (apps/backend/implementation_plan/plan.py) 
-            if self.status == "human_review" and self.planStatus == "review":
-                pass
-            else:
-                self.status = "backlog"
-                self.planStatus = "pending"
+            if self.status == "human_review" and self.planStatus == "review":
+                pass
+            elif self.status == "in_progress" and self.planStatus == "in_progress":
+                # Preserve explicit transition after approval
+                pass
+            else:
+                self.status = "backlog"
+                self.planStatus = "pending"
🧰 Tools
🪛 GitHub Actions: Lint

[error] 195-195: Ruff format check detected formatting changes. 2 files would be reformatted. Run 'ruff format' to fix formatting.

🤖 Prompt for AI Agents 
In `@apps/backend/agents/coder.py` around lines 195 - 205, The transition to
in_progress is being overwritten because ImplementationPlan.save() calls
update_status_from_subtasks() which recomputes status; modify the flow so the
explicit in_progress transition persists: either add a
save(preserve_status=True) (or a persist_without_recompute()/write_raw(path)) to
ImplementationPlan and call that here after setting plan.status and
plan.planStatus, or update ImplementationPlan.save to accept an option to skip
update_status_from_subtasks; then use that new API in the block that handles
approval (the code that calls load_implementation_plan, ReviewState.load,
review_state.is_approval_valid) so the in_progress value is written to disk
without being recomputed away.

else:
# Plan changed after approval or not approved - stay in human_review
print_status("Plan requires re-approval before execution can continue", "warning")
return
Comment on lines +195 to +209
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 22, 2026

Choose a reason for hiding this comment

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

critical

This block has a bug: load_implementation_plan from agents.utils returns a dictionary, but the code attempts to access attributes (.status) and methods (.save()) as if plan were an object. This will raise an AttributeError and cause a crash.

The logic should be updated to correctly load an ImplementationPlan object instance to allow for attribute access and use of the .save() method.

Suggested change
plan = load_implementation_plan(spec_dir)
if plan and plan.status == "human_review" and plan.planStatus == "review":
from review.state import ReviewState
review_state = ReviewState.load(spec_dir)
if review_state.is_approval_valid(spec_dir):
# User approved the plan - transition to in_progress now that execution begins
plan.status = "in_progress"
plan.planStatus = "in_progress"
plan_file = spec_dir / "implementation_plan.json"
plan.save(plan_file)
print_status("Transitioned from human_review to in_progress after approval", "success")
else:
# Plan changed after approval or not approved - stay in human_review
print_status("Plan requires re-approval before execution can continue", "warning")
return
from implementation_plan.plan import ImplementationPlan
plan_file = spec_dir / "implementation_plan.json"
if plan_file.exists():
plan = ImplementationPlan.load(plan_file)
if plan.status == "human_review" and plan.planStatus == "review":
from review.state import ReviewState
review_state = ReviewState.load(spec_dir)
if review_state.is_approval_valid(spec_dir):
# User approved the plan - transition to in_progress now that execution begins
plan.status = "in_progress"
plan.planStatus = "in_progress"
plan.save(plan_file)
print_status("Transitioned from human_review to in_progress after approval", "success")
else:
# Plan changed after approval or not approved - stay in human_review
print_status("Plan requires re-approval before execution can continue", "warning")
return


# Check if already complete
if is_build_complete(spec_dir):
print_build_complete_banner(spec_dir)
Expand Down
12 changes: 11 additions & 1 deletion apps/backend/implementation_plan/plan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,17 @@ def update_status_from_subtasks(self):
all_subtasks = [s for p in self.phases for s in p.subtasks]

if not all_subtasks:
# No subtasks yet - stay in backlog/pending
# No subtasks yet - check if this is a failed planning state
# Fix for #1149: If status is human_review but no phases/subtasks exist,
# planning crashed - reset to backlog instead of preserving invalid state
if self.status == "human_review" and not self.phases:
# Planning failed - don't show as human_review with Resume button
self.status = "backlog"
self.planStatus = "pending"
self.recoveryNote = "Planning phase failed - no implementation plan created"
return
Comment on lines +170 to +178
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Ruff format is failing for this file.

CI indicates ruff format would reformat this file. Please run ruff format and re-commit.

🧰 Tools
🪛 GitHub Actions: Lint

[error] 174-174: Ruff format check detected formatting changes. Run 'ruff format' to fix formatting.

🤖 Prompt for AI Agents 
In `@apps/backend/implementation_plan/plan.py` around lines 170 - 178, Run the
Python formatter (ruff format) on the implementation plan module to fix CI
formatting failures: apply ruff format to the file containing the code that
checks self.status == "human_review" and self.phases (the block that sets
self.status, self.planStatus, and self.recoveryNote), re-stage the reformatted
apps/backend/implementation_plan/plan.py, and re-commit so the CI ruff check
passes.

⚠️ Potential issue | 🟠 Major

Broaden the failed-planning reset to cover empty phases with zero subtasks.

In this not all_subtasks branch, gating on not self.phases misses cases where phases exist but are empty (partial/crashed write). That still leaves human_review stuck with no actionable subtasks. Consider treating any human_review + no subtasks as a failed plan here.

🐛 Proposed fix 
-            if self.status == "human_review" and not self.phases:
+            if self.status == "human_review":
📝 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.

Suggested change
# No subtasks yet - check if this is a failed planning state
# Fix for #1149: If status is human_review but no phases/subtasks exist,
# planning crashed - reset to backlog instead of preserving invalid state
if self.status == "human_review" and not self.phases:
# Planning failed - don't show as human_review with Resume button
self.status = "backlog"
self.planStatus = "pending"
self.recoveryNote = "Planning phase failed - no implementation plan created"
return
# No subtasks yet - check if this is a failed planning state
# Fix for `#1149`: If status is human_review but no phases/subtasks exist,
# planning crashed - reset to backlog instead of preserving invalid state
if self.status == "human_review":
# Planning failed - don't show as human_review with Resume button
self.status = "backlog"
self.planStatus = "pending"
self.recoveryNote = "Planning phase failed - no implementation plan created"
return
🧰 Tools
🪛 GitHub Actions: Lint

[error] 174-174: Ruff format check detected formatting changes. Run 'ruff format' to fix formatting.

🤖 Prompt for AI Agents 
In `@apps/backend/implementation_plan/plan.py` around lines 170 - 178, The current
check only resets when self.status == "human_review" and not self.phases, which
misses cases where phases exist but contain zero subtasks; update the condition
in the same block (where self.status, self.planStatus, and self.recoveryNote are
set) to detect "no subtasks" by verifying that none of the phases have subtasks
(e.g., replace/not-only-check self.phases with a test like "no phase in
self.phases has subtasks" or using any()/all() over phase.subtasks) and then
perform the same reset to backlog, planStatus="pending" and set recoveryNote.


# Normal case: No subtasks yet - stay in backlog/pending
if not self.status:
self.status = "backlog"
if not self.planStatus:
Expand Down
38 changes: 29 additions & 9 deletions apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,15 +282,35 @@ export function registerAgenteventsHandlers(
}
} else {
notificationService.notifyTaskFailed(taskTitle, project.id, taskId);
persistStatus("human_review");
// Include projectId for multi-project filtering (issue #723)
safeSendToRenderer(
getMainWindow,
IPC_CHANNELS.TASK_STATUS_CHANGE,
taskId,
"human_review" as TaskStatus,
projectId
);
// FIX (ACS-71, #1149): Only set human_review if subtasks exist
// If planning crashed before creating phases, don't set human_review
// This prevents tasks from getting stuck in "needs resume" state
const hasSubtasksOnFail = task.subtasks && task.subtasks.length > 0;
if (hasSubtasksOnFail) {
persistStatus("human_review");
// Include projectId for multi-project filtering (issue #723)
safeSendToRenderer(
getMainWindow,
IPC_CHANNELS.TASK_STATUS_CHANGE,
taskId,
"human_review" as TaskStatus,
projectId
);
} else {
// No subtasks - planning failed before creating phases
// Keep task in backlog so user can retry
console.warn(
`[Task ${taskId}] Process failed but no subtasks created - resetting to backlog`
);
persistStatus("backlog");
safeSendToRenderer(
getMainWindow,
IPC_CHANNELS.TASK_STATUS_CHANGE,
taskId,
"backlog" as TaskStatus,
projectId
);
}
}
}
} catch (error) {
Expand Down
Loading