fix(jobs): detect a dead local server and record a terminal server_died error (BE-4751)

[mattmillerai]

ELI-5

If you start a job with comfy run (no --wait), a little background watcher keeps an eye on it and writes what it sees into a state file. But if ComfyUI itself dies mid-job — say the GPU runs out of memory and the OS kills it — the watcher never noticed. It kept asking a dead server for news every 2 seconds for six hours, and comfy jobs ls cheerfully reported the job as queued the whole time. This teaches the watcher to check whether the server is actually still alive, and to write down server_died when it isn't.

What changed

The watcher's local branch now does an explicit liveness probe each cycle, using the same check_comfy_server_running helper comfy jobs already uses for its own up/down check.

• Server up → the streak counter resets and the normal poll runs, exactly as before.
• Server down → the counter increments. Three consecutive failed probes (_SERVER_DOWN_CONSECUTIVE_LIMIT = 3, ≈6s at the 2s poll interval — enough to ride out a blip or a quick restart) on a non-terminal job writes a terminal error with code: "server_died", naming the host, port, last status and probe count, then breaks out so the existing _notify fires. Any successful probe in between resets the streak, so a restart within the window is not death.
• Under the limit, the poll is skipped for that cycle — it can only fail against a server we just failed to reach.
• A state that is already terminal keeps its own verdict: a dead server can't invalidate a job that already completed or failed. The watcher just stops.
• Probe and poll now resolve their target through one shared _resolve_watch_target(state, host, port) (the resolve_local_host_port precedence + IPv6 bracketing lifted out of _poll_local_once), so they can never disagree about which server is being watched.
• The cloud branch is untouched, and jobs.py::_snapshot keeps its error-swallowing — other callers rely on it.

Why this shape

_snapshot deliberately swallows the connection failure on /queue (except RuntimeError: q = {}) and returns None on /history, and None is indistinguishable from the legitimate "prompt hasn't shown up yet" case. So the poll path structurally cannot tell a dead server from a slow one — a separate liveness signal is the only thing that can.

Tests

Six new tests in tests/comfy_cli/jobs/test_jobs.py (where the other watcher tests already live — the ticket offered a new file as the alternative). They monkeypatch the probe and time.sleep, and assert against the real on-disk state file via the autouse _isolate_jobs_state_dir fixture rather than an in-memory object:

1. probe fails 3× while queued → file ends status == "error", error.code == "server_died", details.last_status == "queued", host/port/probe-count populated, loop exits (no 6h wait), and the poll is asserted never to run against a server that failed its probe;
2. flap False, False, True → streak resets, no server_died, a subsequent completed snapshot ends status == "completed" with outputs;
3. already-terminal state + failing probe → no overwrite (status, error and outputs all preserved);
4. _notify called exactly once, with the final server_died state (asserted inside test 1);
5. probe and poll resolve to the same address, including IPv6 bracketing (::1 → [::1]);
6. an unexpected exception from the probe counts as "down" rather than crashing the watcher.

Full suite: 2774 passed, 37 skipped.

Judgment calls and deviations — please read

• The diff is not confined to job_watcher.py + tests. It also adds server_died to comfy_cli/error_codes.py. That registry is the canonical agent-facing contract and the repo's own test_every_raised_code_is_registered fails without it, so the ticket's "confined to" criterion could not be met as written. This is the only extra file.
• ruff check . is not clean on this branch — and it is not clean on main either. There are 15 pre-existing UP038 errors (comfy_cli/workflow_to_api.py, cloud/oauth.py, command/generate/emit.py, cql/default_workflow.py, registry/config_parser.py) plus one pre-existing ruff format diff in tests/comfy_cli/command/github/test_pr.py. None are in files this PR touches, and I left them alone rather than smuggling an unrelated cleanup into this diff. ruff check and ruff format --check are clean on all three touched files.
• New behavior beyond the ticket, deliberately: when the state is already terminal and the server is confirmed gone, the watcher now breaks instead of spinning to the 6h ceiling and then overwriting the verdict with watcher_timeout. Without this the "don't overwrite a terminal state" rule would have left an unbreakable loop. Only reachable when a watcher is (re-)launched against an already-finished job.
• Riskiest line — the false-positive question. A busy-but-alive server that can't answer /history within the probe's 5s timeout three times running would be misreported as dead. Two things bound that: the probe reuses the product's established liveness signal, and comfy jobs status / jobs ls already hard-exit server_not_running on a single failure of that same probe at the same choke point — so this check is strictly more tolerant than behavior users already depend on. Raising _SERVER_DOWN_CONSECUTIVE_LIMIT is a one-line change if field data disagrees.
• Negative-claim falsification trigger — considered, does not fire. The diff adds a terminal error path, but it denies no product capability: it reports an empirically observed runtime condition, and the "does the server exist" claim is exactly what the code probes live, three times, before asserting it. Nothing here tells a user or agent that a feature is unavailable, and no test was flipped to assert a dead end.

Result

comfy jobs ls / jobs wait now surface server_died with the prompt_id instead of a forever-queued ghost. (jobs status follows once its independent state-file-fallback ticket lands — no ordering dependency.)

[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: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

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: ASSERTIVE

Plan: Pro Plus

Run ID: c3fb889f-16b6-494c-9a29-bf1d7405a7d8

📥 Commits

Reviewing files that changed from the base of the PR and between 6c88dd7 and 38d0b02.

📒 Files selected for processing (3)

• comfy_cli/command/job_watcher.py
• comfy_cli/error_codes.py
• tests/comfy_cli/jobs/test_jobs.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-4751-watcher-server-died

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-4751-watcher-server-died

---

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

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 5 finding(s).

Severity	Count
🟠 High	1
🟡 Medium	2
🟢 Low	2

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)

[skishore23]

LGTM