Commit ad3c45ec2a6cac6c5348dd4d9660020be307c29c

Authored by bboonnee
1 parent 4cd87c41

fix: manual_resume auto-continue allowlist missing reviewing_planner_output

Real incident (2026-08-13, task b243dfe8): _handle_task_route's auto-continue
allowlist was ("queued", "planning_brd_prd_sdd", "coding", "validating").
Org Graph unification Phase 1 (2026-08-08) added 'reviewing_planner_output' to
task_execution._RESUMABLE_STATUSES, but nobody updated this separate allowlist
to match -- the same exact bug the 2026-08-07 'validating' fix (see the
existing comment right above this change) was supposed to have taught us to
watch for, just one status later.

Effect: manual_resume onto a task landing on 'reviewing_planner_output' flipped
the DB status back and returned 200, but spawned no background thread. The task
sat completely idle (zero new execution_event rows, no real subprocess) --
which looks identical to "still stuck" from the console, even though the actual
underlying bug that originally blocked it (a separate, already-fixed issue) was
long gone. Diagnosed live: resumed b243dfe8, watched it sit with no activity
for 7+ minutes, found zero events past the resume's own status-change row and
no claude subprocess running anywhere.

New regression test mirrors the existing 'validating' one exactly.

814/814 tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
src/backend_server.py
@@ -561,8 +561,21 @@ class _Handler(BaseHTTPRequestHandler): @@ -561,8 +561,21 @@ class _Handler(BaseHTTPRequestHandler):
561 # path, still didn't know that, so a manual Resume on a task stuck 561 # path, still didn't know that, so a manual Resume on a task stuck
562 # in 'validating' flipped the status back with nothing ever 562 # in 'validating' flipped the status back with nothing ever
563 # actually continuing it. 563 # actually continuing it.
  564 + #
  565 + # 'reviewing_planner_output' added 2026-08-13 (real incident, task
  566 + # b243dfe8): same exact bug, one status later -- Org Graph
  567 + # unification Phase 1 (2026-08-08) added this status to
  568 + # task_execution._RESUMABLE_STATUSES, but nobody updated THIS
  569 + # separate allowlist to match, so a manual_resume off
  570 + # blocked_pending_human onto 'reviewing_planner_output' flipped the
  571 + # DB status and spawned nothing -- the task sat completely idle
  572 + # (zero new execution_event rows, no claude subprocess) looking
  573 + # exactly like it was "stuck again", when actually nothing was
  574 + # running at all. Any status added to _RESUMABLE_STATUSES going
  575 + # forward needs to land here too -- the two lists have now drifted
  576 + # out of sync twice for the identical reason.
564 if pre_resume_reason not in ("budget_exceeded", "loop_limit_reached") and result.get("new_status") in ( 577 if pre_resume_reason not in ("budget_exceeded", "loop_limit_reached") and result.get("new_status") in (
565 - "queued", "planning_brd_prd_sdd", "coding", "validating", 578 + "queued", "planning_brd_prd_sdd", "reviewing_planner_output", "coding", "validating",
566 ): 579 ):
567 task_row = self.server.conn.execute( 580 task_row = self.server.conn.execute(
568 "SELECT project_id, goal_text FROM task WHERE task_id = ?", (task_id,) 581 "SELECT project_id, goal_text FROM task WHERE task_id = ?", (task_id,)
src/test_backend_server.py
@@ -912,6 +912,32 @@ class TaskExecutionRouteTest(unittest.TestCase): @@ -912,6 +912,32 @@ class TaskExecutionRouteTest(unittest.TestCase):
912 self.assertEqual(status, 200) 912 self.assertEqual(status, 200)
913 mock_start.assert_called_once_with(self.server.db_path, "task-1", "/tmp/proj-1", "the goal") 913 mock_start.assert_called_once_with(self.server.db_path, "task-1", "/tmp/proj-1", "the goal")
914 914
  915 + def test_manual_resume_from_reviewing_planner_output_auto_continues(self):
  916 + # 2026-08-13 real incident (task b243dfe8): same exact bug as
  917 + # 'validating' above, one status later -- Org Graph unification
  918 + # Phase 1 (2026-08-08) added 'reviewing_planner_output' to
  919 + # task_execution._RESUMABLE_STATUSES, but this separate allowlist
  920 + # was never updated to match. manual_resume flipped the DB status
  921 + # back and returned 200, but spawned no background thread -- the
  922 + # task sat completely idle (zero new execution_event rows, no real
  923 + # subprocess) for minutes, looking exactly like a fresh hang even
  924 + # though the actual underlying bug (a different one, already fixed
  925 + # the same day) was long gone.
  926 + self.server.conn.execute(
  927 + "UPDATE task SET status = 'blocked_pending_human', blocked_reason = 'execution_error' "
  928 + "WHERE task_id = 'task-1'"
  929 + )
  930 + self.server.conn.execute(
  931 + "INSERT INTO execution_event (event_id, project_id, task_id, event_type, payload) "
  932 + "VALUES ('evt-1', 'proj-1', 'task-1', 'task_status_changed', "
  933 + "'{\"from_status\": \"reviewing_planner_output\", \"to_status\": \"blocked_pending_human\"}')"
  934 + )
  935 + self.server.conn.commit()
  936 + with patch.object(backend_server.task_execution, "start_task_execution_in_background") as mock_start:
  937 + status, body = self._post("/api/tasks/task-1/route", {"decided_route": "manual_resume"})
  938 + self.assertEqual(status, 200)
  939 + mock_start.assert_called_once_with(self.server.db_path, "task-1", "/tmp/proj-1", "the goal")
  940 +
915 def test_execute_already_in_flight_returns_409_not_500(self): 941 def test_execute_already_in_flight_returns_409_not_500(self):
916 with patch.object( 942 with patch.object(
917 backend_server.task_execution, "start_task_execution_in_background", 943 backend_server.task_execution, "start_task_execution_in_background",