fix(windows): restore CREATE_NO_WINDOW after d3d621f7c revert + add local-commit safety guard to hermes update - #86678
Conversation
…ccurrence) Installer's fast-forward-fails handling does 'git reset' to origin/main when local and origin diverge, silently discarding committed local-only work. This happened once already tonight (recovered via reflog then); the official updater just did it again. Re-committing on top of new origin HEAD (715d26c): - console-flash fix (windows_hide_flags on git/npm subprocess calls) - gateway RestartOnFailure fix (sh.Run synchronous + exit code propagation) - MCP header normalization fix (_normalize_headers) (cherry picked from commit a4b63e0)
_git() and _gh() in hermes_cli/web_git.py (backend for the desktop's coding-rail status, worktree lanes, review pane, and branch switch) were missing creationflags=windows_hide_flags() -- unlike the sibling bounded_git_probe() helper used elsewhere, these called subprocess.run directly with no window suppression. This is the actual source of the still-observed console flash after the update_cmd.py/main.py fix: confirmed via live process monitoring, a burst of 12+ git subprocess spawns (each with its own conhost/OpenConsole window) fires right at desktop app launch, tracing to this module's git status/branch/worktree probes. (cherry picked from commit 781d2e2)
…evert d3d621f banner.py's 7 startup-path git subprocess calls (prefetch_banner_data, runs on every CLI/TUI-gateway launch) and ssh-connection.ts's 2 ssh spawns were never re-fixed after d3d621f bulk-reverted three earlier windows-hide PRs across ~40 files. Also carries forward in-progress fixes to main.py (electron dist download, source-build gui launch), npm_engine.py (managed npm upgrade/probe), and update_cmd.py (backup-ref guard so a diverged fast-forward reset can no longer orphan committed local work). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> (cherry picked from commit 84bbaa5)
…revert d3d621f Root cause of the persisting desktop console flash: commit d3d621f bulk- reverted three earlier windows-hide fix PRs (NousResearch#53791/NousResearch#53810/NousResearch#53829) across ~40 files in June; only a handful were individually re-fixed afterward. This restores creationflags=windows_hide_flags() (or windows_detach_popen_kwargs() for the one long-running Popen in raft/adapter.py) on every remaining Windows-reachable subprocess/asyncio spawn found by a full-repo audit: cli.py (24 sites: worktree/branch-resolution helpers on the `hermes -w` startup path), kanban_db.py (6, gateway boot/orphan-reconcile), gateway/run.py (2, asyncio create_subprocess_exec/shell), clipboard.py (4), claw.py (2), mcp_catalog.py, dep_ensure.py, doctor.py, setup.py, dump.py, webhook.py, raft/adapter.py, managed_uv.py (11), mem0/_setup.py (7), honcho/client.py, iron_proxy.py (7), environments/docker.py (19), environments/ssh.py (11), uninstall.py, plugins_cmd.py, profiles.py, secret_sources/{bitwarden,onepassword}.py, discord/voice_mixer.py, contributor_audit.py, profile_distribution.py. Deliberately left visible (interactive/foreground by design): editor launches, `claude setup-token` OAuth login, relaunch.py's self-relaunch, and anything gated to non-Windows-only code paths (service_manager.py's s6-overlay branch, command.py's secret helper, profile-tui.py's pty import). Verified via ast.parse + py_compile on every changed file and a live import check on all touched non-CLI modules. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> (cherry picked from commit 0ede3ff)
The cmd\git.exe shim re-execs the real git in a fresh console, so it flashes a visible window on Windows even when the spawn passes windowsHide. Resolve the real bin\git.exe first (it honours the hidden-console flag) for the update-check and git-panel spawn paths. (cherry picked from commit cc5f7fc)
trevorgordon981
left a comment
There was a problem hiding this comment.
Mostly-mechanical restoration of the bulk-reverted Windows fix, plus two substantive additions (the VBS bWaitOnReturn change and the update backup guard). The restorations are fine. The two additions need work.
1. The Run(..., True) wait may be a no-op for the GUI-subsystem target (core claim likely unproven)
gateway_windows.py runs the gateway via pythonw.exe (the test asserts "pythonw.exe" in content). WScript.Shell.Run with bWaitOnReturn=True only blocks for console-subsystem executables — for a GUI-subsystem app it returns 0 immediately once the shell launches it, without waiting or returning the real exit code. If that's the case, the entire premise of the fix ("RestartOnFailure can finally see the real exit code") is defeated: wscript still returns 0 within milliseconds and a crash still isn't propagated.
The existing test only checks the text of the generated script (", 0, True)", "WScript.Quit exitCode") — it validates the content, not the behavior, so it passes even if the wait silently no-ops on the GUI-subsystem target. Needs a real Windows runtime verification that Run(..., True) on a pythonw launch returns the actual exit code.
2. The update backup guard degrades to a warning and still wipes data
update_cmd.py: when git branch -f hermes-local-fixes-backup HEAD fails, it prints a warning and falls through to git reset --hard anyway. So in exactly the scenario the PR claims to prevent (silent data loss of local commits), data can still be wiped and the user is merely told "may be lost." If the goal is a hard guarantee, the reset should abort (or the guard retry) when the backup ref can't be created.
Also: it's a single hardcoded ref pointing only at the current checked-out HEAD — commits on other local branches are still orphaned by the reset. And it's never rolled back, so it accumulates and would silently -f overwrite a user's own ref of that name.
3. _normalize_headers silently swallows malformed config
tools/mcp_tool.py: strings are split on the first ": " with no .strip(), and dicts missing name/value are dropped silently. "Key:Value" (no space), " Key: Val" (leading space), or {key,val} keys all vanish rather than surfacing. Behavior change too: dict([...]) previously raised ValueError on misuse (loud failure); now it's quiet misbehavior. That's arguably better UX but turns a crash into a silent misconfig — explicit decision + a unit test warranted for new parsing logic with no coverage.
4. Scope creep in raft/adapter.py on POSIX
_spawn_bridge now uses windows_detach_popen_kwargs(), which on POSIX adds start_new_session=True (setsid) — a real behavioral change beyond "restore hide flags," and on the bridge's process group at that. The PR is framed as re-adding hidden windows; call this out explicitly or it'll surprise maintainers expecting a no-op on non-Windows.
Tests
Minimal: one test file touches the VBS text content only. Nothing covers the backup guard (most safety-critical), _normalize_headers branches, the Electron resolveGitBinary reorder, or the ~60 windows_hide_flags() additions. The two non-mechanical changes (#1 backup guard, #2 VBS wait) are exactly where coverage is missing.
…Style Hidden Belt-and-suspenders alongside execText's windowsHide:true, which is not always reliable for powershell.exe -Command specifically. Fires up to 3x in rapid succession during every desktop startup.
…repaint flash Frameless window with titleBarOverlay repaints its Windows-Control-Overlay region with the raw native (unstyled black) frame for one compositor frame on WM_ACTIVATE -- most visible refocusing an already-open window (taskbar click, alt-tab). Re-pushing overlay options on 'focus' forces an immediate themed redraw instead of leaving the stale native frame visible.
The focus-repaint fix from the previous commit fired on every 'focus' event, but Electron fires 'focus' multiple times during a window's own startup/show sequence -- causing a 3-4x repaint burst right at launch, worse than the refocus flash it was meant to fix. Debounced to at most once per second.
trevorgordon981
left a comment
There was a problem hiding this comment.
Ping — I don't see a reply or new commits since my review. Four findings still stand, two of them (1 and 2) blocking from my side:
1. VBS Run(..., True) wait may be a no-op for the GUI-subsystem target (blocking). gateway_windows.py launches via pythonw.exe. WScript.Shell.Run with bWaitOnReturn=True only blocks for console-subsystem executables; for a GUI-subsystem app it returns immediately without the real exit code. If so, the whole "RestartOnFailure can see the real exit code" premise is defeated. The current test only checks the generated script text, not behavior. Needs a real Windows runtime verification that Run(..., True) on a pythonw launch propagates the actual exit code — or an explicit note on why GUI-subsystem still works.
2. Update backup guard degrades to a warning and still wipes data (blocking). update_cmd.py: when git branch -f hermes-local-fixes-backup HEAD fails it warns and falls through to git reset --hard anyway — the exact data-loss the PR claims to prevent. The reset should abort (or retry the guard) when the backup ref can't be created. Also: single hardcoded ref at current HEAD only (other local branches still orphaned), and it's never rolled back / would -f overwrite a user's own ref of that name.
3. _normalize_headers silently swallows malformed config. Strings split on first ": " with no .strip(); dicts missing name/value dropped silently. "Key:Value", " Key: Val", or {key,val} keys vanish. This also flips loud ValueError into silent misbehavior — explicit decision + unit tests for the new parsing (currently no coverage).
4. Scope creep in raft/adapter.py on POSIX. windows_detach_popen_kwargs() adds start_new_session=True (setsid) on POSIX — a real behavior change framed as "restore hide flags." Call it out explicitly, or keep it Windows-gated.
Tests: coverage is still the gap — nothing exercises the backup guard (most safety-critical), _normalize_headers branches, the Electron resolveGitBinary reorder, or the ~60 windows_hide_flags() additions. Findings 1 and 2 are the must-haves; 3 and 4 need at minimum an explicit decision and ideally tests.
…safety guard) into main Reconciles tonight's independently-recovered flash-fix commits with the more complete, already-open upstream PR NousResearch#86678. Clean automatic merge, zero conflicts -- local-only updater self-heal work in update_cmd.py (stale-lock self-heal, deferred-install recovery) preserved alongside the PR's safety guard that pins HEAD to a hermes-local-fixes-backup branch before any future 'hermes update' reset, so this exact incident (local commits silently wiped by git reset --hard origin/main) cannot recur.
fix(windows): restore CREATE_NO_WINDOW after d3d621f revert + add local-commit safety guard to hermes update
|
Problem
Console-flash regression: commit d3d621f (June 2026, "revert(windows): roll back terminal-popup PRs #53791 #53810 #53829") bulk-reverted three earlier windows-hide-flag fixes across ~40 files (1284 lines), stripping
creationflags=windows_hide_flags()from nearly every Windows subprocess spawn in the codebase. Only a handful of files were individually re-fixed since — everything else has been flashing a console window on Windows on everygit/npm/sshsubprocess call for months, including on every single CLI/TUI launch viabanner.py's startup version check.Silent data loss on update: separately,
hermes update's fast-forward-failure path doesgit reset --hard origin/<branch>when local and remote history have diverged, with no backup of the local HEAD first. Any committed-but-unpushed local work is silently discarded. This hit the same user twice in one week before the guard in this PR existed.Fix
creationflags=windows_hide_flags()(orwindows_detach_popen_kwargs()for long-running detached processes) across every remaining file hit by the June revert:banner.py(startup path, highest priority — 7 sites),cli.py(24 sites),kanban_db.py,gateway/run.py,clipboard.py,claw.py,mcp_catalog.py,dep_ensure.py,doctor.py,setup.py,dump.py,gateway/platforms/webhook.py,plugins/platforms/raft/adapter.py,managed_uv.py,plugins/memory/mem0/_setup.py,plugins/memory/honcho/client.py,agent/proxy_sources/iron_proxy.py,tools/environments/docker.py,tools/environments/ssh.py,hermes_cli/uninstall.py,plugins_cmd.py,profiles.py,agent/secret_sources/{bitwarden,onepassword}.py,plugins/platforms/discord/voice_mixer.py,scripts/contributor_audit.py,profile_distribution.py. ~60 call sites total.plugins_cmd.py's_run_plugin_githelper — introduced by a more recent refactor of the plugin-update flow, so it never had the flag to begin with (found while rebasing this branch onto current main).apps/desktop/electron/ssh-connection.ts(2 unhiddensshspawns for Remote/SSH backend profiles) viahiddenWindowsChildOptions().hermes update's reset path: beforegit reset --hard origin/<branch>, pins current HEAD to ahermes-local-fixes-backupbranch and prints the recovery command, so committed local work is always recoverable after a forced reset.RestartOnFailurefix (synchronoussh.Run+ exit-code propagation) and an MCP_normalize_headersfix (headers config coerced to dict before use) that were bundled with the safety-guard commit originally.Verification
ast.parse+py_compile).apps/desktop) rebuilds clean with the restored fixes.service_manager.py(Linux-only s6-overlay branch),agent/secret_sources/command.py(Windows early-return),agent/anthropic_adapter.py(Darwin-gated / intentional interactive OAuth),scripts/profile-tui.py(importspty, doesn't import on Windows at all),hermes_cli/relaunch.py(intentional visible self-relaunch).Reported and fixed by @Oc2cO.