Skip to content

Commit f36b7b4

Browse files
committed
fix: check server responds to HTTP requests before allowing user messages
1 parent a86f047 commit f36b7b4

5 files changed

Lines changed: 53 additions & 47 deletions

File tree

browser-tests/fixtures.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { ENV_FILE } from './constants';
2-
import { test as base } from '@playwright/test';
2+
import { type Page, test as base, expect } from '@playwright/test';
33
import { readFileSync } from 'node:fs';
44

55
interface BrowserTestEnv {
@@ -28,4 +28,29 @@ export const test = base.extend<{ testEnv: BrowserTestEnv }>({
2828
},
2929
});
3030

31-
export { expect } from '@playwright/test';
31+
/**
32+
* Send a chat message and wait for the agent to finish responding.
33+
* Returns the assistant message locator.
34+
*/
35+
export async function sendMessage(page: Page, text: string) {
36+
const chatInput = page.getByTestId('chat-input');
37+
await expect(chatInput).toBeEnabled({ timeout: 60_000 });
38+
39+
const messageList = page.getByTestId('message-list');
40+
const existingCount = await messageList.getByTestId(/^chat-message-/).count();
41+
42+
await chatInput.fill(text);
43+
await page.getByRole('button', { name: 'Send message' }).click();
44+
45+
const assistantMessage = messageList.getByTestId(`chat-message-${existingCount + 1}`);
46+
await expect(assistantMessage).toBeVisible({ timeout: 60_000 });
47+
await expect(assistantMessage).not.toContainText('ECONNREFUSED');
48+
49+
// Wait for streaming to complete so the agent is idle for subsequent tests.
50+
await chatInput.fill('.');
51+
await expect(page.getByRole('button', { name: 'Send message' })).toBeEnabled({ timeout: 30_000 });
52+
53+
return assistantMessage;
54+
}
55+
56+
export { expect };
Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,10 @@
1-
import { expect, test } from '../fixtures';
1+
import { expect, sendMessage, test } from '../fixtures';
22

33
test.describe('Chat invocation', () => {
44
test('send a message and receive a response', async ({ page }) => {
55
await page.goto('/');
66

7-
const chatInput = page.getByTestId('chat-input');
8-
await expect(chatInput).toBeEnabled({ timeout: 60_000 });
9-
10-
await chatInput.fill('What is 2 plus 2? Reply with just the number.');
11-
await page.getByRole('button', { name: 'Send message' }).click();
12-
13-
const messageList = page.getByTestId('message-list');
14-
await expect(messageList).toBeVisible();
15-
16-
await expect(messageList.getByTestId('chat-message-0')).toBeVisible({ timeout: 10_000 });
17-
18-
const assistantMessage = messageList.getByTestId('chat-message-1');
19-
await expect(assistantMessage).toBeVisible({ timeout: 60_000 });
7+
const assistantMessage = await sendMessage(page, 'What is 2 plus 2? Reply with just the number.');
208
await expect(assistantMessage).not.toBeEmpty();
219
});
2210
});

browser-tests/tests/traces.test.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,10 @@
1-
import { expect, test } from '../fixtures';
1+
import { expect, sendMessage, test } from '../fixtures';
22

33
test.describe('Traces', () => {
44
test('traces panel shows trace after invocation', async ({ page }) => {
55
await page.goto('/');
66

7-
const chatInput = page.getByTestId('chat-input');
8-
await expect(chatInput).toBeEnabled({ timeout: 60_000 });
9-
10-
await chatInput.fill('Say hello');
11-
await page.getByRole('button', { name: 'Send message' }).click();
12-
13-
const messageList = page.getByTestId('message-list');
14-
const assistantMessage = messageList.getByTestId('chat-message-1');
15-
await expect(assistantMessage).toBeVisible({ timeout: 60_000 });
16-
await expect(assistantMessage).not.toContainText('ECONNREFUSED');
7+
await sendMessage(page, 'Say hello');
178

189
await page.getByRole('tab', { name: 'Traces' }).click();
1910

src/cli/operations/dev/utils.ts

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { createConnection, createServer } from 'net';
1+
import { createServer } from 'net';
2+
import { request as httpRequest } from 'node:http';
23

34
/** Check if a port is available on a specific host */
45
function checkPort(port: number, host: string): Promise<boolean> {
@@ -39,24 +40,22 @@ export async function waitForPort(port: number, timeoutMs = 3000): Promise<boole
3940
return false;
4041
}
4142

42-
/** Wait until a server is accepting connections on the given port, with timeout. */
43-
export async function waitForServerReady(port: number, timeoutMs = 60000): Promise<boolean> {
43+
/** Wait until a server is responding to HTTP requests on the given port, with timeout. */
44+
export function waitForServerReady(port: number, timeoutMs = 60000): Promise<boolean> {
4445
const start = Date.now();
45-
while (Date.now() - start < timeoutMs) {
46-
const listening = await new Promise<boolean>(resolve => {
47-
const socket = createConnection({ port, host: '127.0.0.1' }, () => {
48-
socket.destroy();
46+
return new Promise(resolve => {
47+
const attempt = () => {
48+
if (Date.now() - start > timeoutMs) return resolve(false);
49+
const req = httpRequest({ hostname: '127.0.0.1', port, path: '/', method: 'GET', timeout: 2000 }, res => {
50+
res.resume();
4951
resolve(true);
5052
});
51-
socket.on('error', () => {
52-
socket.destroy();
53-
resolve(false);
54-
});
55-
});
56-
if (listening) return true;
57-
await new Promise(resolve => setTimeout(resolve, 500));
58-
}
59-
return false;
53+
req.on('error', () => setTimeout(attempt, 500));
54+
req.on('timeout', () => req.destroy());
55+
req.end();
56+
};
57+
attempt();
58+
});
6059
}
6160

6261
/** Sleep helper for retry delays. */

src/cli/operations/dev/web-ui/handlers/start.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,12 @@ async function doStartAgent(
170170
return { success: false, name: agentName, port: 0, error: errorMsg };
171171
}
172172

173-
ctx.runningAgents.set(agentName, { server: agentServer, port: agentPort, protocol: config.protocol });
174-
175-
// Wait for the server to actually accept connections before telling the
176-
// frontend it's ready — otherwise immediate invocations get ECONNREFUSED.
173+
// Wait for the server to accept HTTP requests, not just TCP connections.
174+
// uvicorn binds the port before the ASGI app is ready, so a TCP-only check
175+
// lets the frontend send invocations that get ECONNREFUSED or 503.
176+
// The agent must NOT be added to runningAgents until this check passes,
177+
// otherwise /api/status reports it as running and concurrent /api/start
178+
// calls short-circuit without verifying readiness.
177179
const ready = await waitForServerReady(agentPort);
178180
if (!ready) {
179181
const errorMsg =
@@ -182,5 +184,6 @@ async function doStartAgent(
182184
return { success: false, name: agentName, port: 0, error: errorMsg };
183185
}
184186

187+
ctx.runningAgents.set(agentName, { server: agentServer, port: agentPort, protocol: config.protocol });
185188
return { success: true, name: agentName, port: agentPort };
186189
}

0 commit comments

Comments
 (0)