fix(cron): Windows detach with constructor-only compatibility fallback and file-backed output capture

[VerbalChainsaw]

Problem

On Windows, cron pre-run / no_agent scripts are launched with CREATE_NO_WINDOW only, so they remain tied to the parent's job-object lifetime. Controlled shutdown is drained elsewhere, but terminal/Desktop teardown or process death can still kill a mid-run script.

Why the previous revision was wrong

The previous revision had three structural defects:

1. it applied a detach helper unconditionally and changed POSIX session semantics;
2. it retried the entire subprocess.run() lifecycle after OSError, so an error after child creation could execute a side-effectful script twice;
3. it kept capture_output=True, leaving the detached child writing to anonymous pipes whose reader died with the parent.

Change

The Windows path now uses an explicit Popen helper:

• full detach flags on the primary constructor attempt;
• at most one constructor fallback, only when a parent-in-job launch reports WinError 5;
• no fallback after a Popen object exists;
• inherited unnamed TemporaryFile handles for stdout/stderr;
• stdin=DEVNULL;
• UTF-8 replacement decoding and newline normalization;
• unchanged sanitizer, environment overlay, timeout, redaction, and return contract;
• unchanged POSIX subprocess.run path with no new session behavior.

Windows job-object behavior

Breakaway refusal is not represented identically on every Windows/job configuration.

• Some configurations report access denied during child construction. When that occurs as WinError 5 while the parent is in a job, the constructor is retried once without CREATE_BREAKAWAY_FROM_JOB.
• On the native Windows 11 nested-job configuration tested here, construction succeeded but Windows silently retained the child in each job that forbade breakaway. No fallback or INFO log occurs in that case.
• A child launched from a permitting nested job escaped that job and survived its teardown.

Therefore, survival is guaranteed only for teardown of jobs that permit breakaway. A child retained by a forbidding job may still be reaped when that job closes.

Why file-backed output is necessary

Native testing reproduced the anonymous-pipe failure: quiet children survived parent death, while output-producing modes failed on their next write. The file-backed runner completed all eight tested modes with all side effects and zero leaked temp files.

The change also avoids two pipe-lifetime bugs while the parent is alive:

• a daemonized worker holding the inherited pipe can make subprocess.run() falsely wait until timeout;
• on Windows, timeout cleanup can wedge while communicate() waits on a descendant-held pipe.

Waiting on the direct process with file-backed output removes those pipe-EOF dependencies. Output written by a grandchild after the direct child exits is not collected.

Guarantees and limitations

Guaranteed:

• at-most-once child execution per invocation;
• durable output sink across parent death;
• survival from teardown of a job that permits breakaway;
• no persistent output file after the last handle closes;
• scheduler claims prevent redispatching the occurrence solely to recover output.

Not guaranteed:

• restart recovery of orphaned output/status;
• survival from a job that forbids breakaway;
• descendant-tree termination on timeout.

Evidence

• focused behavioral suite: 53 passed and one privilege-dependent symlink skip on native Windows; the skip reproduced on unpatched main;
• native parent-death and temp-leak tests passed;
• corrected restrictive-job test accepts both documented Windows constructor outcomes while requiring one script execution;
• native anonymous-pipe baseline reproduced the output failure;
• native file-backed matrix completed 8/8 with zero leaks;
• post-spawn fault probe reported one spawn and one side effect;
• adjacent shutdown/claim suites passed.

[alt-glitch]

This was generated by AI during triage.

Companion fix to #43253 — both replace the legacy windows_hide_flags() detach pattern with windows_detach_popen_kwargs() + OSError fallback. This one targets cron/scheduler.py:1047; #43253 targets tools/environments/local.py. Both complete the same Windows job-object detach migration (parity with the merged hermes_cli/gateway.py pattern and #42993). Not duplicates — different call sites; both should land.

[tonydwb]

Code Review Summary

Verdict: Approved

✅ Looks Good

• Windows detach-flag fix for cron/scheduler.py: Replaces the legacy windows_hide_flags() with windows_detach_popen_kwargs(). On restrictive Windows job objects, the old pattern caused cron-spawned scripts to be silently reaped when the parent Hermes process exited — now they properly break away from the job object.
• OSError nested fallback with security awareness: The dual-attempt pattern (primary with breakaway flag → fallback without) mirrors the canonical hermes_cli/gateway.py:716-742. On the rare double-failure path, the code logs a WARNING with only the argv head (not the full argv list) — this is explicitly documented as a security measure since cron scripts can embed secrets in environment variables passed inline.
• Comprehensive AST-based tests: Three tests verify (1) the new helper is used and the old one removed, (2) subprocess.run is wrapped in try/except OSError, and (3) the fallback uses windows_detach_flags_without_breakaway(). These are robust against future refactoring attempts.
• Well-scoped: 2 files, +253/-11, single focused concern. Pairs with sibling PR #43253 (which applies the same fix to tools/environments/local.py).

---

Reviewed by Hermes Agent

[teknium1]

Thanks for tracing this to the cron script runner. Current main still uses windows_hide_flags() at cron/scheduler.py:2099, so the Windows detach premise remains relevant.

Problems

• The new unconditional windows_detach_popen_kwargs() call changes POSIX behavior: the helper returns {"start_new_session": True} on non-Windows (hermes_cli/_subprocess_compat.py:204-234), whereas the current cron subprocess.run has no session kwarg (cron/scheduler.py:2099-2107). The detach/retry path should be Windows-only unless this POSIX behavior change is intentionally designed and covered.
• The proposed tests inspect source/AST shape rather than exercising _run_job_script or an OSError retry. Please replace them with mocked runtime behavior tests for the full-flags first call, no-breakaway retry, and dual-failure result.
• The PR base predates the current subprocess environment sanitizer. Salvage the change into cron/scheduler.py:2096-2107 without losing _sanitize_subprocess_env(os.environ.copy()).

This is an automated hermes-sweeper review.

[teknium1]

windows_detach_popen_kwargs() returns {"start_new_session": True} on POSIX, while the current cron script runner supplies no such kwarg. Please gate this detach helper and its fallback to Windows so this Windows fix does not alter Linux/macOS process-session semantics.

[VerbalChainsaw]

You were right. The reworked cron path does not call windows_detach_popen_kwargs() unconditionally. POSIX remains on the existing subprocess.run path with no new start_new_session behavior, and the Windows-only helper preserves the sanitized environment and interpreter overlay.

The rework also closes two deeper defects:

1. The earlier except OSError: retry subprocess.run(...) boundary could relaunch after a child already existed. The new fallback is confined to child construction. Once Popen returns, wait, timeout, output-read, decode, redaction, and cleanup failures cannot create another child. Native fault injection produced one spawn and one side effect under the failure that previously produced two.

2. A detached child using anonymous pipes still depends on its parent-owned reader. Native Windows testing showed output-producing children failing after parent death, while the file-backed runner completed all eight modes with zero temp-file leaks.

One native finding changed the wording, not the architecture: restrictive nested jobs on the tested Windows 11 build did not raise WinError 5. Child construction succeeded while Windows silently retained the child in the forbidding job. A permitting nested job allowed escape and survival. The compatibility fallback remains for configurations that do surface WinError 5, but the native test now asserts the cross-version invariant (one successful execution with either one full-flags constructor attempt or one constructor fallback) rather than requiring a particular OS error mechanism.

The public claim is therefore narrow: a child survives teardown of jobs that permit breakaway; a child retained by a forbidding job may still be reaped. Orphan output/status is not recovered after restart.