feat: add MCP context local tool adapter#200
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new local tool adapter, scripts/mcp_context_tool.py, which provides a one-shot JSON interface for the deterministic MCP context layer. The changes include documentation for the new adapter, the implementation of the script itself, and comprehensive tests to ensure deterministic behavior and proper error handling. Feedback focuses on improving error diagnostics by including repo-relative paths in exception messages and enforcing stricter type validation for the payload parameter in the request processing logic.
| def _load_request(path: Path | None) -> dict[str, Any]: | ||
| try: | ||
| raw = path.read_text(encoding="utf-8") if path is not None else sys.stdin.read() | ||
| except FileNotFoundError as exc: | ||
| raise RuntimeError(f"missing request file: {path.as_posix() if path is not None else '<stdin>'}") from exc | ||
| try: | ||
| request = json.loads(raw) | ||
| except json.JSONDecodeError as exc: | ||
| raise RuntimeError(f"invalid JSON request: {exc}") from exc | ||
| if not isinstance(request, dict): | ||
| raise RuntimeError("request must be a JSON object") | ||
| return request |
There was a problem hiding this comment.
The error handling for loading the request JSON should include the repo-relative path in the RuntimeError message for both FileNotFoundError and JSONDecodeError, as per the general rules. Additionally, the type check for the request object should also include the path for better diagnostics.
| def _load_request(path: Path | None) -> dict[str, Any]: | |
| try: | |
| raw = path.read_text(encoding="utf-8") if path is not None else sys.stdin.read() | |
| except FileNotFoundError as exc: | |
| raise RuntimeError(f"missing request file: {path.as_posix() if path is not None else '<stdin>'}") from exc | |
| try: | |
| request = json.loads(raw) | |
| except json.JSONDecodeError as exc: | |
| raise RuntimeError(f"invalid JSON request: {exc}") from exc | |
| if not isinstance(request, dict): | |
| raise RuntimeError("request must be a JSON object") | |
| return request | |
| def _load_request(path: Path | None) -> dict[str, Any]: | |
| if path is not None: | |
| try: | |
| display_path = path.resolve().relative_to(REPO_ROOT).as_posix() | |
| except ValueError: | |
| display_path = path.as_posix() | |
| else: | |
| display_path = "<stdin>" | |
| try: | |
| raw = path.read_text(encoding="utf-8") if path is not None else sys.stdin.read() | |
| except FileNotFoundError as exc: | |
| raise RuntimeError(f"missing request file: {display_path}") from exc | |
| try: | |
| request = json.loads(raw) | |
| except json.JSONDecodeError as exc: | |
| raise RuntimeError(f"invalid JSON request: {display_path}: {exc}") from exc | |
| if not isinstance(request, dict): | |
| raise RuntimeError(f"request must be a JSON object: {display_path}") | |
| return request |
References
- When loading JSON files, handle FileNotFoundError and JSONDecodeError by raising a RuntimeError that includes a repo-relative path. Additionally, validate that the decoded payload is of the expected type (e.g., a dictionary) before proceeding.
| def _payload_from_params(params: dict[str, Any]) -> dict[str, Any]: | ||
| payload = params.get("payload") | ||
| if isinstance(payload, dict): | ||
| return payload | ||
|
|
||
| fixture = params.get("fixture") | ||
| if not isinstance(fixture, str) or not fixture: | ||
| raise RuntimeError("request.params requires payload object or fixture path") | ||
| return build_replay_payload(load_fixture_context(Path(fixture))) |
There was a problem hiding this comment.
To maintain strictness as per the general rules, we should explicitly validate that payload is a dictionary if it is provided, rather than silently falling back to checking for a fixture.
| def _payload_from_params(params: dict[str, Any]) -> dict[str, Any]: | |
| payload = params.get("payload") | |
| if isinstance(payload, dict): | |
| return payload | |
| fixture = params.get("fixture") | |
| if not isinstance(fixture, str) or not fixture: | |
| raise RuntimeError("request.params requires payload object or fixture path") | |
| return build_replay_payload(load_fixture_context(Path(fixture))) | |
| def _payload_from_params(params: dict[str, Any]) -> dict[str, Any]: | |
| payload = params.get("payload") | |
| if payload is not None: | |
| if not isinstance(payload, dict): | |
| raise RuntimeError("request.params.payload must be a JSON object") | |
| return payload | |
| fixture = params.get("fixture") | |
| if not isinstance(fixture, str) or not fixture: | |
| raise RuntimeError("request.params requires payload object or fixture path") | |
| return build_replay_payload(load_fixture_context(Path(fixture))) |
References
- When processing JSON data, treat null or missing values for expected list fields as empty lists, but raise a RuntimeError for other non-list types to maintain strictness.
Summary
Adds a minimal deterministic MCP-style local tool adapter for the existing MCP context layer.
This introduces:
scripts/mcp_context_tool.pybuild_replay_payloadrender_prompt_contextvalidate_replay_payloadScope
This is intentionally not:
Validation
python -m compileall -q src/comptext_v7/mcppytest tests/test_mcp_context_layer.py -qpython scripts/mcp_context_tool.pyNotes
The adapter is a local deterministic JSON interface on top of the MCP context primitives. It is intended as the bridge between CLI usage and a later MCP wrapper without expanding CompTextv7 into orchestration or tool execution.