Skip to content

Commit 1f4f3fb

Browse files
committed
fix : Instrumentation,refactoring and bump version
1 parent 32eead6 commit 1f4f3fb

File tree

11 files changed

+127
-169
lines changed

11 files changed

+127
-169
lines changed

src/lib/utils.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import sharp from "sharp";
22
import type { ApiResponse } from "./apiClient.js";
33
import { BrowserStackConfig } from "./types.js";
44
import { getBrowserStackAuth } from "./get-auth.js";
5+
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
6+
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
7+
import { trackMCP } from "../index.js";
58

69
export function sanitizeUrlParam(param: string): string {
710
// Remove any characters that could be used for command injection
@@ -62,3 +65,27 @@ export async function fetchFromBrowserStackAPI(
6265

6366
return res.json();
6467
}
68+
69+
function errorContent(message: string): CallToolResult {
70+
return {
71+
content: [{ type: "text", text: message }],
72+
isError: true,
73+
};
74+
}
75+
76+
export function handleMCPError(
77+
toolName: string,
78+
server: McpServer,
79+
config: BrowserStackConfig,
80+
error: unknown,
81+
) {
82+
trackMCP(toolName, server.server.getClientVersion()!, error, config);
83+
84+
const errorMessage = error instanceof Error ? error.message : "Unknown error";
85+
86+
const readableToolName = toolName.replace(/([A-Z])/g, " $1").toLowerCase();
87+
88+
return errorContent(
89+
`Failed to ${readableToolName}: ${errorMessage}. Please open an issue on GitHub if the problem persists`,
90+
);
91+
}

src/tools/build-insights.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { z } from "zod";
33
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
44
import logger from "../logger.js";
55
import { BrowserStackConfig } from "../lib/types.js";
6-
import { fetchFromBrowserStackAPI } from "../lib/utils.js";
6+
import { fetchFromBrowserStackAPI, handleMCPError } from "../lib/utils.js";
77

88
// Tool function that fetches build insights from two APIs
99
export async function fetchBuildInsightsTool(
@@ -80,16 +80,7 @@ export default function addBuildInsightsTools(
8080
try {
8181
return await fetchBuildInsightsTool(args, config);
8282
} catch (error) {
83-
const errorMessage =
84-
error instanceof Error ? error.message : "Unknown error";
85-
return {
86-
content: [
87-
{
88-
type: "text",
89-
text: `Error during fetching build insights: ${errorMessage}`,
90-
},
91-
],
92-
};
83+
return handleMCPError("fetchBuildInsights", server, config, error);
9384
}
9485
},
9586
);

src/tools/list-test-files.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ export async function addListTestFiles(args: any): Promise<CallToolResult> {
77
const { dirs, language, framework } = args;
88
let testFiles: string[] = [];
99

10+
if (!dirs || dirs.length === 0) {
11+
throw new Error(
12+
"No directories provided to add the test files. Please provide test directories to add percy snapshot commands.",
13+
);
14+
}
15+
1016
for (const dir of dirs) {
1117
const files = await listTestFiles({
1218
language,

src/tools/percy-sdk.ts

Lines changed: 39 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { trackMCP } from "../index.js";
22
import { BrowserStackConfig } from "../lib/types.js";
3-
import { fetchPercyChanges } from "./percy-change.js";
3+
import { fetchPercyChanges } from "./review-agent.js";
44
import { addListTestFiles } from "./list-test-files.js";
55
import { runPercyScan } from "./run-percy-scan.js";
66
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
@@ -25,14 +25,14 @@ import {
2525
FetchPercyChangesParamsShape,
2626
ManagePercyBuildApprovalParamsShape,
2727
} from "./sdk-utils/common/schema.js";
28+
import { handleMCPError } from "../lib/utils.js";
2829

2930
export function registerPercyTools(
3031
server: McpServer,
3132
config: BrowserStackConfig,
3233
) {
3334
const tools: Record<string, any> = {};
3435

35-
// Register setupPercyVisualTesting
3636
tools.setupPercyVisualTesting = server.tool(
3737
"setupPercyVisualTesting",
3838
SETUP_PERCY_DESCRIPTION,
@@ -46,26 +46,11 @@ export function registerPercyTools(
4646
);
4747
return setUpPercyHandler(args, config);
4848
} catch (error) {
49-
trackMCP(
50-
"setupPercyVisualTesting",
51-
server.server.getClientVersion()!,
52-
error,
53-
config,
54-
);
55-
return {
56-
content: [
57-
{
58-
type: "text",
59-
text: error instanceof Error ? error.message : String(error),
60-
},
61-
],
62-
isError: true,
63-
};
49+
return handleMCPError("setupPercyVisualTesting", server, config, error);
6450
}
6551
},
6652
);
6753

68-
// Register addPercySnapshotCommands
6954
tools.addPercySnapshotCommands = server.tool(
7055
"addPercySnapshotCommands",
7156
PERCY_SNAPSHOT_COMMANDS_DESCRIPTION,
@@ -79,26 +64,16 @@ export function registerPercyTools(
7964
);
8065
return await updateTestsWithPercyCommands(args);
8166
} catch (error) {
82-
trackMCP(
67+
return handleMCPError(
8368
"addPercySnapshotCommands",
84-
server.server.getClientVersion()!,
85-
error,
69+
server,
8670
config,
71+
error,
8772
);
88-
return {
89-
content: [
90-
{
91-
type: "text",
92-
text: error instanceof Error ? error.message : String(error),
93-
},
94-
],
95-
isError: true,
96-
};
9773
}
9874
},
9975
);
10076

101-
// Register listTestFiles
10277
tools.listTestFiles = server.tool(
10378
"listTestFiles",
10479
LIST_TEST_FILES_DESCRIPTION,
@@ -108,21 +83,7 @@ export function registerPercyTools(
10883
trackMCP("listTestFiles", server.server.getClientVersion()!, config);
10984
return addListTestFiles(args);
11085
} catch (error) {
111-
trackMCP(
112-
"listTestFiles",
113-
server.server.getClientVersion()!,
114-
error,
115-
config,
116-
);
117-
return {
118-
content: [
119-
{
120-
type: "text",
121-
text: error instanceof Error ? error.message : String(error),
122-
},
123-
],
124-
isError: true,
125-
};
86+
return handleMCPError("listTestFiles", server, config, error);
12687
}
12788
},
12889
);
@@ -132,16 +93,30 @@ export function registerPercyTools(
13293
"Run a Percy visual test scan. Example prompts : Run this Percy build/scan. Never run percy scan/build without this tool",
13394
RunPercyScanParamsShape,
13495
async (args) => {
135-
return runPercyScan(args, config);
96+
try {
97+
trackMCP("runPercyScan", server.server.getClientVersion()!, config);
98+
return runPercyScan(args, config);
99+
} catch (error) {
100+
return handleMCPError("runPercyScan", server, config, error);
101+
}
136102
},
137103
);
138104

139105
tools.fetchPercyChanges = server.tool(
140106
"fetchPercyChanges",
141-
"Retrieves and summarizes all visual changes detected by Percy between the latest and previous builds, helping quickly review what has changed in your project.",
107+
"Retrieves and summarizes all visual changes detected by Percy AI between the latest and previous builds, helping quickly review what has changed in your project.",
142108
FetchPercyChangesParamsShape,
143109
async (args) => {
144-
return await fetchPercyChanges(args, config);
110+
try {
111+
trackMCP(
112+
"fetchPercyChanges",
113+
server.server.getClientVersion()!,
114+
config,
115+
);
116+
return await fetchPercyChanges(args, config);
117+
} catch (error) {
118+
return handleMCPError("fetchPercyChanges", server, config, error);
119+
}
145120
},
146121
);
147122

@@ -150,7 +125,21 @@ export function registerPercyTools(
150125
"Approve or reject a Percy build",
151126
ManagePercyBuildApprovalParamsShape,
152127
async (args) => {
153-
return await approveOrDeclinePercyBuild(args, config);
128+
try {
129+
trackMCP(
130+
"managePercyBuildApproval",
131+
server.server.getClientVersion()!,
132+
config,
133+
);
134+
return await approveOrDeclinePercyBuild(args, config);
135+
} catch (error) {
136+
return handleMCPError(
137+
"managePercyBuildApproval",
138+
server,
139+
config,
140+
error,
141+
);
142+
}
154143
},
155144
);
156145

src/tools/percy-snapshot-utils/detect-test-files.ts

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import fs from "fs";
22
import path from "path";
3-
import logger from "../../logger.js";
43

54
import {
65
SDKSupportedLanguage,
@@ -38,7 +37,7 @@ async function walkDir(
3837
}
3938
}
4039
} catch {
41-
logger.error(`Failed to read directory: ${dir}`);
40+
// ignore
4241
}
4342

4443
return result;
@@ -54,7 +53,6 @@ async function fileContainsRegex(
5453
const content = await fs.promises.readFile(filePath, "utf8");
5554
return regexes.some((re) => re.test(content));
5655
} catch {
57-
logger.warn(`Failed to read file: ${filePath}`);
5856
return false;
5957
}
6058
}
@@ -69,7 +67,6 @@ async function batchRegexCheck(
6967
regexes.length > 0 ? regexes.some((re) => re.test(content)) : false,
7068
);
7169
} catch {
72-
logger.warn(`Failed to read file: ${filePath}`);
7370
return regexGroups.map(() => false);
7471
}
7572
}
@@ -110,7 +107,6 @@ export async function listTestFiles(
110107
const config = TEST_FILE_DETECTION[language];
111108

112109
if (!config) {
113-
logger.error(`Unsupported language: ${language}`);
114110
return [];
115111
}
116112

@@ -135,7 +131,6 @@ export async function listTestFiles(
135131

136132
if (config.namePatterns.some((pattern) => pattern.test(fileName))) {
137133
candidateFiles.set(file, score);
138-
logger.debug(`File matched by name pattern: ${file} (score: ${score})`);
139134
}
140135
}
141136

@@ -147,7 +142,6 @@ export async function listTestFiles(
147142
const fileName = path.basename(file);
148143
const score = getFileScore(fileName, config);
149144
candidateFiles.set(file, score);
150-
logger.debug(`File matched by content regex: ${file} (score: ${score})`);
151145
}
152146
});
153147

@@ -158,16 +152,12 @@ export async function listTestFiles(
158152
try {
159153
const featureFiles = await walkDir(baseDir, [".feature"], 6);
160154
featureFiles.forEach((file) => candidateFiles.set(file, 2));
161-
logger.info(`Added ${featureFiles.length} SpecFlow .feature files`);
162155
} catch {
163-
logger.warn(
164-
`Failed to collect SpecFlow .feature files from baseDir: ${baseDir}`,
165-
);
156+
// ignore
166157
}
167158
}
168159

169160
if (candidateFiles.size === 0) {
170-
logger.info("No test files found matching patterns");
171161
return [];
172162
}
173163

@@ -185,7 +175,6 @@ export async function listTestFiles(
185175
const isUITest = await isLikelyUITest(file);
186176

187177
if (isUITest) {
188-
logger.debug(`File included - strong UI indicators: ${file}`);
189178
return file;
190179
}
191180

@@ -198,43 +187,29 @@ export async function listTestFiles(
198187
config.excludeRegex || [],
199188
]);
200189

201-
// Skip if explicitly excluded (mocks, unit tests, etc.)
202190
if (shouldExclude) {
203-
logger.debug(`File excluded by exclude regex: ${file}`);
204191
return null;
205192
}
206193

207-
// Skip backend tests in any mode
208194
if (hasBackend) {
209-
logger.debug(`File excluded as backend test: ${file}`);
210195
return null;
211196
}
212197

213-
// Include if has explicit UI drivers
214198
if (hasExplicitUI) {
215-
logger.debug(`File included - explicit UI drivers: ${file}`);
216199
return file;
217200
}
218201

219-
// Include if has UI indicators (for cases where drivers aren't explicitly imported)
220202
if (hasUIIndicators) {
221-
logger.debug(`File included - UI indicators: ${file}`);
222203
return file;
223204
}
224205

225-
// In non-strict mode, include high-scoring test files even without explicit UI patterns
226206
if (!strictMode) {
227207
const score = candidateFiles.get(file) || 0;
228208
if (score >= 3) {
229-
// High confidence UI test based on naming
230-
logger.debug(
231-
`File included - high confidence score: ${file} (score: ${score})`,
232-
);
233209
return file;
234210
}
235211
}
236212

237-
logger.debug(`File excluded - no UI patterns detected: ${file}`);
238213
return null;
239214
});
240215

@@ -251,9 +226,6 @@ export async function listTestFiles(
251226
return scoreB - scoreA;
252227
});
253228

254-
logger.info(
255-
`Returning ${uiFiles.length} UI test files from ${candidateFiles.size} total test files`,
256-
);
257229
return uiFiles;
258230
}
259231

0 commit comments

Comments
 (0)