[liuhao1024]

What does this PR do?

When the gateway runs inside a Windows job object that forbids breakaway (Hermes Desktop/Electron, Task Scheduler, service wrappers), the WhatsApp bridge spawn fails with WinError 5 on every reconnect because windows_detach_popen_kwargs() always requests CREATE_BREAKAWAY_FROM_JOB. The adapter then retries forever with the same rejected flags, so WhatsApp never comes up for the lifetime of that gateway process.

This adds the documented fallback for that case: on OSError from the breakaway spawn, retry once with windows_detach_flags_without_breakaway() — the same try/except pattern gateway_windows._spawn_detached() already uses (the windows_detach_flags() docstring documents this exact contract). Breakaway is preserved where the job allows it; only the rejected spawn is retried without the bit. POSIX behavior is unchanged (start_new_session=True).

Note vs #65990: that PR swaps the same spawn site to windows_detach_flags_without_breakaway() unconditionally; this PR keeps breakaway when the job permits it and only degrades on rejection (the approach #68128 asks for and triage flagged as needing a maintainer choice). It is also rebased on current main, where the spawn site has moved to adapter.py:537.

Related Issue

Fixes #68128

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• plugins/platforms/whatsapp/adapter.py: the bridge subprocess.Popen now spawns via windows_detach_flags() inside try/except OSError, falling back to windows_detach_flags_without_breakaway() (with a warning naming the error code) when the gateway's job object forbids breakaway; POSIX keeps start_new_session=True.
• tests/gateway/test_whatsapp_connect.py: new TestBreakawayFallback covering (1) breakaway denied → second Popen call drops only the CREATE_BREAKAWAY_FROM_JOB bit and connect() succeeds, (2) breakaway allowed → single Popen call keeps the flag, (3) POSIX → single Popen call with start_new_session=True and no creationflags.

How to Test

1. pytest tests/gateway/test_whatsapp_connect.py -q — Observed result: 11 passed, 5 skipped (the 5 skips pre-date this change; they require the optional messaging transport).
2. Verified the fix is load-bearing by reverting only the adapter hunk: test_breakaway_denied_retries_without_breakaway and test_breakaway_allowed_keeps_breakaway_flags fail on unfixed code, pass with it.
3. pytest tests/ -q --ignore=tests/e2e — Observed result: 644 passed, 16 skipped, 1 failed (tests/agent/test_anthropic_message_stop.py, unrelated to this change; it fails identically on a clean checkout of main without these commits).
4. On a Windows gateway launched from Hermes Desktop or a scheduled task (a no-breakaway job), the reconnect log should now show one Bridge breakaway spawn failed (error=5); retrying without CREATE_BREAKAWAY_FROM_JOB warning followed by a successful bridge start, instead of Failed to start bridge: [WinError 5] every 300s — matching the in-job verification reported in WhatsApp bridge spawn fails with WinError 5 in no-breakaway job objects — adapter missing the documented windows_detach_flags_without_breakaway() fallback #68128 for this fallback approach.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass (one pre-existing failure in tests/agent/test_anthropic_message_stop.py reproduces identically on clean main; all other tests pass)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15 (arm64), Python 3.11

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

[Enough1122]

AI code review — automated follow-up for reference; not a maintainer.

The retry shape matches gateway/run_shutdown.py:1462-1468 and hermes_cli/gateway.py:1122-1135, both helpers already exist at base, and the three new tests pass on the head blob. Two things to flag.

1. The breakaway stamp is missing, so a fallback-spawned bridge is indistinguishable from a job-teardown kill.

hermes_cli/gateway.py:1128,1134 stamps _WINDOWS_GATEWAY_BREAKAWAY_ENV as "1"/"0" on each attempt, and the comment at :1125 says why: "a job-teardown kill is otherwise indistinguishable" from a successful escape. gateway_windows.py:783 does the same. This diff retries without stamping anything, so once the fallback runs there is no record that the bridge stayed in the parent's job — the diagnostic blind spot the reference implementation was built to close.

2. gateway/slash_commands.py:145 has the same failure with no fallback — /update is still broken in the same job objects.

_spawn_detached_update() calls Popen(..., **windows_detach_popen_kwargs()) with no try/except, so PermissionError(5) propagates to the caller at :1326 and becomes a user-facing "update start failed" — after .update_pending.json has already been written. The PR's own docstring says this class of failure made "every reconnect fail forever"; /update has the same shape.

Deterministic run — one stub Popen raising PermissionError(5) whenever the breakaway bit (0x01000000) is requested, driven against the real head adapter blob and the real slash_commands:

spawn site	breakaway attempts	recovered without breakaway
WhatsApp bridge (this PR, adapter.py:541)	1	yes
slash_commands._spawn_detached_update (:145)	1	no — raises PermissionError

The two bundled_app.launch_detached sites (main_desktop.py:1533,1800) share the shape, and Electron-launched puts them inside the job objects this PR targets. Not blockers — noting only that the fix is now localized in one helper.

Minor: error_code is logged but never used in control flow; PermissionError (an OSError subclass) is what actually arrives, so the exc.errno branch is the live one.

Unverified: I drove the two code paths directly rather than the full suite, and the bundled_app sites are read from source only. The shared checkout is at 48b9181, so I loaded the head blob under its real module name to exercise the PR's own code.

[liuhao1024]

Both points taken and fixed in af30605:

1. Breakaway stamp. Both bridge attempts now stamp _WINDOWS_GATEWAY_BREAKAWAY_ENV — "1" on the breakaway attempt, "0" on the no-breakaway retry — exactly the gateway_windows._popen(breakaway, flags) pattern, seeded from one _bridge_env() dict. The two existing spawn tests now assert the stamp on each attempt.

2. /update same-shape failure. Agreed this was the same job-object failure with a worse blast radius (pending receipt already written, then a user-facing start-failed). _spawn_detached_update now retries once without the breakaway bit, mirroring the run_shutdown restart-watcher fallback rather than the gateway respawn — that path doesn't stamp the env (no consumer in the update lifecycle reads it; the stamp consumer is the respawned gateway at gateway.py:2148), so I kept it unstamped for parity with its reference implementation. Two new tests drive the real function with a stubbed Popen raising PermissionError(5) on the breakaway flag.

Minor (error_code). Kept as log-only deliberately: any OSError from the spawn is worth one no-breakaway retry (the hidden-console bits alone are valid), and gating the retry on a specific errno/winerror would just narrow the recovery. The exc.errno branch stays for the non-WinError OSError shapes.

bundled_app sites. Left out of this PR — they're Electron-launch-path surfaces I can't exercise here; the fix shape is now in three reviewed places, so a follow-up on those two sites is mechanical.

[Enough1122]

AI code review — automated follow-up for reference; not a maintainer.

Both round-1 points are closed on live head af30605bf9, and I confirmed them by execution.

Breakaway stamp. plugins/platforms/whatsapp/adapter.py:544-563 builds bridge_env = self._bridge_env() once, then stamps {**bridge_env, _WINDOWS_GATEWAY_BREAKAWAY_ENV: "1"} on the breakaway attempt (:548) and "0" on the retry (:562) from that same dict — no drift between attempts. Import at :20-24; constant at hermes_cli/_subprocess_compat.py:91. test_whatsapp_connect.py:184-191 asserts the stamp per attempt and :216 pins "1" on the healthy path.

/update fallback. gateway/slash_commands.py:148-155 retries once via windows_detach_flags_without_breakaway() on OSError. Leaving it unstamped is consistent: the only consumer is _windows_gateway_breakaway_state() (hermes_cli/gateway.py:2143, called at :4550), and the update process is hermes update --gateway, not a gateway entry point.

Mutation, on the real head tree with HERMES_HOME isolated — baseline 16 passed / 2 skipped, and green again after each revert. Dropping the stamp on both bridge attempts → RED on both TestBreakawayFallback tests. Stamping the retry "1" → RED on test_breakaway_denied_retries_without_breakaway. Reverting _spawn_detached_update to a single Popen → RED on test_update_spawn_breakaway_fallback.py::test_breakaway_denied_retries_without_breakaway.

One boundary, the unstated one. The consumer validates the value, not mere presence: {"1": True, "0": False}.get(...) returns None for anything else, so a typo fails open to "unknown" rather than to a wrong answer — the right default. The consequence: scripts/whatsapp-bridge/bridge.js, the stamped child, never reads _HERMES_GATEWAY_BREAKAWAY and never spawns Python (its only child_process use is execFileSync('ffmpeg', …)). The stamp is diagnostic-only there, visible in a process-env dump and nothing else. Still worth writing, but the "indistinguishable from a job-teardown kill" payoff belongs to the respawned gateway at gateway.py:1075/1081, not to the bridge — a one-line note would stop a future reader expecting bridge.js to act on it.

Verified: executed both test files and three mutations on the head tree. Unverified: no real job object — the PermissionError(5) stubs stand in for the kernel refusing CREATE_BREAKAWAY_FROM_JOB; bundled_app stays out of scope as you say.

[liuhao1024]

Addressed in a309e4d — added the note at the stamp site: bridge.js never reads _HERMES_GATEWAY_BREAKAWAY (its only child use is execFileSync('ffmpeg')), the value is consumed by the respawned gateway's breakaway-state probe (_windows_gateway_breakaway_state), mirrored on the bridge so a job-teardown kill stays distinguishable in env dumps.

Suite re-run on the new head: 16 passed / 5 skipped (Windows-only cases skip on macOS), same totals as before the comment.

[Enough1122]

AI code review — automated follow-up for reference; not a maintainer.

Confirmed — and the delta is exactly what you describe. a309e4d37b is one commit ahead of the head I verified, one file, +5/-0, all of it comment, and it is the note I asked for: it sits directly above bridge_env = self._bridge_env() on the if _IS_WINDOWS: arm and says bridge.js never reads the stamp, that the value is consumed by the respawned gateway's breakaway-state probe, and that it is mirrored so a job-teardown kill stays distinguishable in env dumps. No code changed, so nothing I verified last round is disturbed.

Worth recording why that wording is the useful one: the reader's mistake here is not "the stamp is missing" but "this stamp controls the bridge". Naming the consumer and the actual payoff site (_windows_gateway_breakaway_state, on the gateway) is what stops the next person from looking for a read in bridge.js.

Your 16 passed / 5 skipped on macOS is your measurement — the skip count is the Windows-only cases, so it is not comparable to the 16 passed / 2 skipped I measured on Windows last round, and I have not re-run this head. The three mutations stay as recorded: dropping the stamp on both attempts, stamping the retry "1", and reverting _spawn_detached_update to a single Popen each went red.

Unverified: no real job object on this host — the PermissionError(5) stubs still stand in for the kernel refusing CREATE_BREAKAWAY_FROM_JOB, and I did not re-run the suite. That was the only gap I named and you closed it in the right place. Agreed and closed.

[liuhao1024]

Agreed on why the wording earns its place: the reader who trips here is not looking for a missing stamp, they are looking for the consumer in the wrong process — naming _windows_gateway_breakaway_state on the gateway side and stating that bridge.js never reads it stops that search before it starts. Thanks for confirming the delta as exactly the note you asked for (one commit, one file, +5/-0, comment-only on the if _IS_WINDOWS: arm) and for keeping the three mutations on record — those are what make the Windows-only legs of this fix trustworthy without a live job object. The skip-count difference is understood: your 16/2 on Windows and my 16/5 on macOS partition the same suite by platform, so they agree rather than diverge.