fix(tests,mcp): harden _run_cli helper + propagate exit codes under python -m#352
Open
Huntehhh wants to merge 1 commit into
Open
fix(tests,mcp): harden _run_cli helper + propagate exit codes under python -m#352Huntehhh wants to merge 1 commit into
Huntehhh wants to merge 1 commit into
Conversation
Two tiny additive fixes that close the test-infrastructure gap left open by PR-2b (buildingjoshbetter#351). Surfaced live by Hunter's test run hitting Defender ASR on his Block-mode Win11 box (Windows confirmed `truememory-ingest.exe` blocked at 7:44 PM). tests/test_cli_help.py:_run_cli helper - Replaced the two-arm helper (shutil.which("truememory-mcp") → bare shim, fall back to python -m) with a single `[sys.executable, "-m", "truememory.mcp_server"]` invocation. The shim path is faster (cached imports) but is a setuptools / uv trampoline with a per- install unique hash — ASR rule 01443614 silently kills it at launch on hardened Win11 baselines. Routing through the signed python.exe wrapper passes ASR everywhere; the per-test re-import cost is negligible at test scale. truememory/mcp_server.py:1521 (__main__ block) - Changed `main()` to `sys.exit(main() or 0)`. The setuptools console- script wrapper around `truememory-mcp` already does `sys.exit(main())`, so exit codes propagated correctly under the shim. Under `python -m` the bare `main()` discarded the return value — exit-code-2 paths (unknown flag, positional arg typo) silently masked as exit 0. Bundled here because the _run_cli rewrite above exposed the bug: 2 of the 7 newly -m-routed tests assert `returncode != 0`. Test results on Windows: - 7 of 9 tests in test_cli_help.py pass after this PR (up from 0 of 9 on Block-mode ASR boxes pre-fix) - Remaining 2 failures (test_help_via_console_script_does_not_hang, test_ingest_version_flag_exits_cleanly) invoke the shim directly outside the helper — covered by PR-2b (buildingjoshbetter#351) lines 94-98 + 137-152. Coordination context (multi-agent sweep): - Targets origin/main directly per agent-B's relay (separate region from buildingjoshbetter#351's lines 94-98 + 137-152 — mechanical merge order). - mcp_server.py:1521 is unclaimed by any other agent's file ownership region. Co-Authored-By: claude-opus-4-7 <wontreply@getfucked.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two tiny additive fixes that close the test-infrastructure gap left open by #351 (PR-2b Windows subprocess portability). Surfaced live by a test run on a hardened-Win11 Block-mode ASR host hitting Defender on
truememory-ingest.exe.Branched off
origin/maindirectly. Different region from #351's edits in the same file — mechanical merge regardless of order.What changed
tests/test_cli_help.py:_run_clihelperReplaced the two-arm helper (
shutil.which("truememory-mcp")→ bare shim, fall back topython -m) with a single[sys.executable, "-m", "truememory.mcp_server"]invocation. The shim path is faster (cached imports) but is a setuptools / uv trampoline with a per-install unique hash — Microsoft Defender's Attack-Surface-Reduction rule01443614-cd74-433a-b99e-2ecdc07bfc25("Block executable files from running unless they meet a prevalence, age, or trusted list criteria") silently kills it at launch on hardened Win11 baselines. Routing through the signedpython.exewrapper passes ASR everywhere; the per-test re-import cost is negligible at test scale.truememory/mcp_server.py:1521(__main__block)Changed
main()tosys.exit(main() or 0). The setuptools console-script wrapper aroundtruememory-mcpalready doessys.exit(main()), so exit codes propagated correctly under the shim. Underpython -mthe baremain()discarded the return value — exit-code-2 paths (unknown flag, positional-arg typo) silently masked as exit 0.Bundled here because the
_run_clirewrite above exposed the bug: 2 of the 7 newly-m-routed tests assertreturncode != 0(test_unknown_flag_exits_nonzero_not_hang,test_positional_arg_exits_nonzero_not_hang). Without thesys.exitline, those would silently fail on every CI run that exercisedpython -m. Single-concern in spirit — "makepython -m truememory.mcp_serverbehave like the shim under all invocation paths" — even though it touches two files.Test results (Windows, Defender ASR Block mode)
Before this PR:
After this PR:
The 2 remaining are PR-2b (#351) territory — they invoke the shim directly outside the helper at lines 94-98 + 137-152. Will pass once #351 lands (or when this branch rebases on it).
Test plan
pytest tests/test_cli_help.py -von Linux / macOS — all 9 pass (no ASR there)pytest tests/test_cli_help.py -von Windows Block-mode ASR — 7 of 9 pass (until fix(mcp,model_server,hooks,ingest): Windows subprocess portability #351 merges, then 9 of 9)python -m truememory.mcp_server --halp— exits 2 with usage text on stderr (previously exited 0)python -m truememory.mcp_server help(positional, no dashes) — exits 2 (previously exited 0)truememory-mcp --halpvia the shim — still exits 2 (unchanged behaviour)Coordination context (multi-agent sweep)
PR-2c in the sweep registered after agent-B's PR-2b (#351) explicitly handed off the
_run_cli-helper hardening (agent-B relay 2026-05-17 ~02:20 ET — "test-infra single-concern, doesn't belong on top of subprocess story").Sibling sweep PRs:
_setup_claudehygiene) · #349 (logging + Popen cleanup) · #350 (docs Windows callouts) · this PRmcp_server.py:1521is unclaimed by any other agent's file ownership region per the multi-agent coordination registry.Merge ordering
Order-independent against #351. Different region of
tests/test_cli_help.pyfrom #351 (this PR rewrites the_run_clihelper at lines 19-36; #351 fixes the two shim-direct tests at lines 94-98 + 137-152). Mechanical inter-PR merge regardless of order.Depends on: none.
mcp_server.py:1521(sys.exit(main() or 0)line in the__main__block) is unclaimed by any other agent's file ownership region.Blocks: none.
Recommended sequence position: after #344 / #345 / #351 so the test-helper fixes are validated against the full subprocess-portability stack on a Windows runner (#353).