diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b62db8e..8f5caeec8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Test connection stops reading once it has seen the agent answer + +The button that checks an agent before it is registered sends it a real run and reads what comes +back, needing only the opening of the stream to tell an AG-UI agent from a web server that happens to +be reachable. It was reading the whole reply first and applying that limit afterwards, so the check +took as long as the agent's run did. An agent that streams for more than fifteen seconds — a Bot +working through a document, a model answering slowly — was given up on mid-answer and reported as +`The agent started answering and the connection broke`, about a connection that had not broken and an +agent that had answered correctly in its first two events. It now reads the opening it needs, closes +the connection, and answers in the time the agent took to start rather than the time it took to +finish. ### A key pasted with a line break in it is now refused, instead of reported as an unreachable agent The box that holds an agent's key takes whatever is pasted into it, and what comes off a clipboard is diff --git a/server/src/agents/connection-test.ts b/server/src/agents/connection-test.ts index 1606e2149..b310558db 100644 --- a/server/src/agents/connection-test.ts +++ b/server/src/agents/connection-test.ts @@ -20,6 +20,41 @@ const TEST_TIMEOUT_MS = 15_000; /** Enough of the stream to prove it is an agent. Reading it all could mean reading a whole reply. */ const MAX_BYTES = 8_000; +/** + * The opening of the answer, and then stop reading. + * + * The cap above was applied to a string this process had already taken in full, because + * `response.text()` reads a body to its end. An agent that streams — which is what an agent does — + * therefore held the form open for as long as its run took, and if that outlasted the timeout the + * abort came back through `text()` as a rejected read: `ok: false`, status 200, "The agent started + * answering and the connection broke." Nothing broke. The agent had answered correctly, in the first + * two events, and the person registering it was told to go and look at their own service. + * + * Counted in bytes, which is what the cap is named in and what a chunk off the wire is measured in. + * The chunk that crosses the cap is kept whole rather than cut at it: it is one read past the cap at + * most, and a cut through a multi-byte character would put a replacement character in the middle of + * a line the scan is about to read. + */ +async function readOpening(body: ReadableStream): Promise { + const reader = body.getReader(); + const decoder = new TextDecoder(); + let text = ""; + let bytes = 0; + try { + while (bytes < MAX_BYTES) { + const { done, value } = await reader.read(); + if (done) break; + bytes += value.byteLength; + text += decoder.decode(value, { stream: true }); + } + } finally { + // The agent is very likely still writing. Nothing is going to read the rest, and leaving it open + // holds a socket into somebody's agent for as long as that run cares to go on. + void reader.cancel().catch(() => undefined); + } + return text; +} + export type ConnectionTestResult = | { ok: true; @@ -152,7 +187,7 @@ export async function testAgentConnection( let body: string; try { - body = (await response.text()).slice(0, MAX_BYTES); + body = response.body ? await readOpening(response.body) : ""; } catch { return { ok: false, diff --git a/server/tests/agent-connection-live.test.ts b/server/tests/agent-connection-live.test.ts index 3b2e9468e..b0d40f442 100644 --- a/server/tests/agent-connection-live.test.ts +++ b/server/tests/agent-connection-live.test.ts @@ -248,3 +248,71 @@ describe("registering an agent that answers badly", () => { expect(result.reason.length).toBeGreaterThan(0); }); }); + +/** + * An agent that keeps talking. + * + * The check needs the opening of the stream and says so: the cap beside it is named for how much of + * the answer is enough to prove the far end is an agent. What decides whether that cap means + * anything is whether the read stops there, and the person registering an agent that streams for a + * while is the one who finds out. + */ +describe("registering an agent that streams a long answer", () => { + /** The events, and then a run that goes on writing: a Bot working through a long document. */ + function talkativeAgent() { + return Bun.serve({ + port: 0, + fetch: () => + new Response( + new ReadableStream({ + async start(controller) { + const encoder = new TextEncoder(); + controller.enqueue( + encoder.encode( + 'event: RUN_STARTED\ndata: {"type":"RUN_STARTED"}\n\n' + + 'event: TEXT_MESSAGE_START\ndata: {"type":"TEXT_MESSAGE_START"}\n\n', + ), + ); + // A kilobyte at a time, slowly, the way a model streams. Nothing here closes: the run + // is still going, which is the whole point. + for (let chunk = 0; chunk < 4_000; chunk += 1) { + controller.enqueue( + encoder.encode( + `event: TEXT_MESSAGE_CONTENT\ndata: {"type":"TEXT_MESSAGE_CONTENT","delta":"${"x".repeat(960)}"}\n\n`, + ), + ); + await Bun.sleep(20); + } + controller.close(); + }, + }), + { headers: { "content-type": "text/event-stream" } }, + ), + }); + } + + test("is reported as working, without waiting for it to finish", async () => { + const agent = talkativeAgent(); + + try { + const started = Date.now(); + const result = await testAgentConnection( + `http://127.0.0.1:${agent.port}/ag-ui`, + { allowPrivateHosts: true, timeoutMs: 3_000 }, + ); + const took = Date.now() - started; + + // It answered, immediately and correctly, with events this check can read. Anything else is + // this deployment describing a working agent as one that may not be reachable. + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.events).toContain("RUN_STARTED"); + // Well inside the timeout. The check has what it needs after the opening of the stream, and + // an answer that arrives only when the agent stops talking is an answer that depends on the + // agent stopping. + expect(took).toBeLessThan(2_000); + } finally { + agent.stop(true); + } + }); +});