Skip to content

Commit 5c06e22

Browse files
author
Test User
committed
fix(tasks): address code review feedback
- Return success=False when API key missing (blocking issue) - Add ASSIGNED status check to prevent duplicate triggers - Add debug logging for zero pending tasks case - Add test for ASSIGNED status blocking
1 parent 7c8b856 commit 5c06e22

File tree

2 files changed

+62
-18
lines changed

2 files changed

+62
-18
lines changed

codeframe/ui/routers/tasks.py

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -444,40 +444,54 @@ async def assign_pending_tasks(
444444
pending_count = len(pending_unassigned)
445445

446446
if pending_count == 0:
447+
# Debug logging to help diagnose why tasks might appear stuck
448+
logger.debug(
449+
f"assign_pending_tasks called for project {project_id} but found 0 pending unassigned tasks. "
450+
f"Total tasks: {len(tasks)}, statuses: {[t.status.value for t in tasks]}"
451+
)
447452
return TaskAssignmentResponse(
448453
success=True,
449454
pending_count=0,
450455
message="No pending unassigned tasks to assign."
451456
)
452457

453458
# Check if execution is already in progress (Phase 1 fix for concurrent execution)
454-
in_progress_tasks = [t for t in tasks if t.status == TaskStatus.IN_PROGRESS]
455-
if in_progress_tasks:
459+
# Include ASSIGNED status to prevent race between assignment and execution start
460+
executing_tasks = [
461+
t for t in tasks
462+
if t.status in [TaskStatus.ASSIGNED, TaskStatus.IN_PROGRESS]
463+
]
464+
if executing_tasks:
456465
logger.info(
457466
f"⏳ Execution already in progress for project {project_id}: "
458-
f"{len(in_progress_tasks)} tasks running"
467+
f"{len(executing_tasks)} tasks assigned/running"
459468
)
460469
return TaskAssignmentResponse(
461470
success=True,
462471
pending_count=pending_count,
463-
message=f"Execution already in progress ({len(in_progress_tasks)} task(s) running). Please wait."
472+
message=f"Execution already in progress ({len(executing_tasks)} task(s) assigned/running). Please wait."
464473
)
465474

466475
# Schedule multi-agent execution in background
467476
api_key = os.environ.get("ANTHROPIC_API_KEY")
468-
if api_key:
469-
background_tasks.add_task(
470-
start_development_execution,
471-
project_id,
472-
db,
473-
manager,
474-
api_key
475-
)
476-
logger.info(f"✅ Scheduled task assignment for project {project_id} ({pending_count} pending tasks)")
477-
else:
477+
if not api_key:
478478
logger.warning(
479479
f"⚠️ ANTHROPIC_API_KEY not configured - cannot assign tasks for project {project_id}"
480480
)
481+
return TaskAssignmentResponse(
482+
success=False,
483+
pending_count=pending_count,
484+
message="Cannot assign tasks: API key not configured. Please contact administrator."
485+
)
486+
487+
background_tasks.add_task(
488+
start_development_execution,
489+
project_id,
490+
db,
491+
manager,
492+
api_key
493+
)
494+
logger.info(f"✅ Scheduled task assignment for project {project_id} ({pending_count} pending tasks)")
481495

482496
return TaskAssignmentResponse(
483497
success=True,

tests/ui/test_assign_pending_tasks.py

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,12 @@ async def test_assign_pending_tasks_without_api_key(
206206
current_user=mock_user
207207
)
208208

209-
# Should still return success (pending tasks exist)
210-
assert response.success is True
209+
# Should return failure when API key missing
210+
assert response.success is False
211211
assert response.pending_count == 1
212+
assert "api key" in response.message.lower() or "not configured" in response.message.lower()
212213

213-
# But background task should NOT be scheduled
214+
# Background task should NOT be scheduled
214215
mock_background_tasks.add_task.assert_not_called()
215216

216217
# Should log warning
@@ -281,7 +282,7 @@ async def test_assign_pending_tasks_allowed_when_no_execution_in_progress(
281282
"""Test that assignment proceeds when no tasks are in progress."""
282283
from codeframe.ui.routers.tasks import assign_pending_tasks
283284

284-
# Setup: Only pending and completed tasks, no in_progress
285+
# Setup: Only pending and completed tasks, no in_progress or assigned
285286
mock_db.get_project_tasks.return_value = [
286287
Task(id=1, project_id=1, title="Task 1", status=TaskStatus.PENDING, assigned_to=None),
287288
Task(id=2, project_id=1, title="Task 2", status=TaskStatus.PENDING, assigned_to=None),
@@ -303,6 +304,35 @@ async def test_assign_pending_tasks_allowed_when_no_execution_in_progress(
303304
assert "started" in response.message.lower()
304305
mock_background_tasks.add_task.assert_called_once()
305306

307+
@pytest.mark.asyncio
308+
async def test_assign_pending_tasks_blocked_when_tasks_assigned(
309+
self, mock_db, mock_user, mock_manager, mock_background_tasks
310+
):
311+
"""Test that assignment is blocked when tasks are in ASSIGNED status."""
312+
from codeframe.ui.routers.tasks import assign_pending_tasks
313+
314+
# Setup: 2 pending tasks + 1 assigned (not yet in_progress)
315+
mock_db.get_project_tasks.return_value = [
316+
Task(id=1, project_id=1, title="Task 1", status=TaskStatus.PENDING, assigned_to=None),
317+
Task(id=2, project_id=1, title="Task 2", status=TaskStatus.PENDING, assigned_to=None),
318+
Task(id=3, project_id=1, title="Task 3", status=TaskStatus.ASSIGNED, assigned_to="agent-1"),
319+
]
320+
321+
with patch("codeframe.ui.routers.tasks.manager", mock_manager), \
322+
patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}):
323+
response = await assign_pending_tasks(
324+
project_id=1,
325+
background_tasks=mock_background_tasks,
326+
db=mock_db,
327+
current_user=mock_user
328+
)
329+
330+
# Should return success but NOT schedule execution
331+
assert response.success is True
332+
assert response.pending_count == 2
333+
assert "in progress" in response.message.lower() or "assigned" in response.message.lower()
334+
mock_background_tasks.add_task.assert_not_called()
335+
306336

307337
class TestAssignPendingTasksResponseModel:
308338
"""Tests for TaskAssignmentResponse model."""

0 commit comments

Comments
 (0)