Skip to content

feat: report new URL after actions that trigger navigation #1853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rogeroliveira84 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: report new URL after actions that trigger navigation #1853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/McpPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {
import {
getNetworkMultiplierFromString,
WaitForHelper,
type WaitForEventsResult,
} from './WaitForHelper.js';

/**
Expand Down Expand Up @@ -113,7 +114,7 @@ export class McpPage implements ContextPage {
waitForEventsAfterAction(
action: () => Promise<unknown>,
options?: {timeout?: number},
): Promise<void> {
): Promise<WaitForEventsResult> {
const helper = this.createWaitForHelper(
this.cpuThrottlingRate,
getNetworkMultiplierFromString(this.networkConditions),
Expand Down
14 changes: 13 additions & 1 deletion src/WaitForHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,12 @@ export class WaitForHelper {
async waitForEventsAfterAction(
action: () => Promise<unknown>,
options?: {timeout?: number},
): Promise<void> {
): Promise<WaitForEventsResult> {
let navigated = false;
const navigationFinished = this.waitForNavigationStarted()
.then(navigationStated => {
if (navigationStated) {
navigated = true;
return this.#page.waitForNavigation({
timeout: options?.timeout ?? this.#navigationTimeout,
signal: this.#abortController.signal,
Expand Down Expand Up @@ -159,9 +161,19 @@ export class WaitForHelper {
} finally {
this.#abortController.abort();
}

return {navigated};
Copy link
Copy Markdown
Collaborator

@Lightning00Blade Lightning00Blade Apr 16, 2026

Choose a reason for hiding this comment

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

I think we can simplify this can just provide the URL here if it was changed.

Suggested change
return {navigated};
return {
...(navigated ? {url: this.#page.url()} : {})
};

Copy link
Copy Markdown
Collaborator

@Lightning00Blade Lightning00Blade Apr 17, 2026

Choose a reason for hiding this comment

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

Actually let's call it navigatedToUrl to be more explicit.

}
}

export interface WaitForEventsResult {
/**
* Whether a cross-document navigation started and finished during the
* action. Same-document (history API) navigations are not reported.
Copy link
Copy Markdown
Collaborator

@Lightning00Blade Lightning00Blade Apr 16, 2026

Choose a reason for hiding this comment

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

This is wrong we use Puppeteer waitForNavigation and that does support same-document navigation
https://pptr.dev/api/puppeteer.page.waitfornavigation

*/
navigated: boolean;
Copy link
Copy Markdown
Collaborator

@Lightning00Blade Lightning00Blade Apr 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

With the above change this will become

Suggested change
navigated: boolean;
navigatedToUrl?: string;

}

export function getNetworkMultiplierFromString(
condition: string | null,
): number {
Expand Down
3 changes: 2 additions & 1 deletion src/tools/ToolDefinition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {
} from '../types.js';
import type {InstalledExtension} from '../utils/ExtensionRegistry.js';
import type {PaginationOptions} from '../utils/types.js';
import type {WaitForEventsResult} from '../WaitForHelper.js';

import type {ToolCategory} from './categories.js';
import type {
Expand Down Expand Up @@ -211,7 +212,7 @@ export type ContextPage = Readonly<{
waitForEventsAfterAction(
action: () => Promise<unknown>,
options?: {timeout?: number},
): Promise<void>;
): Promise<WaitForEventsResult>;
getInPageTools(): ToolGroup<InPageToolDefinition> | undefined;
}>;

Expand Down
38 changes: 29 additions & 9 deletions src/tools/input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import {zod} from '../third_party/index.js';
import type {ElementHandle, KeyInput} from '../third_party/index.js';
import type {TextSnapshotNode} from '../types.js';
import {parseKey} from '../utils/keyboard.js';
import type {WaitForEventsResult} from '../WaitForHelper.js';

import {ToolCategory} from './categories.js';
import type {ContextPage} from './ToolDefinition.js';
import type {ContextPage, Response} from './ToolDefinition.js';
import {definePageTool} from './ToolDefinition.js';

const dblClickSchema = zod
Expand Down Expand Up @@ -42,6 +43,16 @@ function handleActionError(error: unknown, uid: string) {
);
}

function appendNavigationIfAny(
Copy link
Copy Markdown
Collaborator

@Lightning00Blade Lightning00Blade Apr 16, 2026

Choose a reason for hiding this comment

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

Let's rename it to appendWaitForResult to match the interface we provide.
We can also move this into the WaitForHelper file as we can use in other places as well.

page: ContextPage,
response: Response,
result: WaitForEventsResult,
) {
if (result.navigated) {
response.appendResponseLine(`Page navigated to ${page.pptrPage.url()}.`);
}
}

export const click = definePageTool({
name: 'click',
description: `Clicks on the provided element`,
Expand All @@ -62,7 +73,7 @@ export const click = definePageTool({
const uid = request.params.uid;
const handle = await request.page.getElementByUid(uid);
try {
await request.page.waitForEventsAfterAction(async () => {
const result = await request.page.waitForEventsAfterAction(async () => {
await handle.asLocator().click({
count: request.params.dblClick ? 2 : 1,
});
Expand All @@ -72,6 +83,7 @@ export const click = definePageTool({
? `Successfully double clicked on the element`
: `Successfully clicked on the element`,
);
appendNavigationIfAny(request.page, response, result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
Expand Down Expand Up @@ -99,7 +111,7 @@ export const clickAt = definePageTool({
},
handler: async (request, response) => {
const page = request.page;
await page.waitForEventsAfterAction(async () => {
const result = await page.waitForEventsAfterAction(async () => {
await page.pptrPage.mouse.click(request.params.x, request.params.y, {
clickCount: request.params.dblClick ? 2 : 1,
});
Expand All @@ -109,6 +121,7 @@ export const clickAt = definePageTool({
? `Successfully double clicked at the coordinates`
: `Successfully clicked at the coordinates`,
);
appendNavigationIfAny(page, response, result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
Expand All @@ -134,10 +147,11 @@ export const hover = definePageTool({
const uid = request.params.uid;
const handle = await request.page.getElementByUid(uid);
try {
await request.page.waitForEventsAfterAction(async () => {
const result = await request.page.waitForEventsAfterAction(async () => {
await handle.asLocator().hover();
});
response.appendResponseLine(`Successfully hovered over the element`);
appendNavigationIfAny(request.page, response, result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
Expand Down Expand Up @@ -235,7 +249,7 @@ export const fill = definePageTool({
},
handler: async (request, response, context) => {
const page = request.page;
await page.waitForEventsAfterAction(async () => {
const result = await page.waitForEventsAfterAction(async () => {
await fillFormElement(
request.params.uid,
request.params.value,
Expand All @@ -244,6 +258,7 @@ export const fill = definePageTool({
);
});
response.appendResponseLine(`Successfully filled out the element`);
appendNavigationIfAny(page, response, result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
Expand All @@ -263,7 +278,7 @@ export const typeText = definePageTool({
},
handler: async (request, response) => {
const page = request.page;
await page.waitForEventsAfterAction(async () => {
const result = await page.waitForEventsAfterAction(async () => {
await page.pptrPage.keyboard.type(request.params.text);
if (request.params.submitKey) {
await page.pptrPage.keyboard.press(
Expand All @@ -274,6 +289,7 @@ export const typeText = definePageTool({
response.appendResponseLine(
`Typed text "${request.params.text}${request.params.submitKey ? ` + ${request.params.submitKey}` : ''}"`,
);
appendNavigationIfAny(page, response, result);
},
});

Expand All @@ -295,12 +311,13 @@ export const drag = definePageTool({
);
const toHandle = await request.page.getElementByUid(request.params.to_uid);
try {
await request.page.waitForEventsAfterAction(async () => {
const result = await request.page.waitForEventsAfterAction(async () => {
await fromHandle.drag(toHandle);
await new Promise(resolve => setTimeout(resolve, 50));
await toHandle.drop(fromHandle);
});
response.appendResponseLine(`Successfully dragged an element`);
appendNavigationIfAny(request.page, response, result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
Expand Down Expand Up @@ -332,8 +349,9 @@ export const fillForm = definePageTool({
},
handler: async (request, response, context) => {
const page = request.page;
let lastResult: WaitForEventsResult = {navigated: false};
for (const element of request.params.elements) {
await page.waitForEventsAfterAction(async () => {
lastResult = await page.waitForEventsAfterAction(async () => {
await fillFormElement(
element.uid,
element.value,
Expand All @@ -343,6 +361,7 @@ export const fillForm = definePageTool({
});
}
response.appendResponseLine(`Successfully filled out the form`);
appendNavigationIfAny(page, response, lastResult);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
Expand Down Expand Up @@ -419,7 +438,7 @@ export const pressKey = definePageTool({
const tokens = parseKey(request.params.key);
const [key, ...modifiers] = tokens;

await page.waitForEventsAfterAction(async () => {
const result = await page.waitForEventsAfterAction(async () => {
for (const modifier of modifiers) {
await page.pptrPage.keyboard.down(modifier);
}
Expand All @@ -432,6 +451,7 @@ export const pressKey = definePageTool({
response.appendResponseLine(
`Successfully pressed key: ${request.params.key}`,
);
appendNavigationIfAny(page, response, result);
if (request.params.includeSnapshot) {
response.includeSnapshot();
}
Expand Down
19 changes: 13 additions & 6 deletions src/tools/script.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,15 @@ Example with arguments: \`(el) => {
}

const worker = await getWebWorker(context, serviceWorkerId);
await context
.getSelectedMcpPage()
.waitForEventsAfterAction(async () => {
await performEvaluation(worker, fnString, [], response);
});
const selectedPage = context.getSelectedMcpPage();
const result = await selectedPage.waitForEventsAfterAction(async () => {
await performEvaluation(worker, fnString, [], response);
});
if (result.navigated) {
Copy link
Copy Markdown
Collaborator

@Lightning00Blade Lightning00Blade Apr 16, 2026

Choose a reason for hiding this comment

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

This should use the helper.

response.appendResponseLine(
`Page navigated to ${selectedPage.pptrPage.url()}.`,
);
}
return;
}

Expand All @@ -101,9 +105,12 @@ Example with arguments: \`(el) => {

const evaluatable = await getPageOrFrame(page, frames);

await mcpPage.waitForEventsAfterAction(async () => {
const result = await mcpPage.waitForEventsAfterAction(async () => {
await performEvaluation(evaluatable, fnString, args, response);
});
if (result.navigated) {
Copy link
Copy Markdown
Collaborator

@Lightning00Blade Lightning00Blade Apr 16, 2026

Choose a reason for hiding this comment

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

Same here.

response.appendResponseLine(`Page navigated to ${page.url()}.`);
}
} finally {
void Promise.allSettled(args.map(arg => arg.dispose()));
}
Expand Down
57 changes: 57 additions & 0 deletions tests/tools/input.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,63 @@ describe('input', () => {
});
});

it('reports the new URL when click triggers a navigation', async () => {
server.addHtmlRoute(
'/start',
html`<a href="/after-click">Navigate page</a>`,
);
server.addHtmlRoute('/after-click', html`<main>arrived</main>`);

await withMcpContext(async (response, context) => {
const page = context.getSelectedPptrPage();
await page.goto(server.getRoute('/start'));
await context.createTextSnapshot(context.getSelectedMcpPage());
await click.handler(
{
params: {
uid: '1_1',
},
page: context.getSelectedMcpPage(),
},
response,
context,
);
const expectedUrl = server.getRoute('/after-click');
assert.ok(
response.responseLines.some(
line => line === `Page navigated to ${expectedUrl}.`,
),
`Expected response to mention navigation to ${expectedUrl}, got: ${response.responseLines.join(' | ')}`,
);
});
});

it('does not report navigation when click does not navigate', async () => {
await withMcpContext(async (response, context) => {
const page = context.getSelectedPptrPage();
await page.setContent(
html`<button onclick="this.innerText = 'clicked';">test</button>`,
);
await context.createTextSnapshot(context.getSelectedMcpPage());
await click.handler(
{
params: {
uid: '1_1',
},
page: context.getSelectedMcpPage(),
},
response,
context,
);
assert.ok(
!response.responseLines.some(line =>
line.startsWith('Page navigated to '),
),
`Did not expect a navigation line, got: ${response.responseLines.join(' | ')}`,
);
});
});

it('waits for stable DOM', async () => {
server.addHtmlRoute(
'/unstable',
Expand Down