fix(gateway): keep one Windows gateway autostart mechanism
A successful Scheduled Task install returned without removing an existing Startup-folder Hermes_Gateway.vbs or legacy .cmd, and the fallback path wrote a Startup entry even while a task was still registered. Both fire at logon, so the gateway launched twice. install() now removes Startup entries after the task registers, the fallback is skipped while a task exists, and reconcile_autostart_launchers() converges an existing install to one mechanism. Co-authored-by: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com>
This commit is contained in:
committed by
brooklyn!
parent
d06a3b8a54
commit
33f45ca30b
@@ -609,6 +609,51 @@ def _install_startup_entry(script_path: Path) -> Path:
|
||||
return entry
|
||||
|
||||
|
||||
def _remove_startup_entries() -> tuple[list[str], list[str]]:
|
||||
"""Unlink the Startup-folder entries (``.vbs`` fallback + legacy ``.cmd``); ``(done, warnings)``.
|
||||
|
||||
A failure (file locked, access denied) is reported rather than swallowed so callers warn
|
||||
instead of claiming a single autostart mechanism.
|
||||
"""
|
||||
done: list[str] = []
|
||||
warnings: list[str] = []
|
||||
for path in (get_startup_entry_path(), _legacy_startup_entry_path()):
|
||||
try:
|
||||
path.unlink()
|
||||
done.append(f"Removed redundant Windows login item: {path}")
|
||||
except FileNotFoundError:
|
||||
pass
|
||||
except OSError:
|
||||
warnings.append(f"Could not remove redundant Windows login item: {path} (locked or access denied; it still fires at logon)")
|
||||
return done, warnings
|
||||
|
||||
|
||||
def redundant_autostart_entries() -> list[Path]:
|
||||
"""Startup-folder entries that fire the gateway a second time at logon: every entry beside a
|
||||
registered Scheduled Task, or a legacy ``.cmd`` beside the ``.vbs`` fallback."""
|
||||
entries = [p for p in (get_startup_entry_path(), _legacy_startup_entry_path()) if p.exists()]
|
||||
if is_task_registered():
|
||||
return entries
|
||||
return entries[1:]
|
||||
|
||||
|
||||
def reconcile_autostart_launchers() -> tuple[list[str], list[str]]:
|
||||
"""Converge gateway logon persistence to ONE mechanism; returns ``(done, warnings)`` messages.
|
||||
|
||||
The Scheduled Task and the Startup-folder entry are alternatives, but a successful task install
|
||||
never removed an earlier fallback and pre-#45610 installs left a ``cmd.exe`` launcher behind, so
|
||||
logon could fire the launcher twice (#80569). Task registered: remove the Startup entries. No
|
||||
task but a legacy ``.cmd``: rewrite it as the console-less ``.vbs`` fallback. File operations
|
||||
only (no schtasks mutation, no elevation), so install, update and doctor can all run it.
|
||||
"""
|
||||
if is_task_registered():
|
||||
return _remove_startup_entries()
|
||||
if _legacy_startup_entry_path().exists():
|
||||
entry = _install_startup_entry(_write_task_script())
|
||||
return [f"Migrated legacy Windows login item to: {entry}"], []
|
||||
return [], []
|
||||
|
||||
|
||||
def _resolve_detached_python(python_exe: str) -> tuple[str, Path, list[str]]:
|
||||
"""Return (hidden_console_python, venv_dir, extra_pythonpath) for detached runs. ``extra_pythonpath``
|
||||
is always empty now; the tuple shape is kept so every call site stays unchanged.
|
||||
@@ -823,8 +868,14 @@ def _start_or_report_running(running_pids: list[int] | None = None) -> None:
|
||||
def _install_startup_fallback(script_path: Path, start_now: bool, detail: str) -> None:
|
||||
"""Install the Startup-folder fallback and optionally start once."""
|
||||
print(f"↻ Scheduled Task install blocked ({detail.splitlines()[0]}) — using Startup folder fallback")
|
||||
entry = _install_startup_entry(script_path)
|
||||
print(f"✓ Installed Windows login item: {entry}")
|
||||
if is_task_registered():
|
||||
# An earlier task survives (UAC declined, access denied on re-create) and still fires at
|
||||
# logon; adding the fallback beside it would start the gateway twice (#80569).
|
||||
print("⚠ Scheduled Task is still registered — skipped the Startup fallback to avoid a duplicate autostart.")
|
||||
print(" If that task is disabled or broken, run 'hermes gateway uninstall', then install again.")
|
||||
else:
|
||||
entry = _install_startup_entry(script_path)
|
||||
print(f"✓ Installed Windows login item: {entry}")
|
||||
print(f" Task script: {script_path}")
|
||||
|
||||
# Re-running install must be safe: the fallback only installs login persistence; starting is
|
||||
@@ -910,6 +961,12 @@ def install(
|
||||
print(f"✓ {detail}")
|
||||
print(f" Task script: {script_path}")
|
||||
print("ℹ Gateway auto-start installed for Windows login.")
|
||||
# A Startup-folder entry from an earlier fallback install would fire alongside the task (#80569).
|
||||
done, warnings = _remove_startup_entries()
|
||||
for message in done:
|
||||
print(f"✓ {message}")
|
||||
for message in warnings:
|
||||
print(f"⚠ {message}")
|
||||
if start_now:
|
||||
_start_or_report_running()
|
||||
else:
|
||||
|
||||
@@ -419,6 +419,60 @@ def test_uninstall_and_reinstall_sweep_stale_startup_staging_file(monkeypatch, t
|
||||
assert not staging.exists()
|
||||
|
||||
|
||||
def _startup_with_fallback_and_legacy_entries(monkeypatch, tmp_path):
|
||||
"""A Startup folder holding both the .vbs fallback and the pre-#45610 .cmd launcher."""
|
||||
startup = tmp_path / "Startup"
|
||||
startup.mkdir(parents=True)
|
||||
script = tmp_path / "gateway-service" / "Hermes_Gateway_alice.cmd"
|
||||
vbs, cmd = startup / "Hermes_Gateway_alice.vbs", startup / "Hermes_Gateway_alice.cmd"
|
||||
vbs.write_text(gateway_windows._build_startup_launcher(script), encoding="utf-8")
|
||||
cmd.write_text("@echo off", encoding="utf-8")
|
||||
monkeypatch.setattr(gateway_windows, "_assert_windows", lambda: None)
|
||||
monkeypatch.setattr(gateway_windows, "get_task_name", lambda: "Hermes_Gateway_alice")
|
||||
monkeypatch.setattr(gateway_windows, "get_startup_entry_path", lambda: vbs)
|
||||
monkeypatch.setattr(gateway_windows, "_legacy_startup_entry_path", lambda: cmd)
|
||||
monkeypatch.setattr(gateway_windows, "_write_task_script", lambda: script)
|
||||
return startup, script
|
||||
|
||||
|
||||
def test_scheduled_task_install_removes_startup_entries_that_would_double_launch(monkeypatch, tmp_path, capsys):
|
||||
"""#80569: a Scheduled Task install beside an earlier Startup fallback (or legacy .cmd) left both
|
||||
firing at logon. Installing the task converges to the task alone."""
|
||||
startup, _script = _startup_with_fallback_and_legacy_entries(monkeypatch, tmp_path)
|
||||
monkeypatch.setattr(gateway_windows, "_prompt_install_choices", lambda *a, **k: (False, True))
|
||||
monkeypatch.setattr(gateway_windows, "_is_running_as_admin", lambda: True)
|
||||
monkeypatch.setattr(gateway_windows, "_install_scheduled_task", lambda name, path: (True, "created"))
|
||||
monkeypatch.setattr(gateway_windows, "_print_next_steps", lambda: None)
|
||||
|
||||
gateway_windows.install()
|
||||
|
||||
assert sorted(p.name for p in startup.iterdir()) == []
|
||||
assert "Removed redundant Windows login item" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_reconcile_leaves_one_autostart_mechanism(monkeypatch, tmp_path):
|
||||
"""#80569: what `hermes update` and `hermes doctor --fix` run. Beside a registered task every
|
||||
Startup entry is redundant; with no task a legacy .cmd next to the .vbs is. After reconcile
|
||||
nothing is redundant and exactly one mechanism remains."""
|
||||
startup, _script = _startup_with_fallback_and_legacy_entries(monkeypatch, tmp_path)
|
||||
registered = {"task": True}
|
||||
monkeypatch.setattr(gateway_windows, "is_task_registered", lambda: registered["task"])
|
||||
|
||||
assert len(gateway_windows.redundant_autostart_entries()) == 2
|
||||
done, warnings = gateway_windows.reconcile_autostart_launchers()
|
||||
assert (len(done), warnings) == (2, [])
|
||||
assert gateway_windows.redundant_autostart_entries() == []
|
||||
assert list(startup.iterdir()) == []
|
||||
|
||||
# No task: the .vbs fallback is the mechanism, a leftover legacy .cmd beside it is the duplicate.
|
||||
registered["task"] = False
|
||||
_startup_with_fallback_and_legacy_entries(monkeypatch, tmp_path / "no-task")
|
||||
assert [p.suffix for p in gateway_windows.redundant_autostart_entries()] == [".cmd"]
|
||||
gateway_windows.reconcile_autostart_launchers()
|
||||
assert gateway_windows.redundant_autostart_entries() == []
|
||||
assert [p.name for p in (tmp_path / "no-task" / "Startup").iterdir()] == ["Hermes_Gateway_alice.vbs"]
|
||||
|
||||
|
||||
def test_status_names_and_uninstall_removes_pre_suffix_launchers(monkeypatch, tmp_path, capsys):
|
||||
"""#116157: a Scheduled Task ``Hermes_Gateway`` and a Startup ``Hermes_Gateway.vbs`` left from before
|
||||
per-profile suffixes are invisible to every ``get_task_name()``-keyed operation. ``status`` must name
|
||||
|
||||
@@ -222,6 +222,7 @@ What happens under the hood:
|
||||
|
||||
1. `schtasks /Create /SC ONLOGON /RL LIMITED /TN Hermes_Gateway` — registers a task that runs at your login with standard (non-elevated) permissions. No UAC prompt.
|
||||
2. If schtasks is blocked by group policy, falls back to writing a small `Hermes_Gateway.vbs` launcher (run hidden via `wscript.exe`) into `%APPDATA%\Microsoft\Windows\Start Menu\Programs\Startup`. Same effect, slightly cruder. A VBScript is used rather than a `cmd.exe` shortcut because a console allocated at logon can receive a close event that kills the gateway before it finishes starting.
|
||||
Only one of the two is ever kept: a successful task install removes any Startup-folder entry (including a legacy `Hermes_Gateway.cmd`), the fallback is skipped while a task is still registered, and `hermes update` / `hermes doctor --fix` clean up older installs that have both, since both would launch the gateway at logon.
|
||||
3. Spawns the gateway **detached via `pythonw.exe`** — not `python.exe`. `pythonw.exe` has no console attached, which immunizes it against `CTRL_C_EVENT` broadcasts from sibling processes (a real issue that used to kill the gateway when you Ctrl+C'd anything in the same process group).
|
||||
|
||||
Flags used when spawning: `DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW | CREATE_BREAKAWAY_FROM_JOB`.
|
||||
|
||||
Reference in New Issue
Block a user