Skip to content

Commit 8434f8e

Browse files
committed
refactor(cdp): Phase 87/88 — hardening + benchmark-surfaced fixes
Phase 87 — post-refactor hardening (after Phase 86b's cdp-client extraction): - Extract cdp/recovery.ts (probeFreshness, recoverFromStaleTarget) from utils.ts. Replaces error-string matching for stale detection with the __RN_AGENT.__v version probe as the primary signal. utils.ts shrinks; recovery is testable in isolation. - Extract cdp/connect.ts (autoConnect, discoverAndConnect, connectToTarget, connectWs) via a ConnectContext. Shrinks CDPClient facade to a thin shell over the cdp/* modules. - Extract cdp/helper-expr.ts — helperExpr + bridgeWithFallback now pure functions testable without a CDPClient. - cdp/state.ts now exposes a setter-based ResettableState interface, replacing the `as unknown as CDPResettableState` cast. Rename a private field and the type system actually catches it. - D631 (S1): cache the freshness probe for 2s per (client, connectionGeneration) via a WeakMap. Eliminates the 30-150ms extra round-trip on back-to-back tool calls. - D632 (S2): RingBuffer gains an optional indexKey option building a parallel Map<key, entry>. Network event handlers + tools/network-body.ts swap findLast (O(n)) for getByKey (O(1)). - D634: optional `code` field on ResultEnvelope — ToolErrorCode union of STALE_TARGET / HELPERS_STALE / RECONNECT_TIMEOUT / NOT_CONNECTED / HELPERS_NOT_INJECTED. Agents can branch on code instead of regex on error text. - B110/D630: MCP server version now comes from package.json at module load (readFileSync of ../package.json). Replaces the hardcoded '0.10.1' literal in index.ts that had drifted 7 minor versions from the real version. sync-versions.sh gets a regex guard that fails CI if any future `version: '...'` literal appears in src/. Phase 88 — benchmark-surfaced plugin fixes: - B111/D635: cdp_connect now accepts targetId + bundleId filters threaded through autoConnect -> discoverAndConnect -> selectTarget. Fixes silent attachment to zombie Expo Go host.exp.Exponent pages when the real com.<app> target is also present. selectTarget also supports a soft preferredBundleId for auto-selection. Back-compat preserved — legacy string signature still works. - B113/D636: device_screenshot dropped the unconditional `--format` flag (agent-device >= 0.8.0 rejects it with Unknown flag: --format). Now uses --out <path> explicitly. The handler was split into a pure buildScreenshotArgs() + thin delegate so tests have no runAgentDevice coupling. Tests: +45 across the session (158 -> 203). - cdp-state.test.js: 7 tests for resetState setter interface + sleep - cdp-discovery.test.js: 9 tests for filterValidTargets + selectTarget, plus 7 for the B111 filter matrix (targetId, bundleId, preferredBundleId, precedence, legacy signature) - cdp-transport.test.js: 11 tests for sendWithTimeout, rejectAllPending, handleMessage, including malformed-message tolerance - device-screenshot.test.js: 5 tests guarding against --format regression - ring-buffer.test.js: 7 new tests for the indexKey feature (eviction, key-reuse, clear semantics) All 203 tests pass; TypeScript clean; dist/ rebuilt.
1 parent 82de735 commit 8434f8e

36 files changed

Lines changed: 1657 additions & 652 deletions

scripts/cdp-bridge/dist/cdp-client.js

Lines changed: 54 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import { detectBridge } from './bridge-detector.js';
44
import { logger } from './logger.js';
55
import { performSetup, reinjectHelpers as reinjectHelpersFn } from './cdp/setup.js';
66
import { resetState, setActiveFlag, clearActiveFlag, sleep } from './cdp/state.js';
7-
import { CDP_TIMEOUT_FAST, CDP_TIMEOUT_MS, timeoutForMethod } from './cdp/timeout-config.js';
7+
import { CDP_TIMEOUT_MS, timeoutForMethod } from './cdp/timeout-config.js';
88
import { sendWithTimeout as sendMsg, rejectAllPending as rejectPending, handleMessage as handleMsg } from './cdp/transport.js';
99
import { wireEventHandlers, parseNetworkHookMessage as parseNetHook } from './cdp/event-handlers.js';
10-
import { discover, discoverForList } from './cdp/discovery.js';
10+
import { discoverForList } from './cdp/discovery.js';
11+
import { helperExpr as helperExprFn, bridgeWithFallback as bridgeWithFallbackFn } from './cdp/helper-expr.js';
12+
import { autoConnect as autoConnectFn, discoverAndConnect as discoverAndConnectFn, } from './cdp/connect.js';
1113
import { handleClose as handleCloseFn, reconnect as reconnectFn, softReconnect as softReconnectFn, startBackgroundPoll as startBgPoll, stopBackgroundPoll as stopBgPoll, } from './cdp/reconnection.js';
1214
export class CDPClient {
1315
ws = null;
@@ -42,7 +44,7 @@ export class CDPClient {
4244
constructor(port) {
4345
this._port = port ?? 8081;
4446
this._consoleBuffer = new RingBuffer(200);
45-
this._networkBuffer = new RingBuffer(100);
47+
this._networkBuffer = new RingBuffer(100, { indexKey: (e) => e.id });
4648
this._logBuffer = new RingBuffer(50);
4749
}
4850
get state() { return this._state; }
@@ -66,12 +68,10 @@ export class CDPClient {
6668
return { active: this.reconnecting, lastAttempt: this._lastReconnectAttempt, attemptCount: this._reconnectAttemptCount };
6769
}
6870
helperExpr(call) {
69-
return this._bridgeDetected ? `__RN_DEV_BRIDGE__.${call}` : `__RN_AGENT.${call}`;
71+
return helperExprFn(call, this._bridgeDetected);
7072
}
7173
bridgeWithFallback(call) {
72-
return this._bridgeDetected
73-
? `(function() { var fb = false; try { var r = __RN_DEV_BRIDGE__.${call}; var p = JSON.parse(r); if (p && (p.__agent_error || p.error)) fb = true; else return r; } catch(e) { fb = true; } if (fb) return __RN_AGENT.${call}; })()`
74-
: `__RN_AGENT.${call}`;
74+
return bridgeWithFallbackFn(call, this._bridgeDetected);
7575
}
7676
async reinjectHelpers(waitTimeout) {
7777
if (!this.isConnected)
@@ -84,84 +84,25 @@ export class CDPClient {
8484
}
8585
return ok;
8686
}
87-
async autoConnect(portHint, platformFilter) {
88-
if (this._state === 'connecting' || this.reconnecting) {
89-
throw new Error('Already connecting to Metro...');
90-
}
91-
if (this.disposed) {
92-
throw new Error('Client is disposed. Create a new CDPClient instance.');
93-
}
94-
const effectivePlatform = platformFilter
95-
?? (process.env.RN_PREFERRED_PLATFORM && process.env.RN_PREFERRED_PLATFORM !== 'auto'
96-
? process.env.RN_PREFERRED_PLATFORM
97-
: undefined);
98-
return this.discoverAndConnect(portHint, effectivePlatform);
87+
async autoConnect(portHint, filtersOrPlatform) {
88+
const filters = typeof filtersOrPlatform === 'string'
89+
? { platform: filtersOrPlatform }
90+
: (filtersOrPlatform ?? {});
91+
return autoConnectFn(this.buildConnectCtx(), portHint, filters);
9992
}
10093
async listTargets(portHint) {
10194
return discoverForList(this._port, portHint);
10295
}
103-
_platformFilter;
104-
async discoverAndConnect(portHint, platformFilter) {
105-
if (this.disposed) {
106-
throw new Error('Client is disposed. Create a new CDPClient instance.');
107-
}
108-
if (portHint)
109-
this._port = portHint;
110-
if (platformFilter !== undefined)
111-
this._platformFilter = platformFilter || undefined;
112-
this._state = 'connecting';
113-
let result;
114-
try {
115-
result = await discover(this._port, this._platformFilter);
116-
}
117-
catch (err) {
118-
this._state = 'disconnected';
119-
throw err;
120-
}
121-
const { port: metroPort, targets: sorted, warning: platformFilterWarning } = result;
122-
this._port = metroPort;
123-
let connectedTarget = null;
124-
for (const candidate of sorted) {
125-
try {
126-
await this.connectToTarget(candidate);
127-
const devCheck = await this.evaluate('typeof __DEV__ !== "undefined" && __DEV__ === true');
128-
if (devCheck.value === true) {
129-
connectedTarget = candidate;
130-
break;
131-
}
132-
console.error(`CDP: target ${candidate.id} (${candidate.title}) has __DEV__=${devCheck.value}, skipping`);
133-
if (sorted.indexOf(candidate) < sorted.length - 1) {
134-
if (this.ws) {
135-
this.ws.removeAllListeners();
136-
if (this.ws.readyState === WebSocket.OPEN)
137-
this.ws.close();
138-
this.ws = null;
139-
}
140-
this._state = 'disconnected';
141-
this._helpersInjected = false;
142-
this._connectedTarget = null;
143-
continue;
144-
}
145-
console.error('CDP: no target with __DEV__=true found, using last available target');
146-
connectedTarget = candidate;
147-
}
148-
catch (err) {
149-
if (sorted.indexOf(candidate) < sorted.length - 1)
150-
continue;
151-
throw err;
152-
}
153-
}
154-
this._connectionGeneration++;
155-
logger.info('CDP', `Connected to target ${connectedTarget.id} (${connectedTarget.title}) on port ${metroPort}, generation=${this._connectionGeneration}`);
156-
const msg = `Connected to ${connectedTarget.title} on port ${metroPort}`;
157-
return platformFilterWarning ? `${msg}. WARNING: ${platformFilterWarning}` : msg;
96+
_connectFilters = {};
97+
async discoverAndConnect(portHint, filters) {
98+
return discoverAndConnectFn(this.buildConnectCtx(), portHint, filters);
15899
}
159100
async softReconnect() {
160101
return softReconnectFn(this.buildReconnectCtx());
161102
}
162103
async disconnect() {
163104
this.disposed = true;
164-
resetState(this);
105+
resetState(this.buildResettableState());
165106
clearActiveFlag();
166107
this.stopBackgroundPoll();
167108
if (this.ws) {
@@ -258,91 +199,6 @@ export class CDPClient {
258199
async send(method, params) {
259200
return this.sendWithTimeout(method, params, timeoutForMethod(method));
260201
}
261-
async connectToTarget(target, retries = 5) {
262-
let lastError = null;
263-
for (let i = 0; i < retries; i++) {
264-
if (this.disposed || this._softReconnectRequested)
265-
throw new Error('Client disposed or preempted during connection');
266-
try {
267-
await this.connectWs(target.webSocketDebuggerUrl);
268-
// D594: Early stale-target detection — quick probe before full setup
269-
try {
270-
await this.sendWithTimeout('Runtime.evaluate', {
271-
expression: '1+1',
272-
returnByValue: true,
273-
}, CDP_TIMEOUT_FAST);
274-
}
275-
catch {
276-
throw new Error('Target failed pre-flight probe (1+1) — likely a dead JS context');
277-
}
278-
this._connectedTarget = target;
279-
await this.setup();
280-
return;
281-
}
282-
catch (err) {
283-
lastError = err instanceof Error ? err : new Error(String(err));
284-
// Close stale socket if connectWs succeeded but setup failed
285-
if (this.ws) {
286-
this.ws.removeAllListeners();
287-
if (this.ws.readyState === WebSocket.OPEN || this.ws.readyState === WebSocket.CONNECTING) {
288-
this.ws.close();
289-
}
290-
this.ws = null;
291-
}
292-
// Connection refused — nothing listening, don't retry
293-
if (lastError.message.includes('refused')) {
294-
this._state = 'disconnected';
295-
throw new Error('CDP connection refused. Is Metro running and the app loaded?');
296-
}
297-
// Code 1006 and all other errors — retry (1006 is the most common transient failure)
298-
if (i < retries - 1)
299-
await sleep(2000);
300-
}
301-
}
302-
this._state = 'disconnected';
303-
const hint = lastError?.message.includes('1006')
304-
? ' Another debugger may be connected — close React Native DevTools, Flipper, or Chrome DevTools.'
305-
: '';
306-
throw new Error(`Failed to connect after ${retries} attempts.${hint}`);
307-
}
308-
connectWs(url) {
309-
return new Promise((resolve, reject) => {
310-
const ws = new WebSocket(url, {
311-
handshakeTimeout: 5000,
312-
maxPayload: 100 * 1024 * 1024,
313-
});
314-
let settled = false;
315-
ws.on('open', () => {
316-
settled = true;
317-
this.ws = ws;
318-
this._state = 'connected';
319-
resolve();
320-
});
321-
ws.on('error', (err) => {
322-
if (!settled) {
323-
settled = true;
324-
reject(err);
325-
}
326-
else {
327-
console.error('CDP WebSocket error:', err instanceof Error ? err.message : err);
328-
}
329-
});
330-
ws.on('message', (data) => {
331-
this.handleMessage(data);
332-
});
333-
ws.on('close', (code) => {
334-
if (!settled) {
335-
settled = true;
336-
reject(new Error(`WebSocket closed before connecting: ${code}`));
337-
return;
338-
}
339-
if (this.ws === ws) {
340-
this.rejectAllPending(new Error(`WebSocket closed: ${code}`));
341-
this.handleClose(code);
342-
}
343-
});
344-
});
345-
}
346202
handleMessage(data) {
347203
handleMsg(data, this.pending, this.eventHandlers, (params) => this.parseNetworkHookMessage(params));
348204
}
@@ -408,12 +264,49 @@ export class CDPClient {
408264
},
409265
rejectAllPending: (reason) => this.rejectAllPending(reason),
410266
discoverAndConnect: () => this.discoverAndConnect(),
411-
getResettableState: () => this,
267+
getResettableState: () => this.buildResettableState(),
412268
getPort: () => this._port,
413269
setBgPollTimer: (timer) => { this._bgPollTimer = timer; },
414270
getBgPollTimer: () => this._bgPollTimer,
415271
};
416272
}
273+
buildConnectCtx() {
274+
return {
275+
isDisposed: () => this.disposed,
276+
isReconnecting: () => this.reconnecting,
277+
isSoftReconnectRequested: () => this._softReconnectRequested,
278+
getState: () => this._state,
279+
setState: (s) => { this._state = s; },
280+
getPort: () => this._port,
281+
setPort: (v) => { this._port = v; },
282+
getConnectFilters: () => this._connectFilters,
283+
setConnectFilters: (v) => { this._connectFilters = v; },
284+
getWs: () => this.ws,
285+
setWs: (ws) => { this.ws = ws; },
286+
setHelpersInjected: (v) => { this._helpersInjected = v; },
287+
setConnectedTarget: (t) => { this._connectedTarget = t; },
288+
incrementConnectionGeneration: () => ++this._connectionGeneration,
289+
evaluate: (expr) => this.evaluate(expr),
290+
sendWithTimeout: (method, params, ms) => this.sendWithTimeout(method, params, ms),
291+
handleMessage: (data) => this.handleMessage(data),
292+
handleClose: (code) => this.handleClose(code),
293+
rejectAllPending: (reason) => this.rejectAllPending(reason),
294+
setup: () => this.setup(),
295+
};
296+
}
297+
buildResettableState() {
298+
return {
299+
setState: (v) => { this._state = v; },
300+
setHelpersInjected: (v) => { this._helpersInjected = v; },
301+
setBridgeDetected: (v) => { this._bridgeDetected = v; },
302+
setBridgeVersion: (v) => { this._bridgeVersion = v; },
303+
setConnectedTarget: (v) => { this._connectedTarget = v; },
304+
setLogDomainEnabled: (v) => { this._logDomainEnabled = v; },
305+
setProfilerAvailable: (v) => { this._profilerAvailable = v; },
306+
setHeapProfilerAvailable: (v) => { this._heapProfilerAvailable = v; },
307+
clearScripts: () => { this._scripts.clear(); },
308+
};
309+
}
417310
rejectAllPending(reason) {
418311
rejectPending(this.pending, reason);
419312
}

0 commit comments

Comments
 (0)