diff --git a/hermes_cli/gateway_windows.py b/hermes_cli/gateway_windows.py index 83f607a376..b42f2521ef 100644 --- a/hermes_cli/gateway_windows.py +++ b/hermes_cli/gateway_windows.py @@ -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: diff --git a/tests/hermes_cli/test_gateway_windows.py b/tests/hermes_cli/test_gateway_windows.py index 6fa92d2867..3140dada1e 100644 --- a/tests/hermes_cli/test_gateway_windows.py +++ b/tests/hermes_cli/test_gateway_windows.py @@ -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 diff --git a/website/docs/user-guide/windows-native.md b/website/docs/user-guide/windows-native.md index 89aa1fbdcc..f574511bab 100644 --- a/website/docs/user-guide/windows-native.md +++ b/website/docs/user-guide/windows-native.md @@ -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`.