Skip to content

fix(whatsapp): retry bridge spawn without CREATE_BREAKAWAY_FROM_JOB on access-denied - #87777

Open
Aperk357 wants to merge 1 commit into
NousResearch:mainfrom
Aperk357:fix/whatsapp-bridge-breakaway-fallback
Open

fix(whatsapp): retry bridge spawn without CREATE_BREAKAWAY_FROM_JOB on access-denied#87777
Aperk357 wants to merge 1 commit into
NousResearch:mainfrom
Aperk357:fix/whatsapp-bridge-breakaway-fallback

Conversation

@Aperk357

Copy link
Copy Markdown

Summary

  • plugins/platforms/whatsapp/adapter.py's subprocess.Popen call for the WhatsApp bridge used windows_detach_popen_kwargs() unconditionally, which sets CREATE_BREAKAWAY_FROM_JOB.
  • When the parent Electron process's job object doesn't permit breakaway, CreateProcess fails with ERROR_ACCESS_DENIED, surfaced as PermissionError: [WinError 5] Access is denied — observed 1,271+ times in gateway.log, recurring every ~5 minutes.
  • Every other detached-spawn call site already retries without the breakaway flag on OSError (canonical pattern: hermes_cli/gateway_windows.py::_spawn_detached). The WhatsApp bridge's Popen call was the one site missing this fallback.
  • This PR mirrors _spawn_detached's exact pattern: wrap the Popen call in try/except OSError and retry with windows_detach_flags_without_breakaway() on Windows; re-raise unchanged on non-Windows (where there's no breakaway flag to strip).

Test plan

  • Existing tests/gateway/test_whatsapp_connect.py suite passes unchanged (11 passed, 1 skipped pre-existing).
  • Added TestBridgePopenBreakawayFallback with two new tests:
    • test_retries_without_breakaway_on_permission_error: mocks subprocess.Popen to raise PermissionError on the first call, verifies a second call is made with creationflags that exclude CREATE_BREAKAWAY_FROM_JOB (0x01000000), and that connect() succeeds using the retried process.
    • test_does_not_retry_on_non_windows: verifies the retry path is skipped on non-Windows and the original OSError behavior (single Popen call) is preserved.
  • Full new suite run: python -m pytest tests/gateway/test_whatsapp_connect.py -q → 13 passed, 1 skipped.
  • Not tested against the live running bridge (would risk interfering with production Hermes processes on this host).

🤖 Generated with Claude Code

…n access-denied

The WhatsApp bridge's subprocess.Popen call used
windows_detach_popen_kwargs() unconditionally, which sets
CREATE_BREAKAWAY_FROM_JOB. When the parent Electron process's job
object doesn't allow breakaway, CreateProcess fails with
ERROR_ACCESS_DENIED (PermissionError: [WinError 5] Access is denied),
and the bridge never started. Every other detached-spawn call site in
this codebase already retries without the breakaway flag on OSError
(see gateway_windows.py::_spawn_detached); the WhatsApp adapter's
Popen call was missing that fallback, causing the failure to repeat
every ~5 minutes indefinitely (1,271+ occurrences in gateway.log).

Mirrors _spawn_detached's exact pattern: wrap the Popen call in
try/except OSError and retry with
windows_detach_flags_without_breakaway() on Windows.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(whatsapp): retry bridge spawn without CREATE_BREAKAWAY_FROM_JOB on access-denied

  • tests/gateway/test_whatsapp_connect.py test_retries_without_breakaway_on_permission_error: assert "creationflags" in popen_calls[0] or True is a tautology — X or True always evaluates to True, so the first-attempt assertion can never fail. It should be assert "creationflags" in popen_calls[0].
  • plugins/platforms/whatsapp/adapter.py connect(): the except OSError is broad — on Windows any Popen OSError (e.g. ENOENT if the node/bridge path is wrong) triggers a no-breakaway retry that will fail again for the same reason before propagating. Harmless (one wasted spawn), but narrowing to PermissionError (the documented WinError 5 case) would keep the fallback semantically precise and match the comment's intent.
  • The retry re-specifies argv/env/stdout/stderr manually. Functionally equivalent today since windows_detach_popen_kwargs() returns only creationflags on Windows, but composing it as {**windows_detach_popen_kwargs(), "creationflags": windows_detach_flags_without_breakaway()} would keep a single source of truth for the spawn kwargs and stay correct if that helper ever grows more keys.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/whatsapp WhatsApp Business adapter platform/windows Native Windows-specific behavior or breakage P3 Low — cosmetic, nice to have sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Aug 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #65990: both repair the WhatsApp bridge breakaway failure on Windows. This PR preserves breakaway when available and retries without it on access denial; #65990 always omits breakaway. Maintainer selection is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/whatsapp WhatsApp Business adapter platform/windows Native Windows-specific behavior or breakage 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.

3 participants