Skip to content

Refactor systemDetectionConfig for platform commands #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TanNhatCMS wants to merge 2 commits into from
Closed

Refactor systemDetectionConfig for platform commands #2

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cfd64e3
Refactor systemDetectionConfig for platform commands
TanNhatCMS Dec 19, 2025
56d5f19
Enhance JSON handling with error throwing
TanNhatCMS Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions src/CopilotCli.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

namespace Revolution\Laravel\Boost;

use Illuminate\Contracts\Filesystem\FileNotFoundException;
use Illuminate\Support\Facades\File;
use Illuminate\Support\Str;
use JsonException;
use Laravel\Boost\Contracts\Agent;
use Laravel\Boost\Contracts\McpClient;
use Laravel\Boost\Install\CodeEnvironment\CodeEnvironment;
Expand All @@ -30,9 +32,14 @@ public function displayName(): string
*/
public function systemDetectionConfig(Platform $platform): array
{
return [
'command' => 'command -v copilot',
];
return match ($platform) {
Platform::Darwin, Platform::Linux => [
'command' => 'command -v copilot',
],
Platform::Windows => [
'command' => 'where copilot 2>nul',
],
Comment on lines +39 to +41
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The addition of Windows platform detection conflicts with the project's documented constraint that native Windows is not supported (only WSL). According to the README and project guidelines, this package explicitly does not support native Windows. The Windows-specific command 'where copilot 2>nul' should not be added, as it would incorrectly suggest Windows compatibility. Users on Windows should use WSL, which would be detected as Platform::Linux.

Copilot generated this review using guidance from repository custom instructions.
};
Comment on lines +35 to +42
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The new platform-specific detection logic lacks test coverage. Tests should be added to verify that Platform::Linux returns 'command -v copilot', and the Windows case should be removed or tested appropriately based on the resolution of the Windows support issue.

Copilot uses AI. Check for mistakes.
}

/**
Expand All @@ -43,7 +50,8 @@ public function systemDetectionConfig(Platform $platform): array
public function projectDetectionConfig(): array
{
return [
'files' => ['.github/copilot-instructions.md'],
'paths' => ['.github/instructions'],
'files' => ['.github/copilot-instructions.md', '.github/instructions/laravel-boost.instructions.md', 'AGENTS.md', 'CLAUDE.md', 'GEMINI.md'],
Comment on lines +53 to +54
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The newly added detection files ('AGENTS.md', 'CLAUDE.md', 'GEMINI.md') and the '.github/instructions' path are not documented in the README or other documentation. Users should be informed about what these files are for and how they relate to the project detection mechanism, particularly how they differ from the standard '.github/copilot-instructions.md' file.

Copilot uses AI. Check for mistakes.
];
Comment on lines 52 to 55
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The new 'paths' key and additional files in the detection configuration lack test coverage. Tests should verify that the projectDetectionConfig includes the '.github/instructions' path and all the newly added files ('.github/instructions/laravel-boost.instructions.md', 'AGENTS.md', 'CLAUDE.md', 'GEMINI.md').

Copilot uses AI. Check for mistakes.
}

Expand Down Expand Up @@ -91,6 +99,9 @@ public function convertCommandToPhpPath(string $command): string
*
* @param array<int, string> $args
* @param array<string, string> $env
*
* @throws FileNotFoundException
* @throws JsonException
*/
protected function installFileMcp(string $key, string $command, array $args = [], array $env = []): bool
{
Expand All @@ -104,7 +115,7 @@ protected function installFileMcp(string $key, string $command, array $args = []
$config = [];
if (File::exists($path)) {
$existingContent = File::get($path);
$config = json_decode($existingContent, true) ?? [];
$config = json_decode($existingContent, true, 512, JSON_THROW_ON_ERROR) ?? [];
}

$phpPath = $this->convertCommandToPhpPath($command);
Expand All @@ -128,7 +139,7 @@ protected function installFileMcp(string $key, string $command, array $args = []

// Remove empty arrays from existing config to avoid compatibility issues
$config = $this->removeEmptyArrays($config);
$json = json_encode($config, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES);
$json = json_encode($config, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES);
if ($json) {
$json = str_replace("\r\n", "\n", $json);

Expand Down