Skip to content

Commit 83d6f8f

Browse files
committed
fix(worker): gate mcp discovery readiness diagnostics
Signed-off-by: zebbern <185730623+zebbern@users.noreply.github.com>
1 parent 6227377 commit 83d6f8f

2 files changed

Lines changed: 112 additions & 3 deletions

File tree

worker/src/temporal/activities/__tests__/mcp-discovery.activity.test.ts

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,87 @@ import { afterEach, beforeAll, beforeEach, describe, expect, mock, test, vi } fr
22

33
const redisSetex = vi.fn(async (_key: string, _ttlSeconds: number, _value: string) => 'OK');
44
const redisGet = vi.fn(async (_key: string): Promise<string | null> => null);
5+
const mockHeartbeat = vi.fn();
6+
const mockStartMcpDockerServer = vi.fn(async () => ({
7+
endpoint: 'http://localhost:4100/mcp',
8+
containerId: 'mcp-container-1',
9+
}));
10+
const mockExecFile = vi.fn(
11+
(
12+
_file: string,
13+
_args: string[],
14+
callback?: (error: Error | null, stdout: string, stderr: string) => void,
15+
) => {
16+
callback?.(null, '', '');
17+
},
18+
);
19+
const mockSpawn = vi.fn();
520

621
class MockRedis {
722
setex = redisSetex;
823
get = redisGet;
924
}
1025

26+
class MockApplicationFailure extends Error {
27+
type: string;
28+
nonRetryable: boolean;
29+
details?: unknown[];
30+
31+
constructor(message: string, type: string, nonRetryable: boolean, details?: unknown[]) {
32+
super(message);
33+
this.name = 'ApplicationFailure';
34+
this.type = type;
35+
this.nonRetryable = nonRetryable;
36+
this.details = details;
37+
}
38+
39+
static nonRetryable(message: string, type: string, details?: unknown[]) {
40+
return new MockApplicationFailure(message, type, true, details);
41+
}
42+
43+
static retryable(message: string, type: string, details?: unknown[]) {
44+
return new MockApplicationFailure(message, type, false, details);
45+
}
46+
}
47+
1148
mock.module('ioredis', () => ({
1249
default: MockRedis,
1350
}));
1451

52+
mock.module('@temporalio/activity', () => ({
53+
ApplicationFailure: MockApplicationFailure,
54+
Context: {
55+
current: () => ({
56+
heartbeat: mockHeartbeat,
57+
}),
58+
},
59+
}));
60+
61+
mock.module('../../../components/core/mcp-runtime', () => ({
62+
startMcpDockerServer: mockStartMcpDockerServer,
63+
}));
64+
65+
mock.module('node:child_process', () => ({
66+
execFile: mockExecFile,
67+
spawn: mockSpawn,
68+
}));
69+
1570
const originalDebugWorkflow = process.env.SENTRIS_DEBUG_WORKFLOW;
71+
const originalFetch = globalThis.fetch;
1672
let cacheDiscoveryResultActivity: typeof import('../mcp-discovery.activity').cacheDiscoveryResultActivity;
73+
let discoverMcpGroupToolsActivity: typeof import('../mcp-discovery.activity').discoverMcpGroupToolsActivity;
74+
75+
function createJsonResponse(body: unknown): Response {
76+
return {
77+
ok: true,
78+
json: vi.fn(async () => body),
79+
} as unknown as Response;
80+
}
1781

1882
describe('MCP discovery activity diagnostics', () => {
1983
beforeAll(async () => {
20-
({ cacheDiscoveryResultActivity } = await import('../mcp-discovery.activity'));
84+
({ cacheDiscoveryResultActivity, discoverMcpGroupToolsActivity } =
85+
await import('../mcp-discovery.activity'));
2186
});
2287

2388
beforeEach(() => {
@@ -26,6 +91,7 @@ describe('MCP discovery activity diagnostics', () => {
2691
});
2792

2893
afterEach(() => {
94+
globalThis.fetch = originalFetch;
2995
if (originalDebugWorkflow === undefined) {
3096
delete process.env.SENTRIS_DEBUG_WORKFLOW;
3197
} else {
@@ -57,4 +123,47 @@ describe('MCP discovery activity diagnostics', () => {
57123
consoleLogSpy.mockRestore();
58124
}
59125
});
126+
127+
test('discoverMcpGroupToolsActivity does not mirror successful stdio readiness diagnostics to console.log by default', async () => {
128+
globalThis.fetch = vi.fn(async (url: Parameters<typeof fetch>[0]) => {
129+
const href = String(url);
130+
if (href.endsWith('/health')) {
131+
return createJsonResponse({
132+
status: 'ok',
133+
servers: [{ ready: true }],
134+
});
135+
}
136+
return createJsonResponse({
137+
result: {
138+
tools: [{ name: 'list_buckets', description: 'List storage buckets' }],
139+
},
140+
});
141+
}) as unknown as typeof fetch;
142+
const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
143+
144+
try {
145+
const result = await discoverMcpGroupToolsActivity({
146+
servers: [
147+
{
148+
name: 'storage',
149+
transport: 'stdio',
150+
command: 'storage-mcp',
151+
},
152+
],
153+
});
154+
155+
expect(result.results).toEqual([
156+
{
157+
name: 'storage',
158+
tools: [{ name: 'list_buckets', description: 'List storage buckets' }],
159+
},
160+
]);
161+
expect(mockStartMcpDockerServer).toHaveBeenCalledTimes(1);
162+
expect(mockExecFile.mock.calls[0]?.[0]).toBe('docker');
163+
expect(mockExecFile.mock.calls[0]?.[1]).toEqual(['rm', '-f', 'mcp-container-1']);
164+
expect(consoleLogSpy).not.toHaveBeenCalled();
165+
} finally {
166+
consoleLogSpy.mockRestore();
167+
}
168+
});
60169
});

worker/src/temporal/activities/mcp-discovery.activity.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,14 +447,14 @@ async function waitForContainerReady(endpoint: string): Promise<void> {
447447
const servers = data.servers ?? [];
448448
const allReady = servers.every((s) => s.ready);
449449
if (servers.length > 0 && allReady) {
450-
console.log(
450+
workflowDiagnosticLog(
451451
`[MCP Discovery] Container ready after ${attempt + 1}s (${servers.length} server(s) ready)`,
452452
);
453453
return;
454454
}
455455
// HTTP is up but waiting for STDIO client connection
456456
if (attempt % 10 === 0) {
457-
console.log(
457+
workflowDiagnosticLog(
458458
`[MCP Discovery] HTTP ready, waiting for STDIO client... (${servers.filter((s) => s.ready).length}/${servers.length} ready)`,
459459
);
460460
}

0 commit comments

Comments
 (0)