Skip to content

Commit dc2146e

Browse files
committed
feat: add duplicate protection for JSON 5 files
1 parent 1839e11 commit dc2146e

File tree

2 files changed

+179
-96
lines changed

2 files changed

+179
-96
lines changed

src/Install/Mcp/FileWriter.php

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,18 @@ protected function injectIntoExistingConfigKey(string $content, array $matches):
101101
return false;
102102
}
103103

104+
// Filter out servers that already exist
105+
$serversToAdd = $this->filterExistingServers($content, $openBracePos, $closeBracePos);
106+
107+
if (empty($serversToAdd)) {
108+
return true;
109+
}
110+
104111
// Detect indentation from surrounding content
105112
$indentLength = $this->detectIndentation($content, $closeBracePos);
106113

107114
$serverJsonParts = [];
108-
foreach ($this->serversToAdd as $key => $serverConfig) {
115+
foreach ($serversToAdd as $key => $serverConfig) {
109116
$serverJsonParts[] = $this->generateServerJson($key, $serverConfig, $indentLength);
110117
}
111118
$serversJson = implode(','."\n", $serverJsonParts);
@@ -130,6 +137,28 @@ protected function injectIntoExistingConfigKey(string $content, array $matches):
130137
return $this->writeFile($newContent);
131138
}
132139

140+
protected function filterExistingServers(string $content, int $openBracePos, int $closeBracePos): array
141+
{
142+
$configContent = substr($content, $openBracePos + 1, $closeBracePos - $openBracePos - 1);
143+
$filteredServers = [];
144+
145+
foreach ($this->serversToAdd as $key => $serverConfig) {
146+
if (! $this->serverExistsInContent($configContent, $key)) {
147+
$filteredServers[$key] = $serverConfig;
148+
}
149+
}
150+
151+
return $filteredServers;
152+
}
153+
154+
protected function serverExistsInContent(string $content, string $serverKey): bool
155+
{
156+
$quotedPattern = '/["\']'.preg_quote($serverKey, '/').'["\']\\s*:/';
157+
$unquotedPattern = '/(?<=^|\\s|,|{)'.preg_quote($serverKey, '/').'\\s*:/m';
158+
159+
return preg_match($quotedPattern, $content) || preg_match($unquotedPattern, $content);
160+
}
161+
133162
protected function injectNewConfigKey(string $content): bool
134163
{
135164
$openBracePos = strpos($content, '{');

tests/Unit/Install/Mcp/FileWriterTest.php

Lines changed: 149 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,60 @@
282282
expect($writtenContent)->toContain('// Ooo, pretty cool'); // Inline comments preserved
283283
});
284284

285+
test('injecting twice into existing JSON 5 doesn\'t cause duplicates', function () {
286+
$capturedContent = '';
287+
288+
File::clearResolvedInstances();
289+
File::partialMock();
290+
291+
File::shouldReceive('ensureDirectoryExists')->once();
292+
File::shouldReceive('exists')->andReturn(true);
293+
File::shouldReceive('size')->andReturn(1000);
294+
File::shouldReceive('get')->andReturn(fixture('mcp.json5'));
295+
File::shouldReceive('put')
296+
->with(
297+
Mockery::capture($capturedPath),
298+
Mockery::capture($capturedContent)
299+
)
300+
->andReturn(true);
301+
302+
$result = (new FileWriter('/path/to/mcp.json'))
303+
->configKey('servers') // mcp.json5 uses "servers" not "mcpServers"
304+
->addServer('boost', 'php', ['artisan', 'boost:mcp'])
305+
->save();
306+
307+
$boostCounts = substr_count($capturedContent, '"boost"');
308+
expect($result)->toBeTrue();
309+
expect($boostCounts)->toBe(1);
310+
expect($capturedContent)->toContain('"boost"'); // New server added
311+
expect($capturedContent)->toContain('mysql'); // Existing server preserved
312+
expect($capturedContent)->toContain('laravel-boost'); // Existing server preserved
313+
expect($capturedContent)->toContain('// Here are comments within my JSON'); // Comments preserved
314+
expect($capturedContent)->toContain('// Ooo, pretty cool'); // Inline comments preserved
315+
316+
$newContent = $capturedContent;
317+
318+
File::clearResolvedInstances();
319+
File::partialMock();
320+
321+
File::shouldReceive('ensureDirectoryExists')->once();
322+
File::shouldReceive('exists')->andReturn(true);
323+
File::shouldReceive('size')->andReturn(1000);
324+
File::shouldReceive('get')->andReturn($newContent);
325+
326+
$result = (new FileWriter('/path/to/mcp.json'))
327+
->configKey('servers')
328+
->addServer('boost', 'php', ['artisan', 'boost:mcp'])
329+
->save();
330+
331+
// Second call should return true but not modify the file since boost already exists
332+
expect($result)->toBeTrue();
333+
334+
// We should still have only one instance of the boost MCP server
335+
$boostCounts = substr_count($capturedContent, '"boost"');
336+
expect($boostCounts)->toBe(1);
337+
});
338+
285339
test('injects into empty configKey object', function () {
286340
$writtenContent = '';
287341

@@ -364,35 +418,35 @@ function newFileServerConfigurations(): array
364418
{
365419
return [
366420
'single server without args or env' => [
367-
'configKey' => 'servers',
368-
'servers' => [
421+
'servers',
422+
[
369423
'im-new-here' => ['command' => './start-mcp'],
370424
],
371-
'expectedJson' => '{"servers":{"im-new-here":{"command":"./start-mcp"}}}',
425+
'{"servers":{"im-new-here":{"command":"./start-mcp"}}}',
372426
],
373427
'single server with args' => [
374-
'configKey' => 'mcpServers',
375-
'servers' => [
428+
'mcpServers',
429+
[
376430
'boost' => [
377431
'command' => 'php',
378432
'args' => ['artisan', 'boost:mcp'],
379433
],
380434
],
381-
'expectedJson' => '{"mcpServers":{"boost":{"command":"php","args":["artisan","boost:mcp"]}}}',
435+
'{"mcpServers":{"boost":{"command":"php","args":["artisan","boost:mcp"]}}}',
382436
],
383437
'single server with env' => [
384-
'configKey' => 'servers',
385-
'servers' => [
438+
'servers',
439+
[
386440
'mysql' => [
387441
'command' => 'npx',
388442
'env' => ['DB_HOST' => 'localhost', 'DB_PORT' => '3306'],
389443
],
390444
],
391-
'expectedJson' => '{"servers":{"mysql":{"command":"npx","env":{"DB_HOST":"localhost","DB_PORT":"3306"}}}}',
445+
'{"servers":{"mysql":{"command":"npx","env":{"DB_HOST":"localhost","DB_PORT":"3306"}}}}',
392446
],
393447
'multiple servers mixed' => [
394-
'configKey' => 'mcpServers',
395-
'servers' => [
448+
'mcpServers',
449+
[
396450
'boost' => [
397451
'command' => 'php',
398452
'args' => ['artisan', 'boost:mcp'],
@@ -403,14 +457,14 @@ function newFileServerConfigurations(): array
403457
'env' => ['DB_HOST' => 'localhost'],
404458
],
405459
],
406-
'expectedJson' => '{"mcpServers":{"boost":{"command":"php","args":["artisan","boost:mcp"]},"mysql":{"command":"npx","args":["@benborla29/mcp-server-mysql"],"env":{"DB_HOST":"localhost"}}}}',
460+
'{"mcpServers":{"boost":{"command":"php","args":["artisan","boost:mcp"]},"mysql":{"command":"npx","args":["@benborla29/mcp-server-mysql"],"env":{"DB_HOST":"localhost"}}}}',
407461
],
408462
'custom config key' => [
409-
'configKey' => 'customKey',
410-
'servers' => [
463+
'customKey',
464+
[
411465
'test' => ['command' => 'test-cmd'],
412466
],
413-
'expectedJson' => '{"customKey":{"test":{"command":"test-cmd"}}}',
467+
'{"customKey":{"test":{"command":"test-cmd"}}}',
414468
],
415469
];
416470
}
@@ -419,49 +473,49 @@ function commentDetectionCases(): array
419473
{
420474
return [
421475
'plain JSON no comments' => [
422-
'content' => '{"servers": {"test": {"command": "npm"}}}',
423-
'expected' => false,
424-
'description' => 'Plain JSON should return false',
476+
'{"servers": {"test": {"command": "npm"}}}',
477+
false,
478+
'Plain JSON should return false',
425479
],
426480
'JSON with comments in strings' => [
427-
'content' => '{"exampleCode": "// here is the example code\n<?php", "url": "https://example.com/path"}',
428-
'expected' => false,
429-
'description' => 'Comments inside strings should not be detected as real comments',
481+
'{"exampleCode": "// here is the example code\n<?php", "url": "https://example.com/path"}',
482+
false,
483+
'Comments inside strings should not be detected as real comments',
430484
],
431485
'JSON5 with real line comments' => [
432-
'content' => '{"servers": {"test": "value"} // this is a real comment}',
433-
'expected' => true,
434-
'description' => 'Real JSON5 line comments should be detected',
486+
'{"servers": {"test": "value"} // this is a real comment}',
487+
true,
488+
'Real JSON5 line comments should be detected',
435489
],
436490
'JSON5 with comment at start of line' => [
437-
'content' => '{\n // This is a comment\n "servers": {}\n}',
438-
'expected' => true,
439-
'description' => 'Line comments at start should be detected',
491+
'{\n // This is a comment\n "servers": {}\n}',
492+
true,
493+
'Line comments at start should be detected',
440494
],
441495
'complex string with escaped quotes' => [
442-
'content' => '{"code": "console.log(\\"// not a comment\\");", "other": "value"}',
443-
'expected' => false,
444-
'description' => 'Comments in strings with escaped quotes should not be detected',
496+
'{"code": "console.log(\\"// not a comment\\");", "other": "value"}',
497+
false,
498+
'Comments in strings with escaped quotes should not be detected',
445499
],
446500
'multiple comments in strings' => [
447-
'content' => '{"example1": "// comment 1", "example2": "some // comment 2 here"}',
448-
'expected' => false,
449-
'description' => 'Multiple comments in different strings should not be detected',
501+
'{"example1": "// comment 1", "example2": "some // comment 2 here"}',
502+
false,
503+
'Multiple comments in different strings should not be detected',
450504
],
451505
'mixed real and string comments' => [
452-
'content' => '{"example": "// fake comment"} // real comment',
453-
'expected' => true,
454-
'description' => 'Should detect real comment even when fake ones exist in strings',
506+
'{"example": "// fake comment"} // real comment',
507+
true,
508+
'Should detect real comment even when fake ones exist in strings',
455509
],
456510
'empty string' => [
457-
'content' => '',
458-
'expected' => false,
459-
'description' => 'Empty string should return false',
511+
'',
512+
false,
513+
'Empty string should return false',
460514
],
461515
'single slash not comment' => [
462-
'content' => '{"path": "/usr/bin/test"}',
463-
'expected' => false,
464-
'description' => 'Single slash should not be detected as comment',
516+
'{"path": "/usr/bin/test"}',
517+
false,
518+
'Single slash should not be detected as comment',
465519
],
466520
];
467521
}
@@ -470,39 +524,39 @@ function trailingCommaCases(): array
470524
{
471525
return [
472526
'valid JSON no trailing comma' => [
473-
'content' => '{"servers": {"test": "value"}}',
474-
'expected' => true,
475-
'description' => 'Valid JSON should return true (is plain JSON)',
527+
'{"servers": {"test": "value"}}',
528+
true,
529+
'Valid JSON should return true (is plain JSON)',
476530
],
477531
'trailing comma in object same line' => [
478-
'content' => '{"servers": {"test": "value",}}',
479-
'expected' => false,
480-
'description' => 'Trailing comma in object should return false (is JSON5)',
532+
'{"servers": {"test": "value",}}',
533+
false,
534+
'Trailing comma in object should return false (is JSON5)',
481535
],
482536
'trailing comma in array same line' => [
483-
'content' => '{"items": ["a", "b", "c",]}',
484-
'expected' => false,
485-
'description' => 'Trailing comma in array should return false (is JSON5)',
537+
'{"items": ["a", "b", "c",]}',
538+
false,
539+
'Trailing comma in array should return false (is JSON5)',
486540
],
487541
'trailing comma across newlines in object' => [
488-
'content' => "{\n \"servers\": {\n \"test\": \"value\",\n }\n}",
489-
'expected' => false,
490-
'description' => 'Trailing comma across newlines in object should be detected',
542+
"{\n \"servers\": {\n \"test\": \"value\",\n }\n}",
543+
false,
544+
'Trailing comma across newlines in object should be detected',
491545
],
492546
'trailing comma across newlines in array' => [
493-
'content' => "{\n \"items\": [\n \"a\",\n \"b\",\n ]\n}",
494-
'expected' => false,
495-
'description' => 'Trailing comma across newlines in array should be detected',
547+
"{\n \"items\": [\n \"a\",\n \"b\",\n ]\n}",
548+
false,
549+
'Trailing comma across newlines in array should be detected',
496550
],
497551
'trailing comma with tabs and spaces' => [
498-
'content' => "{\n \"test\": \"value\",\t \n}",
499-
'expected' => false,
500-
'description' => 'Trailing comma with mixed whitespace should be detected',
552+
"{\n \"test\": \"value\",\t \n}",
553+
false,
554+
'Trailing comma with mixed whitespace should be detected',
501555
],
502556
'comma in string not trailing' => [
503-
'content' => '{"example": "value,", "other": "test"}',
504-
'expected' => true,
505-
'description' => 'Comma inside string should not be detected as trailing',
557+
'{"example": "value,", "other": "test"}',
558+
true,
559+
'Comma inside string should not be detected as trailing',
506560
],
507561
];
508562
}
@@ -511,52 +565,52 @@ function indentationDetectionCases(): array
511565
{
512566
return [
513567
'mcp.json5 servers indentation' => [
514-
'content' => "{\n // Here are comments within my JSON\n \"servers\": {\n \"mysql\": {\n \"command\": \"npx\"\n },\n \"laravel-boost\": {\n \"command\": \"php\"\n }\n },\n \"inputs\": []\n}",
515-
'position' => 200, // Position near end of servers block
516-
'expected' => 8,
517-
'description' => 'Should detect 8 spaces for server definitions in mcp.json5',
568+
"{\n // Here are comments within my JSON\n \"servers\": {\n \"mysql\": {\n \"command\": \"npx\"\n },\n \"laravel-boost\": {\n \"command\": \"php\"\n }\n },\n \"inputs\": []\n}",
569+
200, // Position near end of servers block
570+
8,
571+
'Should detect 8 spaces for server definitions in mcp.json5',
518572
],
519573
'nested object with 4-space base indent' => [
520-
'content' => "{\n \"config\": {\n \"server1\": {\n \"command\": \"test\"\n }\n }\n}",
521-
'position' => 80,
522-
'expected' => 8,
523-
'description' => 'Should detect 8 spaces for nested server definitions',
574+
"{\n \"config\": {\n \"server1\": {\n \"command\": \"test\"\n }\n }\n}",
575+
80,
576+
8,
577+
'Should detect 8 spaces for nested server definitions',
524578
],
525579
'no previous server definitions' => [
526-
'content' => "{\n \"inputs\": []\n}",
527-
'position' => 20,
528-
'expected' => 8,
529-
'description' => 'Should fallback to 8 spaces when no server definitions found',
580+
"{\n \"inputs\": []\n}",
581+
20,
582+
8,
583+
'Should fallback to 8 spaces when no server definitions found',
530584
],
531585
'deeper nesting with 2-space indent' => [
532-
'content' => "{\n \"config\": {\n \"servers\": {\n \"mysql\": {\n \"command\": \"test\"\n }\n }\n }\n}",
533-
'position' => 80,
534-
'expected' => 6,
535-
'description' => 'Should detect correct indentation in deeply nested structures',
586+
"{\n \"config\": {\n \"servers\": {\n \"mysql\": {\n \"command\": \"test\"\n }\n }\n }\n}",
587+
80,
588+
6,
589+
'Should detect correct indentation in deeply nested structures',
536590
],
537591
'single server definition at root level' => [
538-
'content' => "{\n\"mysql\": {\n \"command\": \"npx\"\n}\n}",
539-
'position' => 30,
540-
'expected' => 0,
541-
'description' => 'Should detect no indentation for root-level server definitions',
592+
"{\n\"mysql\": {\n \"command\": \"npx\"\n}\n}",
593+
30,
594+
0,
595+
'Should detect no indentation for root-level server definitions',
542596
],
543597
'multiple server definitions with consistent indentation' => [
544-
'content' => "{\n \"servers\": {\n \"mysql\": {\n \"command\": \"npx\"\n },\n \"postgres\": {\n \"command\": \"pg\"\n }\n }\n}",
545-
'position' => 150,
546-
'expected' => 8,
547-
'description' => 'Should consistently detect indentation across multiple servers',
598+
"{\n \"servers\": {\n \"mysql\": {\n \"command\": \"npx\"\n },\n \"postgres\": {\n \"command\": \"pg\"\n }\n }\n}",
599+
150,
600+
8,
601+
'Should consistently detect indentation across multiple servers',
548602
],
549603
'server definition with comments' => [
550-
'content' => "{\n // Comment here\n \"servers\": {\n \"mysql\": { // inline comment\n \"command\": \"npx\"\n }\n }\n}",
551-
'position' => 120,
552-
'expected' => 8,
553-
'description' => 'Should detect indentation correctly when comments are present',
604+
"{\n // Comment here\n \"servers\": {\n \"mysql\": { // inline comment\n \"command\": \"npx\"\n }\n }\n}",
605+
120,
606+
8,
607+
'Should detect indentation correctly when comments are present',
554608
],
555609
'empty content' => [
556-
'content' => '',
557-
'position' => 0,
558-
'expected' => 8,
559-
'description' => 'Should fallback to 8 spaces for empty content',
610+
'',
611+
0,
612+
8,
613+
'Should fallback to 8 spaces for empty content',
560614
],
561615
];
562616
}

0 commit comments

Comments
 (0)