Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
20 changes: 16 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"package.json"
],
"dependencies": {
"@modelcontextprotocol/sdk": "^1.17.1",
"@modelcontextprotocol/sdk": "1.17.1",
"@types/node": "^24.1.0"
},
"devDependencies": {
Expand Down
1 change: 1 addition & 0 deletions src/tools/xcodebuild/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ export async function xcodebuildBuildTool(args: any) {
guidance: summary.success
? [
`Build completed successfully in ${duration}ms`,
...(summary.warningCount > 0 ? [`⚠️ ${summary.warningCount} warning(s) detected`] : []),
...(usedSmartDestination ? [`Used smart simulator: ${finalConfig.destination}`] : []),
...(hasPreferredConfig ? [`Applied cached project preferences`] : []),
`Use 'xcodebuild-get-details' with buildId '${cacheId}' for full logs`,
Expand Down
3 changes: 3 additions & 0 deletions src/utils/response-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ export function extractBuildSummary(output: string, stderr: string, exitCode: nu
hasErrors: errors.length > 0,
hasWarnings: warnings.length > 0,
firstError: errors[0]?.trim(),
// Include first 10 errors and warnings for immediate visibility
errors: errors.slice(0, 10).map(e => e.trim()),
warnings: warnings.slice(0, 10).map(w => w.trim()),
Comment on lines +196 to +198
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

This PR introduces new fields (errors and warnings arrays) to the build summary return value. However, the PR title and description only mention fixing the @modelcontextprotocol/sdk version issue. This is an unrelated feature addition that should either:

  1. Be documented in the PR description to explain why these fields are being added
  2. Be moved to a separate PR focused on this feature

Additionally, this is a breaking change to the public API of extractBuildSummary (new required fields in the return object) but there's no documentation or discussion about backwards compatibility.

Suggested change
// Include first 10 errors and warnings for immediate visibility
errors: errors.slice(0, 10).map(e => e.trim()),
warnings: warnings.slice(0, 10).map(w => w.trim()),

Copilot uses AI. Check for mistakes.
buildSizeBytes: output.length + stderr.length,
};
}
Expand Down
26 changes: 26 additions & 0 deletions tests/__tests__/utils/response-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,8 @@ Total time: 45.2 seconds
hasErrors: false,
hasWarnings: false,
firstError: undefined,
errors: [],
warnings: [],
buildSizeBytes: output.length,
});
});
Expand All @@ -368,6 +370,8 @@ error: Build failed
hasErrors: true,
hasWarnings: false,
firstError: 'error: Build failed',
errors: ['error: Build failed', '** BUILD FAILED **'],
warnings: [],
buildSizeBytes: output.length + stderr.length,
});
});
Expand All @@ -383,6 +387,10 @@ warning: Deprecated API usage

expect(summary.warningCount).toBe(2);
expect(summary.hasWarnings).toBe(true);
expect(summary.warnings).toEqual([
"warning: Unused variable 'foo'",
'warning: Deprecated API usage',
]);
});

it('should handle output without timing info', () => {
Expand All @@ -404,6 +412,24 @@ error: Some error that happened during success
expect(summary.success).toBe(true); // Exit code 0 and has success indicator
expect(summary.hasErrors).toBe(true); // But still has errors
expect(summary.errorCount).toBe(1);
expect(summary.errors).toEqual(['error: Some error that happened during success']);
});

it('should limit errors and warnings to first 10', () => {
const warnings = Array.from({ length: 15 }, (_, i) => `warning: Warning number ${i + 1}`).join(
'\n'
);
const errors = Array.from({ length: 12 }, (_, i) => `error: Error number ${i + 1}`).join('\n');
const output = `${warnings}\n${errors}\n** BUILD FAILED **`;

const summary = extractBuildSummary(output, '', 1);

expect(summary.warningCount).toBe(15);
expect(summary.errorCount).toBe(13); // 12 errors + BUILD FAILED
expect(summary.warnings).toHaveLength(10); // Limited to 10
expect(summary.errors).toHaveLength(10); // Limited to 10
expect(summary.warnings[0]).toBe('warning: Warning number 1');
expect(summary.warnings[9]).toBe('warning: Warning number 10');
});
});

Expand Down