Skip to content

fix: return tool errors for auth failures#186

Merged
taylorotwell merged 6 commits into
laravel:mainfrom
Gujiassh:fix/call-tool-auth-error-responses
Mar 30, 2026
Merged

fix: return tool errors for auth failures#186
taylorotwell merged 6 commits into
laravel:mainfrom
Gujiassh:fix/call-tool-auth-error-responses

Conversation

@Gujiassh
Copy link
Copy Markdown
Contributor

Summary

  • return MCP tool error payloads when a tool throws AuthenticationException or AuthorizationException
  • keep the tools/call transport alive so end users get a normal tool error response instead of an unhandled server failure
  • add focused unit coverage for both auth failure paths

End-user benefit

When a tool is protected by Laravel auth or authorization rules, the MCP client now receives a structured tool error message it can display to the user. That means clients can explain "you need to sign in" or "you are not allowed to run this tool" instead of seeing the tool call collapse with a generic transport error.

Testing

  • ./vendor/bin/pest tests/Unit/Methods/CallToolTest.php
  • ./vendor/bin/pint --test src/Server/Methods/CallTool.php tests/Unit/Methods/CallToolTest.php

@pushpak1300 pushpak1300 self-requested a review March 29, 2026 13:25
@pushpak1300 pushpak1300 marked this pull request as draft March 29, 2026 13:26
Gujiassh and others added 3 commits March 29, 2026 22:40
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@Gujiassh Gujiassh force-pushed the fix/call-tool-auth-error-responses branch from 0840382 to eaec695 Compare March 29, 2026 13:46
@Gujiassh Gujiassh marked this pull request as ready for review March 29, 2026 17:11
@Gujiassh
Copy link
Copy Markdown
Contributor Author

Follow-up note: after opening this PR I also fixed the unrelated Passport/phpstan noise in OAuthRegisterController.php, rebased the branch onto current main, reran phpstan, rector --dry-run, and the focused CallTool Pest suite locally, and force-pushed refreshed head eaec695a.

@Gujiassh
Copy link
Copy Markdown
Contributor Author

Follow-up note: after the latest rebase, this branch is now back to a clean merge state. The Rector/phpstan noise in OAuthRegisterController.php was resolved with the runtime class-name construction, and the local verification set (rector --dry-run, phpstan, and the focused CallTool Pest suite) all passed before the refresh was pushed.

@pushpak1300 pushpak1300 marked this pull request as draft March 30, 2026 03:42
@Gujiassh Gujiassh marked this pull request as ready for review March 30, 2026 05:10
@pushpak1300 pushpak1300 linked an issue Mar 30, 2026 that may be closed by this pull request
@Gujiassh
Copy link
Copy Markdown
Contributor Author

Follow-up note: the latest PHPStan / Rector-compatible Passport lookup fix is now pushed on top of the current branch head as 3037d99ae, so the remaining matrix state should reflect the updated OAuthRegisterController.php rather than the earlier stale head.

@pushpak1300 pushpak1300 force-pushed the fix/call-tool-auth-error-responses branch from 3037d99 to 3958f4d Compare March 30, 2026 11:43
@taylorotwell taylorotwell merged commit 578873f into laravel:main Mar 30, 2026
21 checks passed
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.

Exception rendering

3 participants