diff --git a/packages/core/src/runner/runner.ts b/packages/core/src/runner/runner.ts index 0987fb70..ab081b7f 100644 --- a/packages/core/src/runner/runner.ts +++ b/packages/core/src/runner/runner.ts @@ -142,7 +142,7 @@ import { attachAndroid, detachAndroid, promptDevices } from "../android/service" import { forgetAndroidRun } from "../android/tools"; import { parseComputerTarget } from "../computer/targets"; import { timedSync } from "../diagnostics/slow"; -import { StreamAccumulator, addUsage, detectLoginFailure, displayToolName, redactBlock } from "./stream"; +import { StreamAccumulator, addUsage, detectLoginFailure, displayToolName, redactBlock, usedBrowser } from "./stream"; import { tailLines } from "./tail"; import { requireLicense } from "../license/license"; @@ -2923,21 +2923,19 @@ function autoTitle(userMessageId: string): string | undefined { async function detectMissingLogin(job: Job, agent: Agent, text: string) { if (missingLoginReported.has(job.runId)) return; for (const name of job.acc.toolsCalled) if (name.endsWith("report_missing_login")) return; + // Without a page the run was on there is no login to add: the sentence quotes an error or another missing login. + if (!agent.browser.enabled || (job.vmId && !job.vmBrowser) || !usedBrowser(job.acc.toolsCalled)) return; const reason = detectLoginFailure(text); if (!reason) return; - let service = "Unknown service"; let url = ""; - if (agent.browser.enabled && (!job.vmId || job.vmBrowser)) { - try { - const page = job.vmId ? await currentVmPage(job.vmId) : await currentPage(runProfileOf(job) ?? resolveProfileForAgent(agent, job.conversationId).id, job.conversationId); - if (page?.url && /^https?:/i.test(page.url)) { - url = page.url; - service = hostnameOf(page.url) || service; - } - } catch (err) { - log.debug(`no current page for agent ${agent.id}`, err); - } + try { + const page = job.vmId ? await currentVmPage(job.vmId) : await currentPage(runProfileOf(job) ?? resolveProfileForAgent(agent, job.conversationId).id, job.conversationId); + if (page?.url && /^https?:/i.test(page.url)) url = page.url; + } catch (err) { + log.debug(`no current page for agent ${agent.id}`, err); } + const service = url ? hostnameOf(url) : ""; + if (!service) return; try { reportMissingLogin({ agentId: agent.id, runId: job.runId, workspaceId: agent.workspaceId, kind: "other", service, url, reason }); log.info(`run ${job.runId}: detected a login problem (${service})`); diff --git a/packages/core/src/runner/stream.ts b/packages/core/src/runner/stream.ts index c5724a92..67ef5642 100644 --- a/packages/core/src/runner/stream.ts +++ b/packages/core/src/runner/stream.ts @@ -1,4 +1,4 @@ -import { isPlaceholderReply, toolActivity, type ActivityNames } from "@godmode/shared"; +import { BROWSER_MCP_NAME, isPlaceholderReply, toolActivity, type ActivityNames } from "@godmode/shared"; /** * Pure parser for Claude Code `--output-format stream-json` events. * @@ -853,3 +853,9 @@ export function detectLoginFailure(text: string): string | null { } return null; } + +/** The run used the browser: a login failure in its answer can be about the page it is on, not just quoted in a report. */ +export function usedBrowser(tools: Iterable): boolean { + for (const name of tools) if (name.startsWith(`mcp__${BROWSER_MCP_NAME}__`)) return true; + return false; +} diff --git a/packages/core/test/runner.test.ts b/packages/core/test/runner.test.ts index ff9caf70..362b6181 100644 --- a/packages/core/test/runner.test.ts +++ b/packages/core/test/runner.test.ts @@ -317,16 +317,12 @@ describe("runner end-to-end with fake claude", () => { } }); - test("login failures in the final answer are reported as missing logins", async () => { + test("a login failure in the answer of a run without a browser page is no missing login", async () => { const { run } = await startChat({ agentId: agent.id, content: "LOGIN_FAIL" }); const finished = await waitForRun(run.id, 20_000); expect(finished.status).toBe("succeeded"); - await until(() => listMissingLogins({}).some((m) => m.runId === run.id), 5000, "missing login"); - const item = listMissingLogins({}).find((m) => m.runId === run.id)!; - expect(item.kind).toBe("other"); - expect(item.agentId).toBe(agent.id); - expect(item.service).toBe("Unknown service"); - expect(item.reason).toContain("couldn't log in to example.com"); + await new Promise((r) => setTimeout(r, 200)); + expect(listMissingLogins({}).filter((m) => m.runId === run.id)).toHaveLength(0); }); test("Claude's API refusing the sign-in is no missing login of a website", async () => { diff --git a/packages/core/test/stream.test.ts b/packages/core/test/stream.test.ts index 18355b9b..3d2d7018 100644 --- a/packages/core/test/stream.test.ts +++ b/packages/core/test/stream.test.ts @@ -3,7 +3,7 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import type { MessageBlock } from "@godmode/shared"; import { isPlaceholderReply } from "@godmode/shared"; -import { MAX_MESSAGE_IMAGE_BASE64, MAX_TOOL_IMAGE_BASE64, StreamAccumulator, detectLoginFailure, displayToolName, redactBlocks, toolResultContent } from "../src/runner/stream"; +import { MAX_MESSAGE_IMAGE_BASE64, MAX_TOOL_IMAGE_BASE64, StreamAccumulator, detectLoginFailure, displayToolName, redactBlocks, toolResultContent, usedBrowser } from "../src/runner/stream"; function events(name: string): unknown[] { return readFileSync(join(import.meta.dir, "fixtures", name), "utf8") @@ -658,4 +658,12 @@ describe("helpers", () => { expect(detectLoginFailure("No login required — the page is public.")).toBeNull(); expect(detectLoginFailure("I couldn't find the log in button at first, but then succeeded.")).toBeNull(); }); + + test("usedBrowser", () => { + expect(usedBrowser(["Bash", "mcp__browser__browser_navigate"])).toBe(true); + expect(usedBrowser(["mcp__browser__retry_with_browser_use_agent"])).toBe(true); + expect(usedBrowser(new Set(["Read", "mcp__godmode__report_missing_login", "mcp__godmode__browser_profiles_list"]))).toBe(false); + expect(usedBrowser(["mcp__playwright__browser_navigate"])).toBe(false); + expect(usedBrowser([])).toBe(false); + }); });