Skip to content

Conversation

@digitall-it
Copy link
Contributor

Summary

  • Skip splitting in normalizeCommand() when the command is an absolute path (Unix / or Windows drive letter C:\), as these may contain spaces (e.g. macOS Application Support)
  • Preserves existing behavior for relative/compound commands like valet php or docker exec container php

Fixes #549
Related: #411, #539

Test Plan

  • Added test: absolute Unix path with spaces is preserved intact
  • Added test: absolute Unix path without spaces is preserved intact
  • Added test: absolute Windows path with spaces is preserved intact
  • Added test: file-based MCP installation writes correct config for paths with spaces
  • All 584 existing tests continue to pass

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings February 9, 2026 10:52
When agents use useAbsolutePathForMcp(), getPhpPath() returns PHP_BINARY
which on macOS with Herd resolves to a path inside "Application Support"
(containing a space). The normalizeCommand() method splits on spaces,
breaking the command into an invalid executable path.

This adds a check to skip splitting when the command is an absolute path
(Unix / or Windows drive letter), preserving paths with spaces while
maintaining existing behavior for relative/compound commands.

Fixes laravel#549

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@digitall-it digitall-it force-pushed the fix/normalize-command-spaces-in-path branch from 18c2fda to 753cc9e Compare February 9, 2026 10:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates Agent::normalizeCommand() to avoid splitting absolute executable paths that may contain spaces (e.g. Herd’s PHP binary on macOS), ensuring generated MCP configs keep the full command path intact.

Changes:

  • Add an absolute-path short-circuit to normalizeCommand() for Unix (/…) and Windows drive-letter (C:\…) paths.
  • Add unit tests covering absolute Unix/Windows paths (with/without spaces) and file-based MCP config output for spaced paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Install/Agents/Agent.php Adjusts command normalization to preserve absolute paths and avoid breaking MCP command fields when paths contain spaces.
tests/Unit/Install/Agents/AgentTest.php Adds regression tests ensuring absolute paths (incl. with spaces) aren’t split and file-based MCP output is correct.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pushpak1300 pushpak1300 merged commit e735881 into laravel:main Feb 9, 2026
15 checks passed
@pushpak1300
Copy link
Member

Thanks

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.

normalizeCommand() splits PHP binary path on spaces, breaking MCP config for Herd on macOS

2 participants