now-ing f37c734406
fix(scheduler): retain launched run when post-launch bookkeeping fails (#4452) (#4504)
* fix(scheduler): retain launched run when post-launch bookkeeping fails

`dispatch_task()` created a `queued` task-run row, then `_launch_run()`
returned a live `run_id`, and only afterward did the queued->running
bookkeeping (`update_status` + `update_after_launch`) run. When that
bookkeeping raised on a transient DB error, the `except` handler marked the
task-run `failed` with `last_run_id=None`. Because `failed` is outside the
partial unique index `uq_scheduled_task_run_active`, this released the
task's single active slot: the next dispatch cycle could no longer see the
still-live run and launched a duplicate. The launched `run_id` was also
dropped, breaking later recovery / reconciliation / cancellation.

Track `launched_run_id` / `launched_thread_id`, set only after `_launch_run`
returns. In the `except` handler:

- If launch already succeeded, keep the task-run row `running` (so it keeps
  holding the active slot and no duplicate launch can occur) and persist the
  launched `run_id` on the parent task for retention. The bookkeeping
  retries are best-effort with logging; if they fail too the row stays
  `queued`, which is still active and still holds the slot, so we still
  report the run as launched.
- If launch itself failed (no live run was created), behave as before:
  mark the task-run `failed` and release the active slot.

The overlap-skip branch is now guarded by `launched_run_id is None` so a
run that already launched can never be reclassified as a skip / failed.

Adds a stateful regression test (`test_post_launch_bookkeeping_failure_does_not_release_active_slot`)
that injects a failure on the queued->running write and asserts a second
dispatch does not launch another run (`launch_count` stays 1) while the
first `run_id` is retained on the task-run row. The test is verified to
fail on `main` and pass with this change. A complement test pins the
pre-launch-failure path (launch itself raises) to ensure the slot is still
released when no live run exists.

Fixes #4452

* style(scheduler): apply ruff format to fix lint-backend CI

Reformat the two files touched by the previous commit with
`ruff format` (line-length=240 config joins the hand-wrapped
condition/log lines). No semantic change.

Fixes the `lint-backend` CI failure on PR #4504.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(scheduler): key retention on launch_succeeded flag

The previous invariant keyed the retention branch off
`launched_run_id is not None`, but the assignment
`launched_run_id = result["run_id"]` is itself post-launch code that
can raise (KeyError/TypeError on a malformed _launch_run result).
In that case launched_run_id stays None and the dispatch falls through
to the pre-launch generic-failure path, marking the task-run row failed
and releasing the active slot -- even though a live run was just
created (same class of bug as #4452, narrower trigger).

Flip a `launch_succeeded` flag immediately after `await _launch_run(...)`
returns, before any further code that can raise, and key both the
overlap-conflict guard and the retention branch off that flag.

Add a regression test with a malformed launch result (missing run_id):
the dispatch reports outcome="launched", the row stays running, and a
second dispatch does not launch a duplicate.

Addresses willem-bd review point 1 on #4504.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(scheduler): don't surface bookkeeping transient as task last_error

In the post-launch retention path the parent task's last_error was set
to the bookkeeping exception -- an infrastructure-level transient, not
a run-level failure. Between the failed bookkeeping write and the run
completing, the task list showed an error on a task whose run was
actively running.

Clear last_error (like the success path's clear-on-launch model): the
run's real terminal outcome is written by handle_run_completion, and
the transient itself is already recorded via logger.exception.

Assert in the retention regression test that the parent task update
carries last_error=None.

Addresses willem-bd review point 2 on #4504 (taking the drop option).

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: now-ing <24534365+now-ing@users.noreply.github.com>
Co-authored-by: now-ing <now-ing@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
2026-07-29 23:12:41 +08:00
..