diff --git a/.changeset/bound-probe-teardown-redaction.md b/.changeset/bound-probe-teardown-redaction.md new file mode 100644 index 000000000..d000c9f94 --- /dev/null +++ b/.changeset/bound-probe-teardown-redaction.md @@ -0,0 +1,7 @@ +--- +"agent-bundle": patch +--- + +Keep timed-out MCP probes responsive when transport teardown stalls, while +continuing the transport close path that terminates stdio children. Redact +absolute paths that follow common key-value and list separators. diff --git a/packages/agent-bundle/src/dev/playground/mcp-probe-service.ts b/packages/agent-bundle/src/dev/playground/mcp-probe-service.ts index 11cb66391..28880f129 100644 --- a/packages/agent-bundle/src/dev/playground/mcp-probe-service.ts +++ b/packages/agent-bundle/src/dev/playground/mcp-probe-service.ts @@ -45,6 +45,7 @@ export const mcpProbeFailureTextLimit = 2_048; const mcpProbeCapabilityLimit = 32; const mcpProbeNameTextLimit = 256; +const mcpProbeTeardownWaitMs = 50; const safeCapabilityName = /^[A-Za-z][A-Za-z0-9_-]{0,63}$/u; const connectionErrorCodes = new Set([ 'EACCES', @@ -126,7 +127,7 @@ const bundlePathPattern = (bundleRoot: string): RegExp => { }; const hasAbsolutePath = (value: string): boolean => - /(?:file:|(?:^|[\s"'([{])\/[^\s,;{}()[\]<>"']+|(?:^|[\s"'([{])[A-Za-z]:[\\/]|\\\\)/u.test(value); + /(?:file:|(?:^|[\s"'([{=,:])\/[^\s,;{}()[\]<>"']+|(?:^|[\s"'([{=,:])[A-Za-z]:[\\/]|\\\\)/u.test(value); /** * Probe text follows the Dev Log browser-wire precedent without coupling this @@ -465,7 +466,19 @@ export class McpProbeService { }); return report; } finally { - await Promise.allSettled([client.close(), transport.close()]); + let timer: NodeJS.Timeout | undefined; + // Keep transport teardown running through its TERM/KILL path without + // allowing a stalled close to extend the probe's total time budget. + const teardown = Promise.allSettled([client.close(), transport.close()]); + const teardownWait = new Promise((resolvePromise) => { + timer = setTimeout(resolvePromise, mcpProbeTeardownWaitMs); + timer.unref(); + }); + try { + await Promise.race([teardown, teardownWait]); + } finally { + if (timer !== undefined) clearTimeout(timer); + } } } diff --git a/packages/agent-bundle/tests/mcp-probe-service.test.ts b/packages/agent-bundle/tests/mcp-probe-service.test.ts index a656303ad..832484ac4 100644 --- a/packages/agent-bundle/tests/mcp-probe-service.test.ts +++ b/packages/agent-bundle/tests/mcp-probe-service.test.ts @@ -127,6 +127,41 @@ it('maps a bounded, frozen successful probe snapshot and redacted launch', async } }); +it('redacts absolute paths after key-value and list separators', async () => { + const root = await createBundle(); + try { + const service = serviceFor(root, { + createClient: () => client({ + getInstructions: () => 'config=/home/alice/private.json', + listTools: async () => ({ + tools: [ + { + description: String.raw`cwd:C:\Users\alice\private`, + inputSchema: { type: 'object' as const }, + name: 'windows-path', + }, + { + description: 'paths,/var/private/config.json', + inputSchema: { type: 'object' as const }, + name: 'list-path', + }, + ], + }), + }), + }); + + const report = await service.probe({ host: 'claude', serverName: 'timeline' }); + + expect(report.snapshot?.instructions).toBe('[REDACTED]'); + expect(report.snapshot?.tools.map((tool) => tool.description)).toEqual([ + '[REDACTED]', + '[REDACTED]', + ]); + } finally { + await rm(root, { force: true, recursive: true }); + } +}); + it('truncates server instructions to the named text budget', async () => { const root = await createBundle(); try { @@ -194,6 +229,48 @@ it('times out within the total budget and destroys the transport', async () => { } }); +it('returns a timed-out report without awaiting stalled teardown', async () => { + const root = await createBundle(); + let clientCloses = 0; + let transportCloses = 0; + let guard: NodeJS.Timeout | undefined; + try { + const stalledClose = async (): Promise => + new Promise((resolvePromise) => setTimeout(resolvePromise, 250)); + const service = serviceFor(root, { + createClient: () => client({ + close: async () => { + clientCloses += 1; + await stalledClose(); + }, + connect: () => new Promise(() => undefined), + }), + createStdioTransport: () => transport(async () => { + transportCloses += 1; + await stalledClose(); + }), + timeoutMs: 10, + }); + + const report = await Promise.race([ + service.probe({ host: 'claude', serverName: 'timeline' }), + new Promise((_resolve, reject) => { + guard = setTimeout( + () => reject(new Error('The timed-out probe remained blocked on teardown.')), + 150, + ); + }), + ]); + + expect(report.status).toBe('timed-out'); + expect(clientCloses).toBe(1); + expect(transportCloses).toBeGreaterThan(0); + } finally { + if (guard !== undefined) clearTimeout(guard); + await rm(root, { force: true, recursive: true }); + } +}); + it('coalesces only identical in-flight probes and clears them after settlement', async () => { const root = await createBundle(); const connectStarted = Promise.withResolvers();