Skip to content

Commit ee9866f

Browse files
quanruclaude
quanru
and
claude
committed
refactor(mcp): improve code quality following yuyutaotao style guide
Type Safety & Documentation: - Add aiAction and aiWaitFor optional methods to BaseAgent interface - Add comprehensive JSDoc for ensureAgent abstract method - Remove complex type assertions in favor of optional method calls Code Organization: - Extract magic numbers to named constants (defaultAppLoadingTimeoutMs, defaultAppLoadingCheckIntervalMs) - Add buildScreenshotContent helper method to reduce code duplication - Unify method naming: initAgentByBridgeMode → initBridgeModeAgent Error Handling & Logging: - Add debug logging to all catch blocks instead of silent failures - Replace error variable 'e' with descriptive 'error' Code Style: - Use optional chaining (?.) for cleaner agent.destroy calls - Simplify verbose comments (require usage explanations) - Improve comment clarity for temporary device creation All changes follow conventional commits and maintain backward compatibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 4b424a7 commit ee9866f

File tree

6 files changed

+63
-71
lines changed

6 files changed

+63
-71
lines changed

‎packages/android-mcp/src/android-tools.ts‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
import { type AndroidAgent, agentFromAdbDevice } from '@midscene/android';
22
import { z } from '@midscene/core';
3-
import { parseBase64 } from '@midscene/shared/img';
43
import { getDebug } from '@midscene/shared/logger';
54
import { BaseMidsceneTools, type ToolDefinition } from '@midscene/shared/mcp';
65

76
const debug = getDebug('mcp:android-tools');
87

8+
// Default timeout for app loading verification
9+
const defaultAppLoadingTimeoutMs = 10000;
10+
const defaultAppLoadingCheckIntervalMs = 2000;
11+
912
/**
1013
* Android-specific tools manager
1114
* Extends BaseMidsceneTools to provide Android ADB device connection tools
1215
*/
1316
export class AndroidMidsceneTools extends BaseMidsceneTools {
1417
protected createTemporaryDevice() {
15-
// Import AndroidDevice class
18+
// Use require to avoid circular dependency with @midscene/android
1619
const { AndroidDevice } = require('@midscene/android');
1720
// Create minimal temporary instance without connecting to device
1821
// The constructor doesn't establish ADB connection
@@ -25,8 +28,8 @@ export class AndroidMidsceneTools extends BaseMidsceneTools {
2528
// destroy it to create a new one with the new device
2629
try {
2730
await this.agent.destroy();
28-
} catch (e) {
29-
// Ignore cleanup errors
31+
} catch (error) {
32+
debug('Failed to destroy agent during cleanup:', error);
3033
}
3134
this.agent = undefined;
3235
}
@@ -78,26 +81,21 @@ export class AndroidMidsceneTools extends BaseMidsceneTools {
7881
await agent.aiWaitFor(
7982
'the app has finished loading and is ready to use',
8083
{
81-
timeoutMs: 10000,
82-
checkIntervalMs: 2000,
84+
timeoutMs: defaultAppLoadingTimeoutMs,
85+
checkIntervalMs: defaultAppLoadingCheckIntervalMs,
8386
},
8487
);
8588
}
8689

8790
const screenshot = await agent.page.screenshotBase64();
88-
const { mimeType, body } = parseBase64(screenshot);
8991

9092
return {
9193
content: [
9294
{
9395
type: 'text',
9496
text: `Connected to Android device${deviceId ? `: ${deviceId}` : ' (auto-detected)'}${uri ? ` and launched: ${uri} (app ready)` : ''}`,
9597
},
96-
{
97-
type: 'image',
98-
data: body,
99-
mimeType,
100-
},
98+
...this.buildScreenshotContent(screenshot),
10199
],
102100
isError: false,
103101
};

‎packages/ios-mcp/src/ios-tools.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
import { z } from '@midscene/core';
22
import { type IOSAgent, agentFromWebDriverAgent } from '@midscene/ios';
3-
import { parseBase64 } from '@midscene/shared/img';
43
import { getDebug } from '@midscene/shared/logger';
54
import { BaseMidsceneTools, type ToolDefinition } from '@midscene/shared/mcp';
65

76
const debug = getDebug('mcp:ios-tools');
87

8+
// Default timeout for app loading verification
9+
const defaultAppLoadingTimeoutMs = 10000;
10+
const defaultAppLoadingCheckIntervalMs = 2000;
11+
912
/**
1013
* iOS-specific tools manager
1114
* Extends BaseMidsceneTools to provide iOS WebDriverAgent connection tools
1215
*/
1316
export class IOSMidsceneTools extends BaseMidsceneTools {
1417
protected createTemporaryDevice() {
15-
// Import IOSDevice class
18+
// Use require to avoid circular dependency with @midscene/ios
1619
const { IOSDevice } = require('@midscene/ios');
1720
// Create minimal temporary instance without connecting to WebDriverAgent
1821
// The constructor only initializes WDA backend, doesn't establish connection
@@ -57,26 +60,21 @@ export class IOSMidsceneTools extends BaseMidsceneTools {
5760
await agent.aiWaitFor(
5861
'the app has finished loading and is ready to use',
5962
{
60-
timeoutMs: 10000,
61-
checkIntervalMs: 2000,
63+
timeoutMs: defaultAppLoadingTimeoutMs,
64+
checkIntervalMs: defaultAppLoadingCheckIntervalMs,
6265
},
6366
);
6467
}
6568

6669
const screenshot = await agent.page.screenshotBase64();
67-
const { mimeType, body } = parseBase64(screenshot);
6870

6971
return {
7072
content: [
7173
{
7274
type: 'text',
7375
text: `Connected to iOS device${uri ? ` and launched: ${uri} (app ready)` : ''}`,
7476
},
75-
{
76-
type: 'image',
77-
data: body,
78-
mimeType,
79-
},
77+
...this.buildScreenshotContent(screenshot),
8078
],
8179
isError: false,
8280
};

‎packages/shared/src/mcp/base-tools.ts‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { parseBase64 } from '@midscene/shared/img';
12
import { getDebug } from '@midscene/shared/logger';
23
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
34
import {
@@ -20,7 +21,11 @@ export abstract class BaseMidsceneTools implements IMidsceneTools {
2021
protected toolDefinitions: ToolDefinition[] = [];
2122

2223
/**
23-
* Must be implemented by subclasses to create platform-specific agent
24+
* Ensure agent is initialized and ready for use.
25+
* Must be implemented by subclasses to create platform-specific agent.
26+
* @param initParam Optional initialization parameter (platform-specific, e.g., URL, device ID)
27+
* @returns Promise resolving to initialized agent instance
28+
* @throws Error if agent initialization fails
2429
*/
2530
protected abstract ensureAgent(initParam?: string): Promise<BaseAgent>;
2631

@@ -152,11 +157,9 @@ export abstract class BaseMidsceneTools implements IMidsceneTools {
152157
} finally {
153158
if (!process.env.MIDSCENE_MCP_DISABLE_AGENT_AUTO_DESTROY) {
154159
try {
155-
if (this.agent?.destroy) {
156-
await this.agent.destroy();
157-
}
158-
} catch (e) {
159-
debug('Failed to destroy agent during cleanup:', e);
160+
await this.agent?.destroy?.();
161+
} catch (error) {
162+
debug('Failed to destroy agent during cleanup:', error);
160163
}
161164
this.agent = undefined;
162165
}
@@ -165,11 +168,23 @@ export abstract class BaseMidsceneTools implements IMidsceneTools {
165168
}
166169

167170
/**
168-
* Cleanup method
171+
* Cleanup method - destroy agent and release resources
169172
*/
170173
public async closeBrowser(): Promise<void> {
171-
if (this.agent?.destroy) {
172-
await this.agent.destroy();
173-
}
174+
await this.agent?.destroy?.();
175+
}
176+
177+
/**
178+
* Helper: Convert base64 screenshot to image content array
179+
*/
180+
protected buildScreenshotContent(screenshot: string) {
181+
const { mimeType, body } = parseBase64(screenshot);
182+
return [
183+
{
184+
type: 'image' as const,
185+
data: body,
186+
mimeType,
187+
},
188+
];
174189
}
175190
}

‎packages/shared/src/mcp/tool-generator.ts‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,10 @@ export function generateToolsFromActionSpace(
3636
handler: async (args: Record<string, unknown>) => {
3737
const agent = await getAgent();
3838

39-
// Call the action through agent's action method
39+
// Call the action through agent's aiAction method
4040
// args already contains the unwrapped parameters (e.g., { locate: {...} })
41-
if ('aiAction' in agent && typeof agent.aiAction === 'function') {
42-
await (
43-
agent as BaseAgent & {
44-
aiAction: (
45-
desc: string,
46-
params: Record<string, unknown>,
47-
) => Promise<void>;
48-
}
49-
).aiAction(`Use the action "${action.name}"`, {
41+
if (agent.aiAction) {
42+
await agent.aiAction(`Use the action "${action.name}"`, {
5043
...args,
5144
});
5245
}
@@ -129,15 +122,8 @@ export function generateCommonTools(
129122
checkIntervalMs?: number;
130123
};
131124

132-
if ('aiWaitFor' in agent && typeof agent.aiWaitFor === 'function') {
133-
await (
134-
agent as BaseAgent & {
135-
aiWaitFor: (
136-
assertion: string,
137-
options: Record<string, unknown>,
138-
) => Promise<void>;
139-
}
140-
).aiWaitFor(assertion, { timeoutMs, checkIntervalMs });
125+
if (agent.aiWaitFor) {
126+
await agent.aiWaitFor(assertion, { timeoutMs, checkIntervalMs });
141127
}
142128

143129
return {

‎packages/shared/src/mcp/types.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ export interface BaseAgent {
7272
page?: {
7373
screenshotBase64(): Promise<string>;
7474
};
75+
aiAction?: (
76+
description: string,
77+
params: Record<string, unknown>,
78+
) => Promise<void>;
79+
aiWaitFor?: (
80+
assertion: string,
81+
options: Record<string, unknown>,
82+
) => Promise<void>;
7583
}
7684

7785
/**

‎packages/web-mcp/src/web-tools.ts‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,7 @@ export class WebMidsceneTools extends BaseMidsceneTools {
1717
);
1818

1919
protected createTemporaryDevice() {
20-
// Import PuppeteerWebPage class using dynamic ESM import
21-
// This is intentionally synchronous despite the async nature of createTemporaryDevice
22-
// because we need the class constructor immediately for tool initialization
23-
// The alternative would be to make all tool initialization async, which is a larger refactor
24-
// eslint-disable-next-line @typescript-eslint/no-var-requires
20+
// Synchronous require needed for tool initialization
2521
const { PuppeteerWebPage } = require('@midscene/web');
2622

2723
// Create minimal mock page object that satisfies the interface
@@ -48,11 +44,9 @@ export class WebMidsceneTools extends BaseMidsceneTools {
4844
// Re-init if URL provided
4945
if (this.agent && openNewTabWithUrl) {
5046
try {
51-
if (this.agent.destroy) {
52-
await this.agent.destroy();
53-
}
54-
} catch (e) {
55-
console.debug('Failed to destroy agent during re-init:', e);
47+
await this.agent?.destroy?.();
48+
} catch (error) {
49+
console.debug('Failed to destroy agent during re-init:', error);
5650
}
5751
this.agent = undefined;
5852
}
@@ -67,7 +61,7 @@ export class WebMidsceneTools extends BaseMidsceneTools {
6761
'Bridge mode requires a URL. Use web_connect tool to connect to a page first.',
6862
);
6963
}
70-
this.agent = (await this.initAgentByBridgeMode(
64+
this.agent = (await this.initBridgeModeAgent(
7165
openNewTabWithUrl,
7266
)) as unknown as BaseAgent;
7367
} else {
@@ -80,7 +74,7 @@ export class WebMidsceneTools extends BaseMidsceneTools {
8074
return this.agent;
8175
}
8276

83-
private async initAgentByBridgeMode(
77+
private async initBridgeModeAgent(
8478
url?: string,
8579
): Promise<AgentOverChromeBridge> {
8680
const agent = new AgentOverChromeBridge({ closeConflictServer: true });
@@ -133,20 +127,13 @@ export class WebMidsceneTools extends BaseMidsceneTools {
133127
};
134128
}
135129

136-
const { parseBase64 } = await import('@midscene/shared/img');
137-
const { mimeType, body } = parseBase64(screenshot);
138-
139130
return {
140131
content: [
141132
{
142133
type: 'text',
143134
text: `Connected to: ${url}`,
144135
},
145-
{
146-
type: 'image',
147-
data: body,
148-
mimeType,
149-
},
136+
...this.buildScreenshotContent(screenshot),
150137
],
151138
};
152139
},

0 commit comments

Comments
 (0)