From 42eb420545fac36a7d240ec1f24eab4f207bd1ac Mon Sep 17 00:00:00 2001 From: cynarAI <155274210+cynarAI@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:24:34 +0200 Subject: [PATCH] Agent replies without thinking aloud or tool names; chart-links E2E names its failure (#199) A run of the real prompt against the local model showed reasoning written as chat text, internal tool names in a capabilities answer, and only a data fetch when asked to build a tool. The working principles now say that everything written is shown, to describe abilities in plain words, and that a request for a tool means building one. The chart-links spec now waits for the chart or its error and reports browser errors, so the recurring 90-second timeout names its cause. The ZIP upload spec matches the file chip exactly, since the agent reply can quote the name. --- apps/web/e2e/chart-links.spec.ts | 46 +++++++++++++++---- apps/web/e2e/media-pipeline.spec.ts | 3 +- .../src/executor-instructions.test.ts | 2 + packages/adapters/src/executor-prompt.ts | 3 +- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/apps/web/e2e/chart-links.spec.ts b/apps/web/e2e/chart-links.spec.ts index 4813a0e..8e6f878 100644 --- a/apps/web/e2e/chart-links.spec.ts +++ b/apps/web/e2e/chart-links.spec.ts @@ -1,15 +1,47 @@ -import { expect, test } from "@playwright/test"; +import { expect, type Locator, type Page, test } from "@playwright/test"; import { captureScreenshot } from "./helpers"; const fixture = "/e2e/fixtures/chart-links.html"; +/** Browser errors per page, so a chart that never renders fails with its cause. */ +const browserErrors = new WeakMap(); + +test.beforeEach(({ page }) => { + const errors: string[] = []; + browserErrors.set(page, errors); + page.on("pageerror", (error) => errors.push(error.message)); + page.on("console", (message) => { + if (message.type() === "error") errors.push(message.text()); + }); + page.on("response", (response) => { + if (response.status() >= 400) errors.push(`${response.status()} ${response.url()}`); + }); +}); + +/** Waits for the plotted dot, failing fast with the chart error and browser errors instead. */ +async function expectChart(page: Page, scope: Locator | Page = page) { + const circle = scope.locator("svg circle").first(); + const failure = scope.getByText("Chart failed to render", { exact: false }).first(); + try { + // Every navigation renders Plot again, which takes seconds on a loaded CI runner. + await expect(circle.or(failure).first()).toBeVisible({ timeout: 90_000 }); + if (await failure.isVisible()) throw new Error(await failure.innerText()); + } catch (error) { + const errors = browserErrors.get(page) ?? []; + throw new Error( + `${error instanceof Error ? error.message : String(error)}\nBrowser errors:\n${errors.join("\n") || "none"}`, + ); + } + return circle; +} + test.describe .serial("chart links", () => { test("cannot execute script when clicked or expanded", async ({ page }, testInfo) => { test.setTimeout(120_000); // ChartCanvas lazy-loads Observable Plot; pay the import cost once before the matrix below. await page.goto(`${fixture}?${new URLSearchParams({ href: "#details", inspect: "0" })}`); - await expect(page.locator("svg circle").first()).toBeVisible({ timeout: 90_000 }); + await expectChart(page); for (const inspect of ["0", "1"]) { for (const href of [ 'javascript:void(document.body.dataset.plotExecuted="yes")', @@ -19,9 +51,7 @@ test.describe "file:///example.txt", ]) { await page.goto(`${fixture}?${new URLSearchParams({ href, inspect })}`); - const circle = page.locator("svg circle").first(); - // Every navigation renders Plot again, which takes seconds on a loaded CI runner. - await expect(circle).toBeVisible({ timeout: 90_000 }); + const circle = await expectChart(page); const assertInert = async () => { const links = page.locator("svg a"); expect(await links.count()).toBeGreaterThan(0); @@ -43,7 +73,7 @@ test.describe await assertInert(); await page.getByRole("button", { name: "Expand", exact: true }).click(); const dialog = page.getByRole("dialog"); - await expect(dialog.locator("svg circle")).toBeVisible({ timeout: 90_000 }); + await expectChart(page, dialog); await dialog.locator("svg circle").click(); await assertInert(); await expect(page).toHaveURL(`${fixture}?${new URLSearchParams({ href, inspect })}`); @@ -62,7 +92,7 @@ test.describe route.fulfill({ body: "Chart destination" }), ); await page.goto(`${fixture}?${new URLSearchParams({ href: "#details" })}`); - await expect(page.locator("svg circle").first()).toBeVisible({ timeout: 90_000 }); + await expectChart(page); for (const href of [ "https://example.test/chart?a=1&b=2", "mailto:chart@example.test", @@ -73,7 +103,7 @@ test.describe "java%73cript:void(0)", ]) { await page.goto(`${fixture}?${new URLSearchParams({ href })}`); - await expect(page.locator("svg circle").first()).toBeVisible({ timeout: 90_000 }); + await expectChart(page); const link = page.locator("svg a").first(); await expect(link).toBeAttached(); expect( diff --git a/apps/web/e2e/media-pipeline.spec.ts b/apps/web/e2e/media-pipeline.spec.ts index 3f6ff70..eefc3a4 100644 --- a/apps/web/e2e/media-pipeline.spec.ts +++ b/apps/web/e2e/media-pipeline.spec.ts @@ -104,7 +104,8 @@ test("a repository ZIP above the inline cap streams up as a file", async ({ page const response = await upload; expect(response.status()).toBe(201); expect(response.request().headers()["content-type"]).toBe("application/zip"); - await expect(page.getByTestId("transcript").getByText("repo.zip")).toBeVisible({ + // Exact: the agent's reply can quote the file name too. + await expect(page.getByTestId("transcript").getByText("repo.zip", { exact: true })).toBeVisible({ timeout: 30_000, }); await captureScreenshot(page, testInfo, "zip-upload"); diff --git a/packages/adapters/src/executor-instructions.test.ts b/packages/adapters/src/executor-instructions.test.ts index 6e934ea..7fce199 100644 --- a/packages/adapters/src/executor-instructions.test.ts +++ b/packages/adapters/src/executor-instructions.test.ts @@ -359,6 +359,8 @@ describe("run instructions", () => { ); expect(instructions).toContain("Say something is done only after a tool confirmed it"); expect(instructions).toContain("Reply in the language the user writes in."); + expect(instructions).toContain("do not think aloud"); + expect(instructions).toContain("never by tool names or ids"); expect(instructions).toContain("No apps are connected yet."); expect(instructions).not.toContain("A schedule started this run"); }); diff --git a/packages/adapters/src/executor-prompt.ts b/packages/adapters/src/executor-prompt.ts index 5a04cb5..48e25b5 100644 --- a/packages/adapters/src/executor-prompt.ts +++ b/packages/adapters/src/executor-prompt.ts @@ -55,10 +55,11 @@ export function formatWorkingPrinciples(toolNames: ReadonlySet): string : undefined, "- Before saying you cannot do something, check your tools, the connected apps and the skills listed here.", canBuildTools - ? "- When no tool does what the task needs, build one: write a script in your workspace, test it, and save how to run it as a skill with skill_create so you and the user can reuse it." + ? "- When the user asks for a tool, or none of yours does what the task needs, build one: write a script in your workspace, test it, and save how to run it as a skill with skill_create so you and the user can reuse it." : undefined, "- Ask only when the answer changes the outcome and there is no sensible default; otherwise pick the default and say which.", "- Reply in the language the user writes in. Lead with the result and keep it short.", + "- Everything you write is shown to the user: do not think aloud, and before a tool call write at most one short sentence. Describe what you can do in plain words, never by tool names or ids.", ] .filter(Boolean) .join("\n");