Skip to content

Psse auto detect and stdio fix - #124

Open
sbblack-qualus wants to merge 2 commits into
Power-Agent:mainfrom
sbblack-qualus:psse-auto-detect-and-stdio-fix
Open

sbblack-qualus wants to merge 2 commits into
Power-Agent:mainfrom
sbblack-qualus:psse-auto-detect-and-stdio-fix

Conversation

@sbblack-qualus

Copy link
Copy Markdown

Add functionality to auto discover the highest version of psse on the machine that has an appropriate psspy package matching the python version from which the MCP server was run. I would suggest doing some homework and selecting an MCP run command setup in your AI harness that runs the MCP server purposely in a version of python that matches a version of psse you have on your machine (PSSE >= 35).

There was also some strange behavior where the injection of report output from api calls like psspy.diff to stdout interfered with FastMCP and crashed the MCP server. I have added wrappers to attempt to keep PSSE output away from STDIO. It seems to be more stable now. I suspect that proper use of the redirect module might be a more eloquent solution, but this seems to work for now.

@qian-harvard qian-harvard 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 working on this. PSS/E auto-discovery and keeping PSS/E's report output off the MCP stream are both useful. Unfortunately the branch can't be merged as it stands. In the merge commit (2c9e052), the conflict in PSSE/psse_mcp.py was resolved by keeping your fork's copy of the file whole. That copy dates from March, so the PR reverts everything that has landed in the PSS/E server since then:

  1. The server no longer starts with the pinned SDK. It imports from mcp.server.fastmcp import FastMCP, but pyproject.toml pins mcp>=2,<3, where that module doesn't exist. Main uses from mcp.server.mcpserver import MCPServer as FastMCP. With main merged in, pytest aborts at collection: tests/test_psse_server.py and tests/test_sandbox.py raise ModuleNotFoundError.
  2. The security hardening from #57 is gone. That covers:
    • the _PROHIBITED_PSSPY_COMMANDS blocklist, so launch_program, runrspnsfile, addmodellibrary and others become callable through run_psspy_command;
    • the checked_path sandbox on case and file arguments;
    • the identifier check in lookup_psspy_command, so lookup_psspy_command("../../pyproject") reads files outside the spec folder again.
  3. PSS/E initialises at import time. Discovery and psseinit run on import and raise on any machine without PSS/E. Main made this lazy (_ensure_psse), and it reads the powermcp config paths (psse.python_lib / psse.bin), which this version drops.
  4. The open_case fixes from #94 are reverted. The ierr from psspy.case is no longer checked, so a failed load reports success with the previous case's counts.
  5. The list_dynamic_output_channels and read_dynamic_output tools from #97/#98 are deleted.

On the two features themselves:

  • Auto-detect.
    • The candidate majors are hard-coded to [36, 35], so PSS/E 37 is never found.
    • PSSE_VERSION=36.2 raises ValueError.
    • The manual override is silently ignored unless both env vars are set.
    • The override defaults to psse36, so a PSS/E 35 user gets No module named 'psse36'.
  • stdout. mcp 2.x already redirects fd 1 to stderr when the stdio transport starts. A tool that print()s or os.write(1, ...)s doesn't corrupt the JSON-RPC stream on main. The crash you saw looks like mcp 1.x behaviour. The part worth keeping is returning the captured text to the client (your _console_output), since otherwise psspy.diff output only reaches stderr.

Could you open a fresh branch from current main and port just the new behaviour onto main's version of the file?

  • Move discovery into _resolve_psse_paths() so it stays lazy. Check powermcp config first, and don't cap the major version.
  • Keep the _console_output capture inside run_psspy_command.
  • Add tests to tests/test_psse_server.py using its fake psspy. CI doesn't have PSS/E, so those tests are what will cover this.

This branch has not been deployed

No deployments
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.

2 participants