Skip to content

fix(cli): add CREATE_BREAKAWAY_FROM_JOB to windows_hide_flags() - #55667

Closed
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/windows-hide-flags-breakaway
Closed

fix(cli): add CREATE_BREAKAWAY_FROM_JOB to windows_hide_flags()#55667
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/windows-hide-flags-breakaway

Conversation

@AlexFucuson9

Copy link
Copy Markdown
Contributor

Summary

Adds CREATE_BREAKAWAY_FROM_JOB to windows_hide_flags() so subprocess console windows don't flash when Hermes runs inside an Electron/Tauri wrapper.

Problem (P2 #55604)

windows_hide_flags() returns only CREATE_NO_WINDOW (0x08000000). Inside an Electron Job Object, CREATE_NO_WINDOW is silently ignored — cmd.exe flashes on every subprocess spawn.

Fix

Add CREATE_BREAKAWAY_FROM_JOB (0x01000000) so child processes escape the parent Job Object. This flag already exists in the module and is used by windows_detach_flags().

Changes

  • hermes_cli/_subprocess_compat.py: 1 line changed

Fixes #55604

windows_hide_flags() returns only CREATE_NO_WINDOW (0x08000000).
When Hermes runs inside an Electron wrapper (or any Job Object),
CREATE_NO_WINDOW alone is silently ignored and cmd.exe windows
flash on every subprocess spawn.

Add CREATE_BREAKAWAY_FROM_JOB (0x01000000) so child processes
escape the parent Job Object before the hide flag takes effect.

Fixes NousResearch#55604
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #55613 — both make the identical one-line change (return _CREATE_NO_WINDOW | _CREATE_BREAKAWAY_FROM_JOB) to windows_hide_flags() for #55604. #55613 came first and additionally adds a docstring rationale and a regression test, so it's the canonical version.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the proposed flag change. There is a compatibility issue with applying it through this shared helper.

Problems

  • hermes_cli/_subprocess_compat.py:138-144 documents that CREATE_BREAKAWAY_FROM_JOB can make CreateProcess raise PermissionError/OSError when a parent job disallows breakaway. The changed windows_hide_flags() value reaches unguarded synchronous process creation in tools/environments/base.py:145-153 and tools/environments/local.py:1074-1088; neither retries without the bit.
  • The PR adds no regression coverage. Existing tests intentionally cover the separate detached-spawn fallback in tests/tools/test_windows_native_support.py:582-625.

Suggested changes

  • Validate the Electron path on native Windows, then keep breakaway targeted to a path that can retry without it when the job rejects breakaway.
  • Add a Windows-specific behavioral test for that fallback, rather than only asserting a creation-flag bit.

Automated hermes-sweeper review.

if not IS_WINDOWS:
return 0
return _CREATE_NO_WINDOW
return _CREATE_NO_WINDOW | _CREATE_BREAKAWAY_FROM_JOB

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper feeds synchronous terminal spawns (for example tools/environments/base.py:145-153). The module documents that a job which disallows breakaway makes this flag raise OSError, but these callers do not retry without it. Please keep this hidden-only helper unchanged or add a targeted path with the documented no-breakaway fallback.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing after the class-level fix in PR #70205 (merged as 0dbf639) — the premise here doesn't hold against Win32 semantics, and the underlying symptom is addressed at a different layer.

Two problems with adding CREATE_BREAKAWAY_FROM_JOB to windows_hide_flags():

  1. Job objects don't affect console/window creation. CREATE_NO_WINDOW is not "silently ignored inside a job object" — jobs govern process lifetime and limits, not console allocation. The flag that actually neutralizes CREATE_NO_WINDOW is DETACHED_PROCESS (documented in the Process Creation Flags spec), and the real cause of the Electron/desktop flashes was the parent being console-less (GUI-subsystem pythonw.exe / DETACHED daemons), so every console-subsystem child had to allocate its own visible console. That's fixed at the root: commit aa2ae36 launches the desktop backend as hidden-console python, and fix(windows): hidden-console daemons — extend the parent-console flash fix to every detached spawn path #70205 extends the same to all detached daemon paths.

  2. Breakaway on every short-lived spawn is harmful. windows_hide_flags() feeds ~40 synchronous capture-output call sites. In a job that doesn't set JOB_OBJECT_LIMIT_BREAKAWAY_OK, CREATE_BREAKAWAY_FROM_JOB makes CreateProcess fail with ERROR_ACCESS_DENIED — and none of those sites have a retry, so probes and helpers would start crashing in restrictive Windows Terminal/container/kiosk configs. Short-lived children being torn down with the parent's job is also the desired behavior for helpers.

If a flash persists at a specific spawn site on current main, that's worth a fresh issue with the process name from the flash — it would point at a leg the parent-console fix doesn't cover. Thanks for digging into the Windows spawn behavior.

@teknium1 teknium1 closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

windows_hide_flags() missing CREATE_BREAKAWAY_FROM_JOB — cmd.exe windows flash in Electron desktop apps (Hermes One, etc.)

3 participants