fix(comfyui): stop the converger relaunching into a port it cannot free #890

Merged

coilysiren merged 1 commit from fix/comfyui-restart-race into main

2026-08-20 06:00:13 +00:00

coilyco-ops commented

2026-08-20 05:49:45 +00:00

Owner

What

The converge's restart step stopped the scheduled task, tried to kill the runtime, then relaunched immediately. When the kill did not land, the new process died on bind and nothing was left serving. That is what took the tower endpoint down for nine minutes on 2026-08-19, documented in #883.

Two independent reasons the kill did not land, both now handled.

1. The kill matched on fields that read back empty

The old filter required Win32_Process.ExecutablePath to equal $PythonExe and CommandLine to be like *ComfyUI*main.py*. Measured against the live runtime on kai-tower-3026:

listener pid   : 39132
Name           : python.exe
ExecutablePath :
CommandLine    :
ExecutablePath -eq expected        : False
CommandLine like *ComfyUI*main.py* : False

Both fields are empty, so the filter could never match and the orphan always survived. This now matches on ownership of the loopback port, which needs no cross-process visibility, and waits for the port to actually clear before starting rather than relaunching four seconds later and hoping.

2. The kill is refused outright, and that is new

Even with the right process identified, Stop-Process fails:

pid=39132 name=python sessionId=0
Stop-Process FAILED: Cannot stop process "python (39132)" ... Access is denied
user=kai-tower-3026\firem  elevated=False  session=1

The runtime is in session 0. The converger runs unelevated in session 1. It cannot signal that process at all, which is also why the two Win32_Process fields above read empty.

This traces to #888, merged at 22:41:38 PDT today, which changed the ComfyUI task principal to -LogonType S4U. S4U tasks run in session 0. Before that change the task was Interactive, the runtime shared the operator's session, and the original ExecutablePath / CommandLine filter would have had the visibility it needs. So #888 quietly removed the converger's ability to manage its own runtime, and this PR is what makes that visible instead of silently fatal.

No restart can succeed while the runtime is unreachable this way, so the converge now fails loudly and names the owning pid, its session, and the refusal, leaving the working runtime alone rather than killing the endpoint on the way past.

What this does not do

It does not resolve the session 0 placement. That is a real decision, not a bug to patch here: either the task goes back to a logon type the converger can manage, or the converger gains an elevated path to stop an S4U task, or restart stops going through Stop-Process at all and uses the task surface. #883 tracks it, and whoever owns #888 should weigh in, since the tradeoff that motivated S4U is theirs.

Verification

Live on kai-tower-3026 against a running ComfyUI, forcing the restart path twice by perturbing serve.cmd so the launcher is rewritten:

launcher: converged
Loopback port 8188 is still held, so no restart was attempted and the running ComfyUI
was left alone. Refused: pid 39132 (session 0): Cannot stop process "python (39132)"
because of the following error: Access is denied

Endpoint before and after both runs: loopbackListener=1 http=200. Under the old code this same situation produced a dead endpoint. serve.cmd is back to canonical content, rewritten by the converge itself.

windows-comfyui-convergence-test passes 6 of 6 and pre-commit run --all-files is clean.

I could not add a unit test for this path. Reproducing it needs a real listener the test process is forbidden to kill, which the harness cannot construct. The live evidence above is what I have.

Refs #883

🤖 Generated with Claude Code

## What The converge's restart step stopped the scheduled task, tried to kill the runtime, then relaunched immediately. When the kill did not land, the new process died on bind and nothing was left serving. That is what took the tower endpoint down for nine minutes on 2026-08-19, documented in #883. Two independent reasons the kill did not land, both now handled. ## 1. The kill matched on fields that read back empty The old filter required `Win32_Process.ExecutablePath` to equal `$PythonExe` and `CommandLine` to be like `*ComfyUI*main.py*`. Measured against the live runtime on `kai-tower-3026`: ``` listener pid : 39132 Name : python.exe ExecutablePath : CommandLine : ExecutablePath -eq expected : False CommandLine like *ComfyUI*main.py* : False ``` Both fields are empty, so the filter could never match and the orphan always survived. This now matches on ownership of the loopback port, which needs no cross-process visibility, and waits for the port to actually clear before starting rather than relaunching four seconds later and hoping. ## 2. The kill is refused outright, and that is new Even with the right process identified, `Stop-Process` fails: ``` pid=39132 name=python sessionId=0 Stop-Process FAILED: Cannot stop process "python (39132)" ... Access is denied user=kai-tower-3026\firem elevated=False session=1 ``` The runtime is in **session 0**. The converger runs unelevated in session 1. It cannot signal that process at all, which is also why the two `Win32_Process` fields above read empty. **This traces to #888**, merged at 22:41:38 PDT today, which changed the ComfyUI task principal to `-LogonType S4U`. S4U tasks run in session 0. Before that change the task was `Interactive`, the runtime shared the operator's session, and the original `ExecutablePath` / `CommandLine` filter would have had the visibility it needs. So #888 quietly removed the converger's ability to manage its own runtime, and this PR is what makes that visible instead of silently fatal. No restart can succeed while the runtime is unreachable this way, so the converge now fails loudly and names the owning pid, its session, and the refusal, leaving the working runtime alone rather than killing the endpoint on the way past. ## What this does not do It does not resolve the session 0 placement. That is a real decision, not a bug to patch here: either the task goes back to a logon type the converger can manage, or the converger gains an elevated path to stop an S4U task, or restart stops going through `Stop-Process` at all and uses the task surface. #883 tracks it, and whoever owns #888 should weigh in, since the tradeoff that motivated S4U is theirs. ## Verification Live on `kai-tower-3026` against a running ComfyUI, forcing the restart path twice by perturbing `serve.cmd` so the launcher is rewritten: ``` launcher: converged Loopback port 8188 is still held, so no restart was attempted and the running ComfyUI was left alone. Refused: pid 39132 (session 0): Cannot stop process "python (39132)" because of the following error: Access is denied ``` Endpoint before and after both runs: `loopbackListener=1 http=200`. Under the old code this same situation produced a dead endpoint. `serve.cmd` is back to canonical content, rewritten by the converge itself. `windows-comfyui-convergence-test` passes 6 of 6 and `pre-commit run --all-files` is clean. I could not add a unit test for this path. Reproducing it needs a real listener the test process is forbidden to kill, which the harness cannot construct. The live evidence above is what I have. Refs #883 🤖 Generated with [Claude Code](https://claude.com/claude-code)

coilyco-ops added 1 commit

2026-08-20 05:49:45 +00:00

fix(comfyui): stop the converger relaunching into a port it cannot free

TruffleHog / Scan for secrets (pull_request) Successful in 5s

Details

CI / lint (pull_request) Successful in 41s

Details

bb69c6a259

The restart step stopped the scheduled task, tried to kill the runtime,
then immediately relaunched. When the kill did not land, the relaunch
died on bind and left nothing serving. That is what took the tower down
for nine minutes on 2026-08-19.

The kill never landed because it matched on Win32_Process ExecutablePath
and CommandLine, both of which read back empty for the running ComfyUI.
Match on ownership of the loopback port instead, which needs no such
visibility, then wait for the port to actually clear before starting.

The kill can still be refused outright: the runtime lands in session 0
while the converger runs unelevated in session 1, so Stop-Process gets
access denied. There is no restart that can succeed in that state, so
fail loudly and name the owning pid, its session, and the refusal rather
than relaunching into a port that will never free. A converge that
cannot restart now leaves the working runtime alone instead of killing
the endpoint.

This does not fix the session 0 placement itself. That needs a decision
about how the task is registered and stays tracked in #883.

Verified on kai-tower-3026 against a live runtime. The converge reports
the refusal with the owning pid and session, and the endpoint stayed up
across both runs.

Refs #883

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-20 05:49:45 +00:00

coilyco-ops referenced this pull request

2026-08-20 05:50:08 +00:00

ComfyUI died on kai-tower-3026 and the scheduled task did not restart it, leaving Tailscale Serve forwarding to a dead listener #883

coilysiren merged commit b097b2856b into main

2026-08-20 06:00:13 +00:00

coilysiren referenced this pull request from a commit

2026-08-20 06:00:14 +00:00

Merge pull request 'fix(comfyui): stop the converger relaunching into a port it cannot free' (#890) from fix/comfyui-restart-race into main

coilyco-ops referenced this pull request

2026-08-20 06:25:05 +00:00