Skip to content

Commit a6cd2cd

Browse files
authored
fix: re-use node ids across snapshots (#814)
previously, Chrome DevTools MCP was strictly requiring the client to use the latest snapshot. This change updates the behavior to be more lenient. Previously issued uids will be recognized if the underlying backend Id (loaderId + backendNodeId) still exists. Related #726
1 parent 56b9f76 commit a6cd2cd

File tree

3 files changed

+136
-22
lines changed

3 files changed

+136
-22
lines changed

src/McpContext.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {WaitForHelper} from './WaitForHelper.js';
4040
export interface TextSnapshotNode extends SerializedAXNode {
4141
id: string;
4242
backendNodeId?: number;
43+
loaderId?: string;
4344
children: TextSnapshotNode[];
4445
}
4546

@@ -129,6 +130,8 @@ export class McpContext implements Context {
129130
#locatorClass: typeof Locator;
130131
#options: McpContextOptions;
131132

133+
#uniqueBackendNodeIdToMcpId = new Map<string, string>();
134+
132135
private constructor(
133136
browser: Browser,
134137
logger: Debugger,
@@ -440,14 +443,6 @@ export class McpContext implements Context {
440443
`No snapshot found. Use ${takeSnapshot.name} to capture one.`,
441444
);
442445
}
443-
const [snapshotId] = uid.split('_');
444-
445-
if (this.#textSnapshot.snapshotId !== snapshotId) {
446-
throw new Error(
447-
'This uid is coming from a stale snapshot. Call take_snapshot to get a fresh snapshot.',
448-
);
449-
}
450-
451446
const node = this.#textSnapshot?.idToNode.get(uid);
452447
if (!node) {
453448
throw new Error('No such element found in the snapshot');
@@ -589,10 +584,24 @@ export class McpContext implements Context {
589584
// will be used for the tree serialization and mapping ids back to nodes.
590585
let idCounter = 0;
591586
const idToNode = new Map<string, TextSnapshotNode>();
587+
const seenUniqueIds = new Set<string>();
592588
const assignIds = (node: SerializedAXNode): TextSnapshotNode => {
589+
let id = '';
590+
// @ts-expect-error untyped loaderId & backendNodeId.
591+
const uniqueBackendId = `${node.loaderId}_${node.backendNodeId}`;
592+
if (this.#uniqueBackendNodeIdToMcpId.has(uniqueBackendId)) {
593+
// Re-use MCP exposed ID if the uniqueId is the same.
594+
id = this.#uniqueBackendNodeIdToMcpId.get(uniqueBackendId)!;
595+
} else {
596+
// Only generate a new ID if we have not seen the node before.
597+
id = `${snapshotId}_${idCounter++}`;
598+
this.#uniqueBackendNodeIdToMcpId.set(uniqueBackendId, id);
599+
}
600+
seenUniqueIds.add(uniqueBackendId);
601+
593602
const nodeWithId: TextSnapshotNode = {
594603
...node,
595-
id: `${snapshotId}_${idCounter++}`,
604+
id,
596605
children: node.children
597606
? node.children.map(child => assignIds(child))
598607
: [],
@@ -626,6 +635,13 @@ export class McpContext implements Context {
626635
data?.cdpBackendNodeId,
627636
);
628637
}
638+
639+
// Clean up unique IDs that we did not see anymore.
640+
for (const key of this.#uniqueBackendNodeIdToMcpId.keys()) {
641+
if (!seenUniqueIds.has(key)) {
642+
this.#uniqueBackendNodeIdToMcpId.delete(key);
643+
}
644+
}
629645
}
630646

631647
getTextSnapshot(): TextSnapshot | null {

tests/McpContext.test.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,16 @@ describe('McpContext', () => {
2020
await withMcpContext(async (_response, context) => {
2121
const page = context.getSelectedPage();
2222
await page.setContent(
23-
html`<button>Click me</button
24-
><input
23+
html`<button>Click me</button>
24+
<input
2525
type="text"
2626
value="Input"
2727
/>`,
2828
);
2929
await context.createTextSnapshot();
3030
assert.ok(await context.getElementByUid('1_1'));
3131
await context.createTextSnapshot();
32-
try {
33-
await context.getElementByUid('1_1');
34-
assert.fail('not reached');
35-
} catch (err) {
36-
assert.strict(
37-
err.message,
38-
'This uid is coming from a stale snapshot. Call take_snapshot to get a fresh snapshot',
39-
);
40-
}
32+
await context.getElementByUid('1_1');
4133
});
4234
});
4335

tests/McpResponse.test.ts

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
traceResultIsSuccess,
1717
} from '../src/trace-processing/parse.js';
1818

19+
import {serverHooks} from './server.js';
1920
import {loadTraceAsBuffer} from './trace-processing/fixtures/load.js';
2021
import {
2122
getImageContent,
@@ -63,8 +64,8 @@ describe('McpResponse', () => {
6364
await withMcpContext(async (response, context) => {
6465
const page = context.getSelectedPage();
6566
await page.setContent(
66-
html`<button>Click me</button
67-
><input
67+
html`<button>Click me</button>
68+
<input
6869
type="text"
6970
value="Input"
7071
/>`,
@@ -145,6 +146,111 @@ describe('McpResponse', () => {
145146
}
146147
});
147148

149+
it('preserves mapping ids across multiple snapshots', async () => {
150+
await withMcpContext(async (response, context) => {
151+
const page = context.getSelectedPage();
152+
await page.setContent(html`
153+
<div>
154+
<button id="btn1">Button 1</button>
155+
<span id="span1">Span 1</span>
156+
</div>
157+
`);
158+
response.includeSnapshot();
159+
// First snapshot
160+
const res1 = await response.handle('test', context);
161+
const text1 = getTextContent(res1.content[0]);
162+
const btn1IdMatch = text1.match(/uid=(\S+) .*Button 1/);
163+
const span1IdMatch = text1.match(/uid=(\S+) .*Span 1/);
164+
165+
assert.ok(btn1IdMatch, 'Button 1 ID not found in first snapshot');
166+
assert.ok(span1IdMatch, 'Span 1 ID not found in first snapshot');
167+
168+
const btn1Id = btn1IdMatch[1];
169+
const span1Id = span1IdMatch[1];
170+
171+
// Modify page: add a new element before the others to potentially shift indices if not stable
172+
await page.evaluate(() => {
173+
const newBtn = document.createElement('button');
174+
newBtn.textContent = 'Button 2';
175+
document.body.prepend(newBtn);
176+
});
177+
178+
// Second snapshot
179+
const res2 = await response.handle('test', context);
180+
const text2 = getTextContent(res2.content[0]);
181+
182+
const btn1IdMatch2 = text2.match(/uid=(\S+) .*Button 1/);
183+
const span1IdMatch2 = text2.match(/uid=(\S+) .*Span 1/);
184+
const btn2IdMatch = text2.match(/uid=(\S+) .*Button 2/);
185+
186+
assert.ok(btn1IdMatch2, 'Button 1 ID not found in second snapshot');
187+
assert.ok(span1IdMatch2, 'Span 1 ID not found in second snapshot');
188+
assert.ok(btn2IdMatch, 'Button 2 ID not found in second snapshot');
189+
190+
assert.strictEqual(
191+
btn1IdMatch2[1],
192+
btn1Id,
193+
'Button 1 ID changed between snapshots',
194+
);
195+
assert.strictEqual(
196+
span1IdMatch2[1],
197+
span1Id,
198+
'Span 1 ID changed between snapshots',
199+
);
200+
assert.notStrictEqual(
201+
btn2IdMatch[1],
202+
btn1Id,
203+
'Button 2 ID collides with Button 1',
204+
);
205+
assert.notStrictEqual(
206+
btn2IdMatch[1],
207+
btn1Id,
208+
'Button 2 ID collides with Button 1',
209+
);
210+
});
211+
});
212+
213+
describe('navigation', () => {
214+
const server = serverHooks();
215+
216+
it('resets ids after navigation', async () => {
217+
await withMcpContext(async (response, context) => {
218+
server.addHtmlRoute(
219+
'/page.html',
220+
html`
221+
<div>
222+
<button id="btn1">Button 1</button>
223+
</div>
224+
`,
225+
);
226+
const page = context.getSelectedPage();
227+
await page.goto(server.getRoute('/page.html'));
228+
229+
response.includeSnapshot();
230+
const res1 = await response.handle('test', context);
231+
const text1 = getTextContent(res1.content[0]);
232+
const btn1IdMatch = text1.match(/uid=(\S+) .*Button 1/);
233+
assert.ok(btn1IdMatch, 'Button 1 ID not found in first snapshot');
234+
const btn1Id = btn1IdMatch[1];
235+
236+
// Navigate to the same page again (or meaningful navigation)
237+
await page.goto(server.getRoute('/page.html'));
238+
239+
const res2 = await response.handle('test', context);
240+
const text2 = getTextContent(res2.content[0]);
241+
const btn1IdMatch2 = text2.match(/uid=(\S+) .*Button 1/);
242+
assert.ok(btn1IdMatch2, 'Button 1 ID not found in second snapshot');
243+
const btn1Id2 = btn1IdMatch2[1];
244+
245+
assert.notStrictEqual(
246+
btn1Id2,
247+
btn1Id,
248+
'ID should reset after navigation',
249+
);
250+
});
251+
});
252+
});
253+
148254
it('adds throttling setting when it is not null', async t => {
149255
await withMcpContext(async (response, context) => {
150256
context.setNetworkConditions('Slow 3G');

0 commit comments

Comments
 (0)