feat(mcp): harden and modernize the MCP server layer#738
Merged
Conversation
- Make operation tools stateless per call: tools hold only the repository class string and resolve a fresh Repository in every handle(), fixing the root cause behind #735 (singleton-cached repository state) and removing the store-model band-aid. Collapse the 8 duplicate execute* dispatch methods in McpToolsManager. - Return real MCP errors: all failure paths now set isError=true via Response::error() with a stable {error, code, errors, detail} payload; actions/getters propagate per-field ValidationException detail like CRUD; raw exception messages only surface when app.debug is on; fix the 'getter' vs 'action' key in the action authorization error. - Add tool annotations (#[IsReadOnly]/#[IsDestructive]/#[IsIdempotent]/ #[IsOpenWorld]) to every operation, wrapper, and search tool, and expose the same hints per operation in the wrapper catalog payloads. - Harden the server: default mode flips to 'wrapper' (breaking, 10.x), the per-request ?mode= override is removed, new restify.mcp.read_only mode gates write operations in discovery and execution, and store file ingestion rejects local paths by default, blocks private-IP URLs, and caps download size. - Emit structuredContent alongside JSON text on tool results so hosts can parse responses deterministically. - New restify:skill artisan command generating SKILL.md + openapi.json from MCP-enabled repositories (auth, pagination, filter conventions, per-repository operations and curl examples). BREAKING CHANGE: restify.mcp.mode now defaults to 'wrapper'; set RESTIFY_MCP_MODE=direct to keep the previous per-operation tool exposure.
Deploying laravel-restify with
|
| Latest commit: |
b3e139e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c51f2e8c.laravel-restify.pages.dev |
| Branch Preview URL: | https://feat-mcp-improvements.laravel-restify.pages.dev |
…uard test /etc/hosts does not exist on Windows runners, so the allow_local_paths assertion received null instead of an UploadedFile. A test-created temp file also proves the default rejection against a genuinely readable file.
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
1 participant
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
Implements the improvements from a full research + adversarial-verification pass over the MCP layer (architecture review, security audit, MCP 2025-06/2025-11 spec ecosystem research). Six changes, each independently verified:
Repositoryperhandle()call. Fixes the root cause behind fix(mcp): store tool must create a fresh model on every call #735 (singleton-cached repository state) and removes the store-model band-aid. Also collapses the 8 duplicateexecute*dispatch methods inMcpToolsManagerand fixes a latent bug wherediscover-repositoriesalways reported the baseRepositorymodel.isError=trueviaResponse::error()with a stable{error, code, errors, detail}payload. Actions/getters propagate per-fieldValidationExceptiondetail like CRUD store/update already did. Raw exception messages/class names only surface underapp.debug. Fixes the'getter'vs'action'key in the action authorization error.#[IsReadOnly]/#[IsDestructive]/#[IsIdempotent]/#[IsOpenWorld]on every operation, wrapper, and search tool so hosts get the auto-approve-vs-confirm safety signal. The wrapper catalog (get-repository-operations/get-operation-details) carries the same hints per operation.wrapper(see breaking change), the per-request?mode=override is removed (clients could previously expand the exposed tool surface), newrestify.mcp.read_onlyconfig gates write operations centrally in both discovery and execution, and store file ingestion rejects local filesystem paths by default, blocks private/link-local/loopback IP URLs, and caps download size.structuredContent(MCP 2025-06-18) alongside the existing JSON text for deterministic parsing by hosts.restify:skillcommand — generatesSKILL.md(Agent Skills format: auth, pagination, thefilterName=valuefilter convention, per-repository operations with curl examples) +openapi.jsonfrom MCP-enabled repositories.Breaking change
restify.mcp.modenow defaults towrapper(4 progressive-disclosure tools) instead ofdirect(one tool per operation). Research shows tool-selection accuracy collapses with large tool counts, which direct mode produces on any non-trivial app. SetRESTIFY_MCP_MODE=directto restore the previous behavior.Test plan
New tests:
McpErrorResponseTest,McpServerHardeningTest,McpToolAnnotationsTest,SkillCommandTest, plus regression tests for consecutive stores, cross-repository isolation, structured content, and read-only/SSRF guards.Known follow-ups (low severity, out of scope)
composer analyseis broken repo-wide by a pre-existing invalid PHPStan config key (checkMissingIterableValueType), unrelated to this diff.