fix(tools): harden spawn_local Windows job detachment

[VerbalChainsaw]

What does this PR do?

Uses Windows job breakaway for the primary spawn_local attempt so pipe-backed background children can survive owning-job teardown, while retaining a job-bound compatibility fallback when breakaway is denied.

ProcessRegistry.spawn_local (the pipe-backed, non-PTY local path) passed only windows_hide_flags() (CREATE_NO_WINDOW), leaving the child inside the parent's job object.

Change

1. The primary spawn uses windows_detach_popen_kwargs(). On Windows it breaks the child away from the owning job (CREATE_BREAKAWAY_FROM_JOB plus detached flags) so it survives that job's teardown. On POSIX it is start_new_session=True (the session behavior current main already had, now supplied by the helper, so the hard-coded kwarg is dropped and there is no duplicate).
2. Fallback: if a restrictive job denies breakaway (OSError), retry once with windows_detach_flags_without_breakaway(). This preserves launch compatibility, but the child stays job-bound and cannot be guaranteed to survive the owning job's teardown.
3. POSIX errors and a Windows dual failure propagate, so spawn_local never registers a session that falsely claims a process started, and nothing (command, env, cwd) is logged.
4. The post-spawn setup-failure cleanup uses the start-time-validated tree terminator on Windows, since the detached child owns descendants that a bare proc.kill() would orphan. Adds close_fds=True.

Scope: the pipe-backed, non-PTY path only. use_pty=True (pywinpty) is untouched.

Tests

• Mocked Popen behavioral suite: primary breakaway; breakaway-denied retry (argv and every non-creationflags kwarg preserved, and the flag sets differ by exactly the breakaway bit); dual-failure surfacing (the fallback's exception propagates and no session is registered); post-setup-failure tree-kill; POSIX single-session; POSIX error not retried.
• Native Windows integration (pure ctypes, no pywin32): assigns the test process to a BREAKAWAY_OK job and asserts the real spawn_local child is not a job member, with output capture verified. It passes on this branch and reproduces the failure on pristine main.

[VerbalChainsaw]

cc @teknium1 — this is the agent-path half of the same fix you landed in #40909 for the gateway watcher. The agent's terminal(background=true) path was the one call site #40909 missed; this PR closes that gap and should be mergeable independently of the broader #40899 Windows Service refactor. Heads-up that the canonical test runner scripts/run_tests.sh has a separate Windows-compat issue (tracked in a local scratchpad, not part of this PR); the bg-detach tests were verified via direct pytest per AGENTS.md's fallback. The deterministic regression test catches the bug in 0.30s via IsProcessInJob if you want to spot-check the mechanism.

[liuhao1024]

Positive verification — clean review, no issues found.

Reviewed the full diff: the change correctly replaces windows_hide_flags() with windows_detach_popen_kwargs() in process_registry.spawn_local, matching the pattern already used by the other local-spawn call sites (hermes_cli/gateway.py, gateway/run.py, gateway/slash_commands.py). The redundant preexec_fn=os.setsid on POSIX is also correctly removed since windows_detach_popen_kwargs() returns start_new_session: True which covers that.

The static source-text regression tests are a smart approach — they catch a revert of the helper call on any CI platform, not just Windows. The integration test with job-object setup is thorough and deterministic.

One note: the test file docstring references test_spawn_local_child_survives_60s but the actual test function is named test_spawn_local_child_breaks_away_from_parent_job. Minor docstring mismatch.

[VerbalChainsaw]

Addressed the review nit (commit b8377c3ec): the source-file comment in tools/process_registry.py now points to BOTH regression tests explicitly, with a note that test_spawn_local_child_breaks_away_from_parent_job is the load-bearing deterministic guard and test_spawn_local_child_survives_60s is the symptom-level 60s end-to-end. Force-pushed to the same branch. Re-verified on a fresh shallow clone of origin/main: 3 fast tests pass in 0.56s, 60s test passes, unpatched spawn_local fails the deterministic test in 0.30s with REGRESSION message.

[tonydwb]

Code Review: Comment. Large fix (490 lines) for Windows background subprocess detachment. Due to size and cross-platform nature, recommend thorough manual review before merge.

[teknium1]

Thanks for tracing the local terminal(background=true) path; the premise is still present on current main: tools/process_registry.py:762-775 sends only windows_hide_flags() to its local Popen, and tools/terminal_tool.py:2360-2380 routes local background jobs through that method.

Problems

• The proposed use of windows_detach_popen_kwargs() needs the established breakaway-denied fallback. hermes_cli/_subprocess_compat.py:138-144 documents that CREATE_BREAKAWAY_FROM_JOB can raise PermissionError/OSError; hermes_cli/gateway.py:854-870 catches that failure and retries with windows_detach_flags_without_breakaway(). The PR diff adds no equivalent retry, so a restrictive parent job changes this path from a best-effort background launch into a launch failure.

Suggested changes

• Add the Windows-only retry and a mocked-Popen regression test for it.
• Consider replacing the two duplicate source-text checks with behavioral assertions on the kwargs passed to Popen; the current tests unnecessarily constrain equivalent future refactors.

Automated hermes-sweeper review.

[teknium1]

windows_detach_popen_kwargs() includes CREATE_BREAKAWAY_FROM_JOB, which can raise OSError when the parent job disallows breakaway (hermes_cli/_subprocess_compat.py:138-144). Please mirror the retry-without-breakaway fallback already used by hermes_cli/gateway.py:854-870; otherwise this turns some Windows background launches into immediate failures.

[VerbalChainsaw]

Hardened the Windows job-detachment path and validated the final diff against current main.

• The primary spawn breaks the child away from the owning job via windows_detach_popen_kwargs(). On OSError it retries once without breakaway (windows_detach_flags_without_breakaway()), mirroring gateway_windows._spawn_detached. The fallback keeps the launch working but stays job-bound, so I do not claim it survives an owning-job teardown.
• Current main already hard-codes start_new_session=True here, so spreading windows_detach_popen_kwargs() on its own would have duplicated it (TypeError on POSIX). I dropped the literal kwarg and let the helper supply the one session kwarg. No preexec_fn.
• POSIX errors and a Windows dual failure propagate (no session registered, command never logged). The post-spawn setup-failure path now tree-terminates on Windows too, since the detached child owns descendants that a bare kill would orphan. Added close_fds=True.
• Replaced the source-text checks with a mocked Popen behavioral suite (retry, exact breakaway flag delta, dual-failure surfacing, tree-kill on setup failure, POSIX single-session) plus a pure-ctypes native Windows integration test asserting the real child is not a job member. It passes here and reproduces the failure on pristine main.