Skip to content

fix(mcp): ti_mcp, wr_mcp and wrx_mcp keep Fortran output off the JSON-RPC pipe (#262) - #263

Merged
k-yoshimi merged 1 commit into
developfrom
fix/mcp-fd-isolation-ti-wr-wrx-262
Oct 17, 2026
Merged

k-yoshimi merged 1 commit into
developfrom
fix/mcp-fd-isolation-ti-wr-wrx-262

Conversation

@k-yoshimi

Copy link
Copy Markdown
Owner

Closes nothing automatically (develop-targeted); fixes #262.

What

ti_mcp, wr_mcp and wrx_mcp get the fd isolation that #257 gave eq_mcp, tr_mcp and tot_mcp, with the same wiring:

  • import _fd_isolation after the wrapper import (the module sits in python/, which every server already puts on sys.path).
  • _fd_isolation.install() in main() immediately before build_server(), after the --help / --print-tools / missing-SDK returns. Nothing at import time (PR #226 follow-up: fix MCP stdio isolation, TR lifecycle, and EQ save error reporting #227 item 1).
  • A docstring section per server, same text as the other three.
  • tests/test_fd_isolation.py per server running the shared checks in python/mcp-servers/fd_isolation_cases.py (import leaves fd 1 and sys.stdout alone; main() installs before the server is built; --help / --print-tools / missing SDK install nothing; 2>&1 layout; closed fd 2).
  • python/tests/test_fd_isolation.py lists all six servers, so a shared case a server's file forgets to call fails there.
  • CHANGELOG entry.

fp_mcp is left to #232, which its author is rebasing onto the shared module.

Why

The three servers have the same layout as the other three and the same exposure: the library's WRITE(6,...) goes to fd 1, which for a stdio server is the JSON-RPC pipe to the client. One diagnostic line and the client logs "Failed to parse JSONRPC message from server"; in the worst case the session drops. See #227 and #257 for the measurements on tr/eq/tot.

Verification

Local, macOS, --forked --timeout=120 --timeout-method=signal:

run result
ti/wr/wrx tests/test_fd_isolation.py + python/tests/test_fd_isolation.py 41 passed
ti/wr/wrx tests/ (incl. test_server.py) + eq/tr/tot fd tests 156 passed, 3 skipped (pre-existing skips)

The tests need no .so: the shared cases stub build_server, so they run where wr cannot be built (macOS).

Review

In-house + Codex reviews on the diff before push (pre-push gate).

🤖 Generated with Claude Code

https://claude.ai/code/session_01TA2KahTs3qLMZL6yjSsWwT

…-RPC pipe (#262)

The three servers had the layout of eq_mcp / tr_mcp / tot_mcp (#257) and
the same exposure: the library's WRITE(6,...) went to fd 1, the pipe to
the MCP client. Each main() now calls _fd_isolation.install() (the module
#257 added, python/_fd_isolation.py) immediately before the stdio server
is built; importing a server module leaves fd 1 and sys.stdout of the
importing process alone, and --help / --print-tools / the missing-SDK path
return before anything is redirected.

Each server gets tests/test_fd_isolation.py running the shared checks of
python/mcp-servers/fd_isolation_cases.py, and the meta-test in
python/tests/test_fd_isolation.py now lists all six servers, so a case a
server's file forgets to call fails there.

fp_mcp is left to #232, which the author is rebasing onto the shared module.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TA2KahTs3qLMZL6yjSsWwT
@k-yoshimi
k-yoshimi merged commit 94515a3 into develop Oct 17, 2026
3 checks passed
HengyuLi-Ozaki-lab pushed a commit to HengyuLi-Ozaki-lab/task that referenced this pull request Oct 18, 2026
…isolation

The logging FAQ of tr / tot / ti / wr / wrx_mcp (and a new Logs section
of eq_mcp's README) now say that the library's WRITE(6,...) lands on
stderr because main() redirects fd 1 (k-yoshimi#257 / k-yoshimi#263), that a merged
stdout/stderr (a terminal, 2>&1) discards it with one warning line, and
how to keep it (2>server.log). Suggested by the docs review of k-yoshimi#263.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TA2KahTs3qLMZL6yjSsWwT
HengyuLi-Ozaki-lab added a commit to HengyuLi-Ozaki-lab/task that referenced this pull request Oct 18, 2026
develop gained k-yoshimi#256, k-yoshimi#257, k-yoshimi#258, k-yoshimi#259, k-yoshimi#261, k-yoshimi#263 and k-yoshimi#264 since the
last merge (f41fe5b).

Two conflicts:
- CHANGELOG.md: this branch's entry and develop's new entries were
  added at the same place under "### Added"; both kept.
- fp/fp_api.f90, the USE lists of the module: this branch reads NTG1,
  TVOLR and the six P*T arrays from fpcomm, k-yoshimi#259 imports
  fp_param_generation; both kept.

And one change that no line conflict shows. k-yoshimi#259 replaced the clear of
g_prepared in fp_api_set_param by a count of registry sets
(fp_param_generation, compared by fp_run with g_prep_generation). The
global scalars of fp_get_state were gated on g_prepared alone, so after
the merge a set no longer zeroed them, against what this branch
documents and tests ("zero ... and again after a set_param until the
next run that is made"). The gate is now "prepared, and no set since
the last prep" (g_prepared .AND. g_prep_generation ==
fp_param_generation()), which also covers sets through tot's "fp:"
dispatcher.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HengyuLi-Ozaki-lab added a commit to HengyuLi-Ozaki-lab/task that referenced this pull request Oct 18, 2026
develop gained k-yoshimi#242, k-yoshimi#243, k-yoshimi#248, k-yoshimi#249, k-yoshimi#251, k-yoshimi#254, k-yoshimi#255, k-yoshimi#256, k-yoshimi#257,
k-yoshimi#258, k-yoshimi#259, k-yoshimi#261, k-yoshimi#263 and k-yoshimi#264 since this branch left it.

One conflict, in python/eqlib/_ffi.py, python/fplib/_ffi.py and
python/trlib/_ffi.py alike: the end of load_library. k-yoshimi#261 (k-yoshimi#246) checks
the state layout of the library just loaded and raises
<Mod>libAbiError on a mismatch; this branch closes the directory it
registered with os.add_dll_directory when the load fails. The check is
now inside the block that cleans up, so a library refused for its
layout does not leave its directory in the process-wide DLL search set
either (or an entry in _DLL_DIR_HANDLES). Order as on develop: the
layout check, then the prototypes.

With the check on the load path, the cases of test_platform_libname.py
that fake the loaded library (ctypes.CDLL patched) were refused by it:
24 of 39 failed. Each of the three files now has an autouse fixture that
patches check_state_layout out for them (the check has its own cases in
test_ffi.py, TestLibraryLayout), and one new case per module puts a
check back that raises AbiMismatchError (with the prototypes passing,
so that only the check can refuse the load) and asserts
<Mod>libAbiError, the directory handle closed and nothing registered;
with the check moved after the cleanup block, that case fails (measured
on eq).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HengyuLi-Ozaki-lab added a commit to HengyuLi-Ozaki-lab/task that referenced this pull request Oct 18, 2026
develop gained, among others, k-yoshimi#257 (python/_fd_isolation.py, the fd
isolation shared by the stdio servers) and k-yoshimi#263 (ti_mcp, wr_mcp and
wrx_mcp adopt it). No conflict. The next commit moves fp_mcp onto the
shared module.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HengyuLi-Ozaki-lab added a commit to HengyuLi-Ozaki-lab/task that referenced this pull request Oct 18, 2026
As said on k-yoshimi#257: with the shared module in develop, fp_mcp drops its
own copy of the isolation and does what the other six servers do since
k-yoshimi#257 and k-yoshimi#263. main() calls _fd_isolation.install() immediately before
the stdio server is built (after the --help / --print-tools /
missing-SDK returns); the module is imported after the library wrapper,
whose directory it shares. fp_mcp/server.py now differs from develop by
that import, that call and the docstring section, which keeps the FP
measurement (FPWRTPRF writes the radial profile table at preparation
and every NTG2STEP steps, every step by default).
So fp_mcp also gets what the module handles beyond the plain redirect:
fd 2 closed, stdout and stderr the same file, one flag per process.

Tests: fp_mcp/tests/test_fd_isolation.py runs the shared checks
(python/mcp-servers/fd_isolation_cases.py), and
python/tests/test_fd_isolation.py lists fp_mcp among the servers whose
test file must call every shared case. fp_mcp/tests/test_import_isolation.py
is removed: its five cases (import leaves fd 1 and sys.stdout alone,
main() installs before the server starts, installing twice, --help and
--print-tools, the missing SDK) are among the shared ones or the
module's own. README Q8 says where unit 6 output goes, as k-yoshimi#264 did for
the other servers; a CHANGELOG entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HengyuLi-Ozaki-lab added a commit to HengyuLi-Ozaki-lab/task that referenced this pull request Oct 21, 2026
develop now carries the pieces split out of this branch (k-yoshimi#256 eq_save,
k-yoshimi#257 the MCP fd isolation, k-yoshimi#265 the mcp pin) and k-yoshimi#232, k-yoshimi#233, k-yoshimi#240, k-yoshimi#242,
k-yoshimi#243, k-yoshimi#248, k-yoshimi#249, k-yoshimi#251, k-yoshimi#254, k-yoshimi#255, k-yoshimi#258, k-yoshimi#259, k-yoshimi#261, k-yoshimi#263, k-yoshimi#264. 43
files conflicted; each was resolved as follows.

- Taken from develop (36 files): every file where this branch only
  changed the mcp pin to "mcp>=0.9,<2" (develop has ">=1.2,<2", k-yoshimi#265):
  the seven pyproject.toml, six READMEs, the fp, ti, tot, wr and wrx
  servers, the Sphinx MCP pages, the trlib presentation and its
  builder, the 2026-04-18 MCP plan; and the eq_save pieces now in
  develop in their reviewed form: eq/eq_api.f90, eq/eq_api.h,
  python/eqlib/tests/test_save.py, eq_mcp/tests/test_save_tool.py; and
  tr_mcp/server.py, whose changes here were this branch's own fd
  isolation (develop's _fd_isolation replaces it) and the os._exit(0)
  at the end of main().
- The os._exit(0) that ended main() of tr_mcp and eq_mcp on this branch
  (to skip the Python teardown, cited in k-yoshimi#227 item 5) is gone with
  them: develop's main() returns 0, as approved in k-yoshimi#257. The abort it
  avoided was a double DEALLOCATE, which this branch fixed in
  tr/trcomm.f90 (DEALLOCATE_TRCOMM returns when nothing is allocated);
  that fix stays, merged without a conflict.
- eq/eqfile.f90: develop's (EQSAVE without an argument, EQSAVE_IERR
  doing the work, and its kept EQDSKR error), plus this branch's
  MODELG=25 in EQ_READ's EQDSK branch (k-yoshimi#228-2). eq/eqmenu.f90 merged
  without a conflict but kept this branch's "CALL EQSAVE(IERR)"; it is
  develop's "CALL EQSAVE" again, as EQSAVE takes no argument.
- python/eqlib/_ffi.py, eqlib.py, tests/test_eqlib.py: develop's, plus
  this branch's get_psi_rz (the eq_common_get_psi_rz_ prototype, the
  Eq.get_psi_rz method, its closed-handle assertion).
- eq_mcp/server.py: develop's (the save tool of k-yoshimi#256, the shared fd
  isolation), plus this branch's get_psi_rz handler and tool and its
  --print-tools entry (13 tools). eq_mcp/tests/test_server.py: this
  branch's (13 tools with save and get_psi_rz), which contains
  develop's change.
- python/mcp-servers/eq_mcp/tests/test_import_isolation.py (this branch
  only, no conflict) is removed: it pinned this branch's own isolation
  helper (_install_fd_isolation, _FD_ISOLATION_INSTALLED in eq_mcp and
  tr_mcp), which develop's python/_fd_isolation.py replaces; develop's
  tests/test_fd_isolation.py of each of the seven servers checks the
  same contract (an import leaves fd 1 and sys.stdout alone).
- .github/workflows/python-tests.yml: develop's mcp pin; this branch's
  numpy install (the get_psi_rz tests import numpy) and ../task symlink
  step kept.
- CHANGELOG.md did not conflict. The three k-yoshimi#228 entries this branch held
  back in d8b6e4d are added to [Unreleased] Fixed, after its first
  entry, where no other open pull request inserts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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