Skip to content

[MCP] Add EEGPrep agent server - #233

Open
suraj-ranganath wants to merge 1 commit into
developfrom
feature/eegprep-mcp-agent-server
Open

[MCP] Add EEGPrep agent server#233
suraj-ranganath wants to merge 1 commit into
developfrom
feature/eegprep-mcp-agent-server

Conversation

@suraj-ranganath

Copy link
Copy Markdown
Member

Add an optional eegprep-mcp entrypoint backed by reusable MCP tool helpers for dataset inspection, validation, CLI schema discovery, bundled agent guidance, and guarded JSON command execution. The server reuses the existing CLI contract, requires explicit write/overwrite approval for mutating commands, and documents the workflow for researchers using AI agents.

@claude

claude Bot commented Jun 20, 2026

Copy link
Copy Markdown

Claude finished @suraj-ranganath's task in 3m 30s —— View job


Code review

  • Overall assessment: Safe to merge. Well-structured, conservative safety model, good test coverage.
  • Highest-risk area: The write/overwrite gating logic in mcp/tools.py (_command_writes / WRITE_FLAGS) — this is the security boundary. I traced every writing command against it and it holds up.
  • Merge recommendation: Safe to merge.

Blocking

None.

Important

None.

Nits

  • server._call catches Exception, not BaseException (src/eegprep/mcp/server.py:167). tools.run_cli_command invokes cli_main, and an argparse code path that raises SystemExit would bypass the structured-error wrapper. In practice this is very unlikely — the allowlist requires an explicit subcommand, --json is enforced, and both the root and subcommand parsers override error() to raise EEGPrepCLIError — so I'm flagging this only as a low-priority robustness note, not a real defect.

Test gaps

  • No happy-path test that allow_write=True actually lets a write command through the gate. test_run_cli_command_requires_json_and_write_confirmation (tests/test_mcp_tools.py:106) confirms the gate rejects writes without approval, but nothing confirms that approval permits execution. A regression that made allow_write=True still raise WRITE_CONFIRMATION_REQUIRED would go uncaught. Concrete suggestion — add a test that runs a real write to tmp_path and asserts it executes and produces output:
    def test_run_cli_command_executes_write_with_approval(tmp_path):
        out = tmp_path / "resampled.set"
        payload = tools.run_cli_command(
            ["resample", str(SAMPLE_DATASET_PATH), "--freq", "64", "--output", str(out), "--json"],
            allow_write=True,
        )
        assert payload["status"] == "ok"
        assert payload["exit_code"] == 0
        assert out.exists()

EEGLAB parity notes

None — this PR is an agent/CLI integration layer with no EEG data-structure or numerical behavior. It correctly reuses the existing CLI contract (discovery, dataset, core) rather than introducing a parallel one, and the bundled skill/docs stay standalone (no MATLAB or vendored EEGLAB runtime dependency), consistent with AGENTS.md.


What I checked and liked

  • Write-safety model is conservative and correct. I verified each mutating path is gated: transform commands (resample/rereference/filter/clean/epoch/ica/report) are unconditionally in WRITE_COMMANDS; pipeline run and batch are gated even when output comes from YAML (no flag present) via the special cases in _command_writes; config-driven writers and migrate convert-script --output / qc report --html are caught through WRITE_FLAGS. --dry-run and pipeline plan/validate correctly classify as read-only.
  • ALLOWED_COMMANDS matches the real CLI subparsers (capabilities, schema, examples, skills, inspect, validate, etc. all exist in cli/main.py).
  • Tooling/contract reuse is good — no second agent contract; _call produces eegprep.error.v1-shaped errors; in-process execution avoids shelling out.
  • Bounded inspection (_bounded_records) preserves the original count while truncating, so the truncation signaling (returned/truncated) is accurate.
  • .notes/implementation-notes.html is intentionally whitelisted in .gitignore, so its inclusion is fine.
    · feature/eegprep-mcp-agent-server

@suraj-ranganath

Copy link
Copy Markdown
Member Author

🤖 Specification

Problem: EEGPrep already has a strong CLI for agents, but MCP clients had no first-class, version-matched way to inspect EEG datasets, discover EEGPrep command contracts, retrieve agent guidance, or run safe JSON commands without scraping terminal help or shelling out manually.

Approach: Added an optional eegprep[mcp] extra and eegprep-mcp entrypoint. The reusable tool layer in src/eegprep/mcp/tools.py exposes capabilities, the MCP agent guide, bounded sample-data inspection, validation, CLI schemas/examples, command planning, and guarded CLI execution. The FastMCP adapter in src/eegprep/mcp/server.py registers those as MCP tools, resources, and a preprocessing planning prompt while keeping the MCP SDK optional.

Key code: run_cli_command() builds an allowlisted command plan, requires --json, blocks file-writing commands until allow_write=True, blocks --overwrite until allow_overwrite=True, then calls eegprep.cli.main.main(arguments) in-process under captured stdout/stderr. No shell execution is used.

Tests: Added tests/test_mcp_tools.py covering capabilities, bundled MCP skill discovery, sample-data summary/events/channels/ICA inspection, dataset validation, command schema/example delegation, read/write/overwrite planning, guarded execution, non-EEGPrep command rejection, FastMCP registration, structured error conversion, and module help/version output. Also ran CLI adjacency tests and docs build.

Copy link
Copy Markdown
Member Author

🤖 Re-audited at e88f5c0c. Looks good. The MCP layer reuses the existing CLI and dataset contracts, keeps the SDK optional, avoids shell execution, and conservatively gates the current file-writing and overwrite paths. Documentation, lint, and the supported-platform test matrix are green.

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