Skip to content

fix(mcp,model_server,hooks,ingest): Windows subprocess portability#351

Open
Huntehhh wants to merge 1 commit into
buildingjoshbetter:mainfrom
Huntehhh:fix/windows-subprocess-portability
Open

fix(mcp,model_server,hooks,ingest): Windows subprocess portability#351
Huntehhh wants to merge 1 commit into
buildingjoshbetter:mainfrom
Huntehhh:fix/windows-subprocess-portability

Conversation

@Huntehhh
Copy link
Copy Markdown
Contributor

Summary

Replaces six POSIX-only subprocess patterns with cross-platform equivalents so TrueMemory runs correctly on Windows. Also fixes a _log_file handle-leak and swaps two ASR-blocked test invocations with python.exe -m equivalents.

Changes per file

truememory/mcp_server.py (~line 1181 — backlog drainer Popen)

  • start_new_session=True guarded with hasattr(os, "setsid") (POSIX probe). Windows branch uses creationflags=CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS.
  • _log_file open/Popen wrapped in try/finally so the file handle closes even if Popen() raises.

truememory/model_server.py (whole file)

  • Replaces hardcoded socket.AF_UNIX with a platform-conditional: AF_UNIX on POSIX, AF_INET TCP on Windows.
  • Windows path: binds 127.0.0.1:0 (OS-assigned port), writes the port to ~/.truememory/model_server.port as a sidecar discovery file.
  • SIGHUP registration guarded with hasattr(signal, "SIGHUP") — the signal doesn't exist on Windows.
  • Idle-checker dummy-wakeup uses whichever socket family the server bound.

truememory/model_client.py (whole file)

  • Mirrors server: _get_server_address() returns the SOCK_PATH string on POSIX or ("127.0.0.1", port) on Windows by reading PORT_PATH.
  • _start_server Popen: same start_new_session / creationflags guard.
  • _send_request: constructs AF_UNIX or AF_INET socket to match platform.

truememory/hooks/core.py (ps/pgrep guards)

  • import psutil added at module top (already a dep).
  • _pid_is_alive: replaces subprocess.run(["ps", ...]) with psutil.Process().status().
  • _count_active_ingest_processes: replaces subprocess.run(["pgrep", ...]) with psutil.process_iter(["cmdline","status"]).

truememory/ingest/cli.py:308

  • Same start_new_session / creationflags guard for cascade Popen.

truememory/ingest/hooks/session_start.py:~226

tests/test_windows_subprocess_portability.py (new — 22 tests)

  • Covers all six regions: Popen platform branches, AF_UNIX/AF_INET selection, port-file roundtrip, psutil process queries, and module import smoke tests.

tests/test_cli_help.py (lines 94-98, 137-152)

  • test_help_via_console_script_does_not_hang: replaced ["truememory-mcp", "--help"] with [sys.executable, "-m", "truememory.mcp_server", "--help"].
  • test_ingest_version_flag_exits_cleanly: replaced [shutil.which("truememory-ingest"), "--version"] with [sys.executable, "-m", "truememory.ingest.cli", "--version"].
  • Both now route through the signed python.exe binary, bypassing Windows Defender ASR rule 01443614 which blocks low-prevalence console-script .exe shims.

Merge ordering

Depends on:

Clean mechanical rebase is expected on merge of either; zero semantic conflicts with #344, #345, #346, #347, #348, #349, or #350.

Test plan

  • python -m pytest tests/test_windows_subprocess_portability.py — 22/22 pass on Windows
  • test_help_via_console_script_does_not_hang — passes via python -m path
  • test_ingest_version_flag_exits_cleanly — passes via python -m path
  • Import smoke: python -c "import truememory.model_server; import truememory.model_client; import truememory.hooks.core" — clean on Windows and POSIX
  • PR-2a (Windows CI runner) validates this on merge

Generated with Claude Code

Replace POSIX-only subprocess patterns with cross-platform equivalents:

mcp_server.py (~line 1181):
- Guard start_new_session with hasattr(os, setsid); Windows branch uses
  CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS via creationflags.
- Wrap _log_file open/Popen in try/finally so handle closes on Popen exception.

model_server.py (whole file):
- Replace hardcoded AF_UNIX socket with platform-conditional: AF_UNIX on POSIX,
  AF_INET TCP on Windows. Windows path binds 127.0.0.1:0 (OS-assigned port)
  and writes the port to ~/.truememory/model_server.port for client discovery.
- Guard SIGHUP registration with hasattr(signal, SIGHUP) — absent on Windows.
- Idle-checker dummy wakeup uses AF_UNIX or AF_INET matching server bind.

model_client.py (whole file):
- Mirror server socket logic: _get_server_address() returns SOCK_PATH string on
  POSIX and (127.0.0.1, port) on Windows via PORT_PATH sidecar file.
- _start_server Popen: same start_new_session / creationflags guard.
- _send_request: constructs AF_UNIX or AF_INET socket depending on platform.

hooks/core.py (ps/pgrep guards):
- Add import psutil at module top.
- _pid_is_alive: replace subprocess.run([ps, ...]) with psutil.Process().
- _count_active_ingest_processes: replace subprocess.run([pgrep, ...]) with
  psutil.process_iter([cmdline,status]).

ingest/cli.py:308:
- Same start_new_session / creationflags guard for cascade Popen.

ingest/hooks/session_start.py:~226:
- Same start_new_session / creationflags guard for drain Popen + try/finally
  for _log_file. Did NOT touch _HAS_FCNTL block (agent-A buildingjoshbetter#345 territory).

tests/test_windows_subprocess_portability.py (new):
- 22 tests covering all six modified regions.

tests/test_cli_help.py (lines 94-98, 137-152):
- Swap bare console-script shim invocations for [sys.executable, -m, ...]
  to bypass Windows Defender ASR rule 01443614 on low-prevalence hosts.

Co-Authored-By: claude-sonnet-4-6 <wontreply@getfucked.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant