diff --git a/apps/server/src/browser.ts b/apps/server/src/browser.ts index 5675d67b3..1de55d87c 100644 --- a/apps/server/src/browser.ts +++ b/apps/server/src/browser.ts @@ -136,11 +136,19 @@ export class BrowserService { const response = await this.request("/sessions", { id, url: target }, signal); return await this.save(owner, await response.json(), id); } catch (error) { - await this.save( - owner, - { ...value, url: target, status: "error", updatedAt: new Date().toISOString() }, - id, - ); + // A Stop is not a broken session. The person asked for the turn to end, the profile and its + // cookies are intact, and `request` reports a cancelled request by throwing the signal's own + // abort - so writing "error" here is what puts "Needs attention" and "Reconnect browser" in + // front of somebody whose browser is working, and hides the preview behind a status they did + // not cause. Only the caller's own abort counts: the 45s request timeout is a real failure and + // leaves the signal untouched, so it still records. + if (!signal?.aborted) { + await this.save( + owner, + { ...value, url: target, status: "error", updatedAt: new Date().toISOString() }, + id, + ); + } throw error; } } diff --git a/tests/browser.test.ts b/tests/browser.test.ts index 7308ae05d..c4773fc9e 100644 --- a/tests/browser.test.ts +++ b/tests/browser.test.ts @@ -263,6 +263,52 @@ test("cancelled chat browser requests do not start navigation or a follow-up rea assert.equal((await db.list("owner", "browsers")).length, 1); }); +test("stopping a turn does not leave the browser marked as needing attention", async (t) => { + const controller = new AbortController(); + let arrived = 0; + const { db, service } = await browserFixture(t, async () => { + arrived += 1; + // The worker has the request and has not answered, which is the state a Stop arrives into. + return new Promise(() => {}); + }); + + const stopped = service.observeForThread( + "owner", + "chat-thread", + savedSession.url, + controller.signal, + ); + // Abort only once the request is genuinely in flight, so this exercises the cancellation path + // rather than the pre-flight check at the top of observeForThread. + while (arrived === 0) await new Promise((resolve) => setTimeout(resolve, 5)); + controller.abort(); + + await assert.rejects(stopped, { name: "AbortError" }); + + const [stored] = await db.list("owner", "browsers"); + // The turn was stopped, not broken. "error" is what the app reads as "Needs attention", hides the + // preview behind, and offers "Reconnect browser" for - none of which a person needs after asking + // for a turn to end, and none of which their profile stopped being true for. + assert.equal(stored?.status, "idle"); +}); + +test("a worker that genuinely fails a turn still marks the browser as needing attention", async (t) => { + // The other half of the rule: only the caller's own Stop is silent. A failure the worker actually + // reported must keep recording, or this would hide a browser that really is broken. + const { db, service } = await browserFixture(t, () => ({ + status: 500, + data: { error: { message: "The browser could not be launched." } }, + })); + + await assert.rejects( + service.observeForThread("owner", "chat-thread", savedSession.url), + /could not be launched/, + ); + + const [stored] = await db.list("owner", "browsers"); + assert.equal(stored?.status, "error"); +}); + test("browser read fails on missing page text instead of inventing observation content", async (t) => { const { db, service } = await browserFixture(t, () => ({ data: { url: savedSession.url, title: savedSession.title }, diff --git a/tests/helpers/browser.ts b/tests/helpers/browser.ts index 41292879e..e15cdb6f2 100644 --- a/tests/helpers/browser.ts +++ b/tests/helpers/browser.ts @@ -13,13 +13,18 @@ import { Files } from "../../apps/server/src/files.ts"; export async function browserFixture( t: TestContext, - handle: (path: string, body: Record) => { status?: number; data: unknown }, + handle: ( + path: string, + body: Record, + ) => { status?: number; data: unknown } | Promise<{ status?: number; data: unknown }>, ) { const server = createServer(async (request, response) => { const chunks = []; for await (const chunk of request) chunks.push(Buffer.from(chunk)); const body = chunks.length ? JSON.parse(Buffer.concat(chunks).toString()) : {}; - const result = handle(request.url ?? "", body); + // Awaited so a handler can model a worker that has not answered yet, which is the state a + // Stop arrives into. + const result = await handle(request.url ?? "", body); response.writeHead(result.status ?? 200, { "content-type": "application/json" }); response.end(JSON.stringify(result.data)); });