Skip to content

Commit 3817ca5

Browse files
Keep the separating comma out of trailing comments when injecting MCP servers (#977)
* Keep the separating comma out of trailing comments when injecting MCP servers * Skip single-quoted strings when masking comments so a // inside a JSON5 string is not read as one * Cut the comma-fix comment blocks to one line where they earn it --------- Co-authored-by: Pushpak Chhajed <pushpak1300@gmail.com>
1 parent 8d8cb70 commit 3817ca5

2 files changed

Lines changed: 90 additions & 12 deletions

File tree

src/Install/Mcp/FileWriter.php

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,14 @@ protected function injectIntoExistingConfigKey(string $content, array $matches):
150150
$commaPosition = $this->findCommaInsertionPoint($content, $openBracePos, $closeBracePos);
151151

152152
if ($commaPosition !== -1) {
153-
$newContent = substr_replace($content, ',', $commaPosition, 0);
154-
$newContent = substr_replace($newContent, $serversJson, $commaPosition + 1, 0);
153+
// Anything non-blank before the brace is a trailing comment: comma goes ahead of it, servers after it.
154+
if (trim(substr($content, $commaPosition, $closeBracePos - $commaPosition)) === '') {
155+
$newContent = substr_replace($content, ',', $commaPosition, 0);
156+
$newContent = substr_replace($newContent, $serversJson, $commaPosition + 1, 0);
157+
} else {
158+
$newContent = substr_replace($content, $serversJson, $closeBracePos, 0);
159+
$newContent = substr_replace($newContent, ',', $commaPosition, 0);
160+
}
155161
} else {
156162
$newContent = substr_replace($content, $serversJson, $closeBracePos, 0);
157163
}
@@ -289,24 +295,24 @@ protected function needsCommaBeforeClosingBrace(string $content, int $openBraceP
289295

290296
protected function findCommaInsertionPoint(string $content, int $openBracePos, int $closeBracePos): int
291297
{
298+
// Strings are matched only to skip them; comments become equal-length spaces so offsets still line up.
299+
$masked = preg_replace_callback(
300+
'/"(?:\\\\.|[^"\\\\])*"|\'(?:\\\\.|[^\'\\\\])*\'|(\/\/[^\r\n]*)|(\/\*[\s\S]*?\*\/)/',
301+
fn (array $match): string => ($match[1] ?? '') !== '' || ($match[2] ?? '') !== ''
302+
? str_repeat(' ', strlen($match[0]))
303+
: $match[0],
304+
$content
305+
) ?? $content;
306+
292307
// Work backwards from closing brace to find last meaningful character
293308
for ($i = $closeBracePos - 1; $i > $openBracePos; $i--) {
294-
$char = $content[$i];
309+
$char = $masked[$i];
295310

296311
// Skip whitespace and newlines
297312
if (in_array($char, [' ', "\t", "\n", "\r"], true)) {
298313
continue;
299314
}
300315

301-
// Skip comments (simple approach - if we hit //, skip to start of line)
302-
if ($i > 0 && $content[$i - 1] === '/' && $char === '/') {
303-
// Find start of this line
304-
$lineStart = strrpos($content, "\n", $i - strlen($content)) ?: 0;
305-
$i = $lineStart;
306-
307-
continue;
308-
}
309-
310316
// Found last meaningful character, comma goes after it
311317
if ($char !== ',') {
312318
return $i + 1;

tests/Unit/Install/Mcp/FileWriterTest.php

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,78 @@
451451
);
452452
});
453453

454+
test('adds the comma outside a trailing comment in the servers object', function (): void {
455+
$writtenContent = '';
456+
457+
$contentWithTrailingComment = <<<'JSON5'
458+
{
459+
"mcpServers": {
460+
"context7": {
461+
"command": "npx"
462+
} // docs lookup
463+
}
464+
}
465+
JSON5;
466+
467+
mockFileOperations(
468+
fileExists: true,
469+
content: $contentWithTrailingComment,
470+
capturedContent: $writtenContent
471+
);
472+
473+
File::shouldReceive('size')->andReturn(200);
474+
475+
$result = (new FileWriter('/path/to/mcp.json'))
476+
->addServerConfig('boost', [
477+
'command' => 'php',
478+
'args' => ['artisan', 'boost:mcp'],
479+
])
480+
->save();
481+
482+
$withoutComments = preg_replace('/\/\/[^\n]*/', '', $writtenContent);
483+
484+
expect($result)->toBeTrue()
485+
->and(json_decode((string) $withoutComments, true))->not->toBeNull()
486+
->and($writtenContent)->toContain(
487+
'"boost"', // New server added
488+
'"context7"', // Existing server preserved
489+
'// docs lookup' // Comment preserved
490+
);
491+
});
492+
493+
test('does not read a // inside a single-quoted string as a comment', function (): void {
494+
$writtenContent = '';
495+
496+
$singleQuotedUrl = <<<'JSON5'
497+
{
498+
'mcpServers': {
499+
'remote': { 'url': 'https://example.test/sse' }
500+
}
501+
}
502+
JSON5;
503+
504+
mockFileOperations(
505+
fileExists: true,
506+
content: $singleQuotedUrl,
507+
capturedContent: $writtenContent
508+
);
509+
510+
File::shouldReceive('size')->andReturn(200);
511+
512+
$result = (new FileWriter('/path/to/mcp.json'))
513+
->addServerConfig('boost', [
514+
'command' => 'php',
515+
'args' => ['artisan', 'boost:mcp'],
516+
])
517+
->save();
518+
519+
expect($result)->toBeTrue()
520+
->and($writtenContent)->toContain(
521+
"'url': 'https://example.test/sse'", // URL survives untouched
522+
'"boost"' // New server added
523+
);
524+
});
525+
454526
test('updates JSON5 file with only single-quoted strings', function (): void {
455527
$writtenContent = '';
456528
$singleQuotedJson5 = <<<'JSON5'

0 commit comments

Comments
 (0)