Skip to content

fix(gateway): retry detached restart helper without job breakaway on Windows - #67917

Closed
askie wants to merge 1 commit into
NousResearch:mainfrom
askie:fix/windows-restart-helper-breakaway
Closed

fix(gateway): retry detached restart helper without job breakaway on Windows#67917
askie wants to merge 1 commit into
NousResearch:mainfrom
askie:fix/windows-restart-helper-breakaway

Conversation

@askie

@askie askie commented Jul 20, 2026

Copy link
Copy Markdown

Problem

On Windows, when the gateway runs inside a job object that disallows breakaway (no JOB_OBJECT_LIMIT_BREAKAWAY_OK — e.g. when started via Task Scheduler), CreateProcess with CREATE_BREAKAWAY_FROM_JOB fails with ERROR_ACCESS_DENIED, which Python surfaces as PermissionError.

GatewayRunner._launch_detached_restart_command lets that exception escape. The caller in request_restart only logs it (Failed to launch detached gateway restart helper: [WinError 5] Access is denied) and then proceeds with stop(restart=True) anyway — so the gateway shuts down for the planned restart and never comes back until someone starts it manually.

This is reproducible in the wild: a messaging-platform plugin auto-update triggers a detached restart, the helper launch fails with WinError 5, and the gateway stays dead for days (the Task Scheduler entry only triggers at logon).

Fix

Retry the watcher spawn without CREATE_BREAKAWAY_FROM_JOB when the first Popen raises OSError — the exact fallback gateway_windows._spawn_detached already implements for the same documented condition, and the pattern windows_detach_flags_without_breakaway() was added for. Without the breakaway flag the watcher stays inside the job, which is still far better than no watcher at all.

Testing

  • Added test_windows_detached_restart_retries_without_breakaway: first spawn raises PermissionError(5), asserts a second spawn happens with the no-breakaway flags and identical argv.
  • pytest tests/gateway/test_restart_drain.py — 25 passed.
  • Verified on a live Windows install where this bug bit twice: gateway now restarts cleanly through the same code path.

…Windows

When the gateway runs inside a job object that disallows breakaway
(no JOB_OBJECT_LIMIT_BREAKAWAY_OK, e.g. under Task Scheduler),
CreateProcess with CREATE_BREAKAWAY_FROM_JOB fails with
ERROR_ACCESS_DENIED. _launch_detached_restart_command let that
PermissionError escape, so the restart helper never launched: the
gateway then stopped for the planned restart and never came back.

Retry the watcher spawn without the breakaway flag, the same fallback
gateway_windows._spawn_detached already uses.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/windows Native Windows-specific behavior or breakage sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows duplicate This issue or pull request already exists labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #42993. Both add the current-main outer watcher retry without CREATE_BREAKAWAY_FROM_JOB; #42993 is the earlier focused implementation and also handles a second spawn failure with safe diagnostics.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows restart fix. This is now redundant with the earlier duplicate implementation.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 30, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants