fix(api): fail_run on background agent-launch error, not stuck IN_PROGRESS (#722)

[frankbria]

What & why

POST /tasks/{id}/start?execute=true creates the run (task → IN_PROGRESS), returns 200, then runs execute_agent in a daemon thread. That thread's except only logged + emitted an SSE ErrorEvent — it never reset the run. A very common misconfig (missing ANTHROPIC_API_KEY, unknown provider) raises ValueError before execute_agent's own try, so the run stayed RUNNING forever: every retry 400'd with "Task already has an active run", and if no SSE client was attached the error was lost entirely.

Fixes #722 [P0.11].

Change

The background except now calls runtime.fail_run(workspace, run.id, reason=str(exc)) before publishing the SSE error, resetting the task to FAILED (a retryable state). Guarded in its own try/except so an already-FAILED run (e.g. an error execute_agent handled internally) can't break the handler.

Tests (tests/ui/test_tasks_start_failure.py)

With execute_agent patched to raise up front: start?execute=true returns 200, the task ends FAILED not IN_PROGRESS, and a second start does not 400. Uses the real daemon thread + a bounded poll (a launch that fails up front resolves in ms) — an inline/synchronous thread would deadlock against start_task_run's open SQLite write, which the real separate-thread flow avoids.

75 passed across tasks_v2 integration + this suite. ruff + mypy clean.

Demo

start response: 200
RUN_FAILED ...
task status after failed launch: FAILED   (was stuck IN_PROGRESS before #722)
retry start status: 200   (not 400 'already active')

Acceptance criteria

• The background except calls runtime.fail_run(...) before publishing the SSE error
• Missing API key / unknown provider yields a retryable task, not permanent IN_PROGRESS
• Test: start-with-execute under a missing key leaves the task non-active afterward

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 15 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6adca22b-8157-4758-bd83-69b0d3c6c2c3

📥 Commits

Reviewing files that changed from the base of the PR and between 0a5340a and 1c051b3.

📒 Files selected for processing (2)

• codeframe/ui/routers/tasks_v2.py
• tests/ui/test_tasks_start_failure.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/p0.11-fail-run-on-launch-error

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Code Review

Summary: Clean, targeted fix for a genuine P0 correctness bug — a background agent launch failure would leave the task permanently stuck in IN_PROGRESS, making all subsequent retries 400 with "already has an active run". The fix is in exactly the right place and the reasoning is sound.

---

What's good

• Fix placement is correct: _run_agent's outer except is the only catch site for pre-execute_agent raises (e.g. missing API key). Calling fail_run there is the right call.
• Double-fail guard is correct: Wrapping fail_run in its own try/except is necessary — fail_run raises ValueError if the run is not RUNNING/BLOCKED, so if execute_agent already handled its own failure internally, the secondary call would otherwise blow up the handler.
• State-before-event ordering: fail_run is called before publisher.publish_sync, so the SSE client reads consistent state.
• Test uses real daemon thread: The PR description explains why — an inline/synchronous invocation would deadlock against start_task_run's open SQLite write. The bounded poll (_wait_until with 5s timeout) is the right approach for async behavior.
• Test covers both acceptance criteria: task lands FAILED, and a second start returns non-400.

---

Issues

Minor — weak assertion in test_failed_launch_allows_restart

assert r2.status_code != 400

This passes on 404, 500, or any other error response. The intent is that a retry is accepted, so the assertion should be:

assert r2.status_code == 200

Minor — comment block exceeds CLAUDE.md's single-line guideline

CLAUDE.md says: "Only add one when the WHY is non-obvious … one short line max". The 5-line comment here is genuinely warranted (the invariant is subtle), but could be condensed without losing meaning:

# Outer catch only: execute_agent handles its own errors; pre-execute raises
# (e.g. missing API key) reach here. Guarded against double-fail (#722).

---

Nit

exc_info=True on a logger.debug call is unusual — it renders a full traceback at debug verbosity. Since this is the expected double-fail path (not an error), omitting it would be less noisy:

logger.debug("fail_run skipped for task  (run not active)", task_id)

---

Overall

The core fix is correct and the test coverage is solid. The != 400 assertion and the comment length are the only things worth addressing before merge. The fix is well-scoped, does not touch unrelated code, and the PR description accurately explains the root cause.