Skip to content

fix: hide WhatsApp adapter helper subprocesses on Windows - #75224

Open
Clawdy-ast wants to merge 1 commit into
NousResearch:mainfrom
Clawdy-ast:fix/windows-whatsapp-console-flash
Open

fix: hide WhatsApp adapter helper subprocesses on Windows#75224
Clawdy-ast wants to merge 1 commit into
NousResearch:mainfrom
Clawdy-ast:fix/windows-whatsapp-console-flash

Conversation

@Clawdy-ast

@Clawdy-ast Clawdy-ast commented Jul 31, 2026

Copy link
Copy Markdown

What does this PR do?

Stops visible console windows flashing on Windows when the gateway runs under pythonw.exe (console-less) and the WhatsApp adapter spawns synchronous helper processes. The worst offender is the Node requirements probe in check_whatsapp_requirements(), which the gateway's channel monitor invokes every ~5 minutes — on an affected machine this produces a visible "Terminal" window flash on a permanent ~5-minute cycle whenever the WhatsApp platform is enabled but its bridge isn't connected.

Empirically traced on Windows 11: a visible-window watcher (EnumWindows polling) captured the flash bursts on a perfect 306-second cadence, each coinciding with a node.exe --version child of the gateway pythonw process. After this fix: zero visible windows across multiple monitor cycles.

Rather than hand-passing creationflags at each call site, this adds a module-level _run_hidden() wrapper that injects creationflags=windows_hide_flags() by default (a no-op returning 0 on POSIX — hermes_cli/_subprocess_compat.py provides the helper), and routes all seven synchronous subprocess.run sites in the adapter through it. This bug class has recurred repeatedly (#53282, #56747, #63698, #68457, and now this PR); with the wrapper, a future spawn site added to this module inherits the fix by default instead of depending on reviewer vigilance.

The long-lived bridge Popen is untouched — it already correctly uses windows_detach_popen_kwargs().

Related Issue

No open issue. Same Windows console-flash class as closed #53282, #56747, #63698, #68457 — this PR covers the WhatsApp adapter call sites those fixes did not reach.

Type of Change

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

Changes Made

  • plugins/platforms/whatsapp/adapter.py — add _run_hidden() (a subprocess.run wrapper defaulting creationflags=windows_hide_flags()) and route all seven synchronous spawn sites through it: the Node --version probe, the taskkill in _terminate_bridge_process(), the netstat/taskkill pair in _kill_port_process(), the lsof/ss probes in _listener_pids_on_port(), and the npm install in connect(). The existing UTF-8 decoding (encoding='utf-8', errors='replace') at every site is preserved.
  • tests/gateway/test_whatsapp_adapter_hides_console_window.py (new) — behavioral tests: each Windows-reachable path is executed with a mocked subprocess.run and the captured kwargs asserted (per review feedback, no source/AST inspection). Includes a contract test on _run_hidden itself as the recurrence guard. POSIX-only paths (lsof/ss) are not asserted as Windows behavior.
  • tests/gateway/test_whatsapp_connect.py — existing bridge-termination assertion updated for the new kwarg.

How to Test

  1. pytest tests/gateway/test_whatsapp_adapter_hides_console_window.py tests/gateway/test_whatsapp_connect.py -q
  2. The new tests fail against the pre-fix adapter (verified red: 6 failures) and pass with this change (verified green: 18 passed).
  3. On Windows: run the gateway under pythonw with WhatsApp enabled and its bridge not connected; pre-fix, a console window flashes every ~5 minutes on the monitor cycle — post-fix, none.

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 the full gateway suite locally (pytest tests/gateway/ -q) on the rebased branch
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 (live gateway, before/after verification)

Documentation & Housekeeping

  • I've updated relevant documentation — N/A (no user-facing behavior change beyond removing the window flash)
  • I've updated cli-config.yaml.example — N/A (no config keys changed)
  • I've updated CONTRIBUTING.md or AGENTS.md — N/A (no architecture change)
  • I've considered cross-platform impact (Windows, macOS): windows_hide_flags() returns 0 on non-Windows by design, and POSIX Popen accepts creationflags=0
  • I've updated tool descriptions/schemas — N/A (no tool behavior changed)

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins 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 labels Jul 31, 2026

@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 tracing the remaining WhatsApp console-flash paths. The production change addresses real omissions on current main: plugins/platforms/whatsapp/adapter.py:223, :349, and :554 call synchronous helpers without creationflags, while hermes_cli/_subprocess_compat.py:212 provides the intended Windows-only CREATE_NO_WINDOW helper.

Problems

  • tests/gateway/test_whatsapp_windows_subprocess_flags.py:20 parses inspect.getsource() with ast. This is a source-shape test, which is explicitly banned by AGENTS.md:1382; it will not validate the runtime subprocess contract.
  • The branch conflicts with current main. Preserve current UTF-8 subprocess decoding at plugins/platforms/whatsapp/adapter.py:226, :352, and :558 when applying the three flags.

Suggested changes

  • Replace the AST test with behavioral mock-based tests that execute each path and assert subprocess.run(..., creationflags=windows_hide_flags()). The existing disconnect assertion at tests/gateway/test_whatsapp_connect.py:395 is the right pattern for the taskkill path.

Automated hermes-sweeper review.

def _subprocess_run_calls(function):
"""Return direct ``subprocess.run`` call nodes from *function*."""
tree = ast.parse(textwrap.dedent(inspect.getsource(function)))
return [

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.

Please replace this source-inspection test with behavioral tests that mock subprocess.run, execute each relevant path, and assert its runtime kwargs. AGENTS.md:1382 explicitly bans tests that read production source because they couple to implementation shape rather than behavior.

The gateway commonly runs under pythonw.exe (console-less); a console
application spawned from it without CREATE_NO_WINDOW creates a visible
window. Worst offender: the Node requirements probe invoked by the
channel monitor every ~5 minutes while the bridge is down.

Route all seven synchronous subprocess.run sites in the WhatsApp
adapter through a module-level _run_hidden() wrapper that injects
creationflags=windows_hide_flags() by default (0 on POSIX), so future
spawn sites inherit the fix. Fifth fix in this console-flash class
(NousResearch#53282, NousResearch#56747, NousResearch#63698, NousResearch#68457).

Behavioral regression tests execute each Windows-reachable path with a
mocked subprocess.run and assert the captured kwargs; a contract test
on the wrapper guards recurrence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Clawdy-ast
Clawdy-ast force-pushed the fix/windows-whatsapp-console-flash branch from bf8c654 to 783c97a Compare August 3, 2026 11:35
@Clawdy-ast

Copy link
Copy Markdown
Author

Thanks for the review — both findings were valid, and the encoding one would have been a silent regression. Point-by-point resolution on the rebased branch:

1. AST/source-shape test (AGENTS.md violation) — RESOLVED, removed.
Replaced with behavioral tests in tests/gateway/test_whatsapp_adapter_hides_console_window.py: each Windows-reachable path is executed with a mocked subprocess.run and the captured kwargs asserted, following the existing disconnect assertion pattern in test_whatsapp_connect.py you pointed to. No test reads source text.

2. Branch conflict / UTF-8 decoding regression — RESOLVED, rebased.
Rebased onto current main. encoding='utf-8', errors='replace' is preserved verbatim at every touched call site (verifiable in the diff: no decoding kwarg is removed anywhere).

One scoping decision made without you, flagged for explicit review: the lsof/ss probes in _listener_pids_on_port() are reachable only from the POSIX branch of _kill_port_process() (adapter.py, the else of the _IS_WINDOWS check), so I did not write Windows-behavior tests for them — asserting CREATE_NO_WINDOW on a POSIX-only path would test an unreachable state. They still route through the wrapper (harmless: flags are 0 on POSIX).

One structural change beyond the review's scope: this is the fifth fix in this console-flash class (#53282, #56747, #63698, #68457). The per-call-site kwargs are now factored into a module-level _run_hidden() wrapper that injects creationflags=windows_hide_flags() by default; all seven synchronous spawn sites route through it, and a contract test on the wrapper replaces the recurrence guard the deleted AST test was providing. New spawn sites in this module now inherit the fix by default.

Verification on the rebased branch (Windows 11, Python 3.13):

  • New test file: red against the pre-fix adapter (6/6 fail), green with the fix.
  • test_whatsapp_adapter_hides_console_window.py + test_whatsapp_connect.py: 18/18 pass.
  • Full tests/gateway/ suite: all failures also reproduce on a pristine upstream/main checkout in the same environment (verified by side-by-side run), i.e. none are introduced by this branch. List available on request.

To make the next round unambiguous, three yes/no questions:

  1. Is the _run_hidden() wrapper refactor acceptable in this PR, or should I split it out and keep this PR to the minimal per-site kwargs?
  2. Is skipping Windows-behavior tests for the POSIX-only lsof/ss paths the correct reading of the testing policy?
  3. Should the pre-existing tests/gateway/ failures on main (reproducible without this branch) be filed as a separate issue, and if so, is there a preferred label?

If all three are "yes / yes / file it", this PR needs no further changes from my side.

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:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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: Console window flashes on every subprocess call (missing CREATE_NO_WINDOW)

3 participants