Skip to content

Commit 7ee3a2f

Browse files
committed
fix(mcp): tighten clear-recordings target gating and validate page upfront
Enforce actions, agent.tools, and inspector settings together before exposing or clearing each target, and validate router page presence upfront before clearing recordings. Also refresh the committed extension bundle for the updated devtools build.
1 parent 5980041 commit 7ee3a2f

6 files changed

Lines changed: 101 additions & 17 deletions

File tree

extension/ui/assets/browser-agent-rpc-BXhoSh1z-C0NC0d_x.js renamed to extension/ui/assets/browser-agent-rpc-BXhoSh1z-DG2zOM1d.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎extension/ui/index.html‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/devtools/src/__tests__/clear-recordings.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,4 +259,52 @@ describe('clear-recordings agent tool', () => {
259259
});
260260
expect(tools).not.toContain('clear-recordings');
261261
});
262+
263+
it('drops clear-recordings when no target has both actions and agent tools enabled', async () => {
264+
const { tools } = await boot({
265+
actions: { http: true, router: false, analog: false },
266+
agent: { tools: { http: false, router: true, analog: true } },
267+
});
268+
expect(tools).not.toContain('clear-recordings');
269+
});
270+
271+
it('blocks target when agent.tools is false even if action is true', async () => {
272+
const { call } = await boot({
273+
actions: { http: true, router: true },
274+
agent: { tools: { http: false, router: true } },
275+
});
276+
await expect(call('clear-recordings', { target: 'http' })).rejects.toThrow(
277+
'HTTP recording is disabled in the devtools config (agent.tools.http is turned off).',
278+
);
279+
});
280+
281+
it('returns unknown page text when requested router page is absent', async () => {
282+
const { call } = await boot();
283+
const result = await call('clear-recordings', { target: 'router', page: 'missing-page-id' });
284+
expect(result).toContain('No page `missing-page-id` is reporting a router report.');
285+
});
286+
287+
it('validates router page upfront and preserves other recordings when target is all and page is unknown', async () => {
288+
const { push, call } = await boot();
289+
await push(
290+
'push-http',
291+
httpReport('page-1', [
292+
{
293+
id: 'call-keep',
294+
url: '/api/preserve',
295+
method: 'GET',
296+
status: 200,
297+
durationMs: 10,
298+
side: 'client',
299+
},
300+
]),
301+
);
302+
303+
const result = await call('clear-recordings', { target: 'all', page: 'non-existent-page' });
304+
expect(result).toContain('No page `non-existent-page` is reporting a router report.');
305+
306+
// HTTP calls must not have been cleared
307+
const httpList = await call('list-http-calls');
308+
expect(httpList).toContain('/api/preserve');
309+
});
262310
});

‎packages/devtools/src/config.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -680,10 +680,10 @@ export function agentAllowed(
680680
return false;
681681
}
682682
if (name === 'clear-recordings') {
683-
return (
684-
(config.agent.tools.http && config.inspectors.http) ||
685-
(config.agent.tools.router && config.inspectors.router) ||
686-
(config.agent.tools.analog && config.inspectors.analog)
683+
return Boolean(
684+
(config.agent.tools.http && config.inspectors.http && config.actions.http) ||
685+
(config.agent.tools.router && config.inspectors.router && config.actions.router) ||
686+
(config.agent.tools.analog && config.inspectors.analog && config.actions.analog),
687687
);
688688
}
689689
const inspector = agentInspector(entry.id);

‎packages/devtools/src/devframe.ts‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,25 +1183,51 @@ const pangular = defineDevframe({
11831183
throw new Error('Provide a valid target: "http", "router", "analog", or "all".');
11841184
}
11851185

1186+
const pageId = pageArgument(args as Record<string, unknown> | undefined);
1187+
if (pageId && (target === 'router' || target === 'all') && !routerPages.has(pageId)) {
1188+
return {
1189+
markdown: unknownPageText(pageId, byRecency(routerPages.values()), 'a router report'),
1190+
};
1191+
}
1192+
11861193
const cleared: string[] = [];
11871194
const skipped: string[] = [];
11881195

1196+
const httpEligible =
1197+
config.actions.http && config.agent.tools.http && config.inspectors.http;
11891198
if (target === 'http' || target === 'all') {
1190-
if (!config.actions.http) {
1191-
if (target === 'http') throw new Error(actionBlockedMessage('http'));
1192-
skipped.push('HTTP calls (actions.http is turned off)');
1199+
if (!httpEligible) {
1200+
const reason = !config.actions.http
1201+
? 'actions.http is turned off'
1202+
: !config.agent.tools.http
1203+
? 'agent.tools.http is turned off'
1204+
: 'inspectors.http is turned off';
1205+
if (target === 'http') {
1206+
if (!config.actions.http) throw new Error(actionBlockedMessage('http'));
1207+
throw new Error(`HTTP recording is disabled in the devtools config (${reason}).`);
1208+
}
1209+
skipped.push(`HTTP calls (${reason})`);
11931210
} else {
11941211
clearHttpCalls();
11951212
cleared.push('recorded HTTP calls');
11961213
}
11971214
}
11981215

1216+
const routerEligible =
1217+
config.actions.router && config.agent.tools.router && config.inspectors.router;
11991218
if (target === 'router' || target === 'all') {
1200-
if (!config.actions.router) {
1201-
if (target === 'router') throw new Error(actionBlockedMessage('router'));
1202-
skipped.push('router navigations (actions.router is turned off)');
1219+
if (!routerEligible) {
1220+
const reason = !config.actions.router
1221+
? 'actions.router is turned off'
1222+
: !config.agent.tools.router
1223+
? 'agent.tools.router is turned off'
1224+
: 'inspectors.router is turned off';
1225+
if (target === 'router') {
1226+
if (!config.actions.router) throw new Error(actionBlockedMessage('router'));
1227+
throw new Error(`Router recording is disabled in the devtools config (${reason}).`);
1228+
}
1229+
skipped.push(`router navigations (${reason})`);
12031230
} else {
1204-
const pageId = pageArgument(args as Record<string, unknown> | undefined);
12051231
if (pageId) {
12061232
if (clearRouterNavigations(routerPages, pageId)) {
12071233
applyRouter(currentRouter(routerPages));
@@ -1223,10 +1249,20 @@ const pangular = defineDevframe({
12231249
}
12241250
}
12251251

1252+
const analogEligible =
1253+
config.actions.analog && config.agent.tools.analog && config.inspectors.analog;
12261254
if (target === 'analog' || target === 'all') {
1227-
if (!config.actions.analog) {
1228-
if (target === 'analog') throw new Error(actionBlockedMessage('analog'));
1229-
skipped.push('Analog calls (actions.analog is turned off)');
1255+
if (!analogEligible) {
1256+
const reason = !config.actions.analog
1257+
? 'actions.analog is turned off'
1258+
: !config.agent.tools.analog
1259+
? 'agent.tools.analog is turned off'
1260+
: 'inspectors.analog is turned off';
1261+
if (target === 'analog') {
1262+
if (!config.actions.analog) throw new Error(actionBlockedMessage('analog'));
1263+
throw new Error(`Analog recording is disabled in the devtools config (${reason}).`);
1264+
}
1265+
skipped.push(`Analog calls (${reason})`);
12301266
} else {
12311267
clearCalls();
12321268
cleared.push('Analog server calls');

0 commit comments

Comments
 (0)