From 81ebe479e8db8a3d59c86cdfb894bd917734492f Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 30 Oct 2026 00:57:59 -0500 Subject: [PATCH] fix(evals): bind rendered Markdown replies to saved run evidence (#18264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip lets people assign work to AI agents and read saved results. > - Product journeys must prove that a result is visible and belongs to the completed run. > - The shared observer compares stored Markdown with browser text. > - A document link can render as an issue chip, so those strings differ. > - This PR keeps run and comment attribution, then compares full text and exact link destinations. > - The benefit is a valid browser result without a false timeout or a weaker provenance check. ## Linked Issues or Issue Description Refs: #18248, #18255. **What happened?** The protected-action story completed both decisions and all four native turns. Its final explanation contained a document link. The browser rendered that link as an issue chip. The observer waited for raw Markdown to equal rendered text, so it could not accept the valid result. **Expected behavior** Accept complete matching content from the final run after reload. Keep the exact link destination, task anchor, author, run and durable final-message checks. Reject missing or changed content. **Steps to reproduce** 1. Save an attributed reply with bold text and a Markdown document link. 2. Render the link as an issue chip in the task bubble. 3. Run `readBoundTaskFinalReply`. The original observer times out. The new browser regression preserves that failure. ## What Changed - Add a UI-owned content projection using its existing Markdown renderer and GFM/issue-reference plugins. Keep DOM observation, attribution and destination checks in the task-reply observer. Keep source Markdown and observed text separately. - Read visible DOM text, link labels and destinations. Require the structured projection to match the actual visible text. Allow only recognized product chips to replace a link label. Ordinary links keep their source label. Reuse the production issue-reference and company-route helpers, plus the current task URL. Ordinary root-relative anchors keep their original path. Preserve exact marker checks and all run/comment/native-event attribution checks. - Check visible decline explanations and unread data in preserved native source text. - Add browser regressions and source/content tests. Reuse the UI’s existing `react-markdown` and `remark-gfm` dependencies. The final diff changes no package manifest or lockfile. - Match generated GFM checkbox states and reject changed states. Document supported content and fail-closed behavior. ## Verification - Original observer: the decorated-link browser regression fails at the final-reply predicate. - Fixed observer: all 21 browser regressions pass, without credentials or model calls. - Focused observer, decline, content and journey contract suites: 64 tests pass. - E2E and UI TypeScript checks pass. All UI token gates pass. The unchanged frozen install passes. - Full CI passed on the preceding 6eba7ff head (41003112166). Full CI passed on the preceding b9a2571 head (41006205475), with both reviewers at 5/5. Current-head full CI passed at b3d2d08305014151e1eca3b295ea588666ce0b92 (41009669966), including typecheck, build and all eight E2E shards. Both reviewers scored this commit 5/5, with no unresolved threads. The initial lockfile policy failure was corrected. The final diff has no manifest or lock changes, and its unchanged frozen install passed. - The optional positive GFM strikethrough assertion also passes. - The Node 24/tsx end-to-end observer probe passes. The callback has no external compiler helper; a browser regression covers actual tsx serialization. - The Paperclip review finding is also fixed: bare URLs and issue keys use the production plugins and the UI’s `scope=accessible` company-prefix context, with RED/GREEN browser coverage. Source Markdown is required in the receipt. - All Greptile findings are fixed, including matching the UI’s accessible-company scope: ordinary link labels, issue-prefix/fragment resolution and ordinary root-relative paths have positive and negative browser coverage. - The deployed attempt remains a failed original measurement. Fresh qualification is required after this repair. ## Risks The comparison normalizes whitespace and inline formatting. It verifies full text and exact link targets, while link labels may become product chips. Images, raw HTML, tables and fenced code fail closed. Those constructs may require a future explicit extension if a journey uses them. No application behavior, model, budget, approval or connection permission changes. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, code execution and browser tools. The host does not expose an exact underlying model version or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal issue identifier - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- tests/runner-e2e/PRODUCT-JOURNEYS.md | 11 +++ .../connection-guidance-evidence.ts | 14 ++-- tests/runner-e2e/connection-guidance.test.ts | 17 +++- tests/runner-e2e/native-task-reply.spec.ts | 81 +++++++++++++++++++ tests/runner-e2e/native-task-reply.test.ts | 16 ++++ tests/runner-e2e/native-task-reply.ts | 59 +++++++++++--- tests/runner-e2e/playwright-support.config.ts | 2 +- .../product-journeys/agent-flows.ts | 5 +- tests/runner-e2e/task-reply-content.test.ts | 41 ++++++++++ tests/runner-e2e/task-reply-content.ts | 57 +++++++++++++ ui/src/lib/markdown-reply-content.ts | 56 +++++++++++++ 11 files changed, 336 insertions(+), 23 deletions(-) create mode 100644 tests/runner-e2e/native-task-reply.spec.ts create mode 100644 tests/runner-e2e/task-reply-content.test.ts create mode 100644 tests/runner-e2e/task-reply-content.ts create mode 100644 ui/src/lib/markdown-reply-content.ts diff --git a/tests/runner-e2e/PRODUCT-JOURNEYS.md b/tests/runner-e2e/PRODUCT-JOURNEYS.md index e90f4723ddf..66983e18052 100644 --- a/tests/runner-e2e/PRODUCT-JOURNEYS.md +++ b/tests/runner-e2e/PRODUCT-JOURNEYS.md @@ -119,6 +119,17 @@ satisfy the check. A pending or suppressed comment can use the same proven nativ reply path. A finish summary by itself, an earlier turn, a progress bubble, or a cancelled run cannot satisfy it. Private receipts record the run, thread anchor and observed text; classic protocol comment matchers stay strict. +Markdown replies compare complete text, GFM checkbox states and exact link destinations after parsing +through the UI-owned projection of its existing Markdown renderer and GFM/issue-reference plugins. The observer receives the +same visible-company prefix list used by the UI, so bare issue keys and URLs +match their rendered links. Whitespace and inline formatting can change during rendering. A +document or issue chip can replace a link label, but must retain its destination. +Ordinary links retain their source label. Destination resolution uses the existing +production company-route helper and the current task URL for fragments. +Receipts retain both source Markdown and observed content. Unsupported structures +such as images, raw HTML, tables and fenced code fail closed. Exact marker checks +remain exact, and decline checks inspect both visible explanations and source +text for unread service data. The decline oracle accepts this explicit native receipt without inventing an issue comment. The final run must start after the saved decline, and the usual truthful explanation, zero service calls and no unread marker checks still apply. diff --git a/tests/runner-e2e/connection-guidance-evidence.ts b/tests/runner-e2e/connection-guidance-evidence.ts index 13eb097668c..3c750678f48 100644 --- a/tests/runner-e2e/connection-guidance-evidence.ts +++ b/tests/runner-e2e/connection-guidance-evidence.ts @@ -1,5 +1,6 @@ import type { StoryComment } from "./everyday-observations.js"; import type { BoundTaskFinalReply } from "./native-task-reply.js"; +import { taskReplyContentMatches } from "./task-reply-content.js"; type Decision = { id: string; kind: string; status: string; resolvedAt?: string | null; @@ -51,7 +52,7 @@ function declineReplyEvidence(input: ConnectionGuidanceDeclineInput) { const native = input.nativeFinalReply; const nativeAttributed = input.caseId === "service-decline" && validDecision && native?.source === "native-turn" && native.agentId === input.leadAgentId && native.observedAfterReload && native.visible && native.settled && native.nativeFinal - && native.finalEvent?.body === native.body + && typeof native.sourceBody === "string" && native.finalEvent?.body === native.sourceBody && taskReplyContentMatches(native, native.finalEvent.body) && (native.anchor === `${native.runId}:turn` || native.anchor?.startsWith(`${native.runId}:turn:`) === true) && input.runs.some(run => run.id === native.runId && run.agentId === input.leadAgentId && run.nativeIssueId === input.issueId && run.runtimeMode === "native" && run.status === "succeeded" && Date.parse(run.startedAt ?? "") >= resolvedAt @@ -59,7 +60,8 @@ function declineReplyEvidence(input: ConnectionGuidanceDeclineInput) { // Keep native evidence explicit. It is a saved final-message/DOM receipt, // never a synthetic issue comment or a finish-summary fallback. return { validDecision, afterDecision: [...afterDecision, ...(nativeAttributed ? [native!] : [])], - attributed: [...attributed, ...(nativeAttributed ? [native!] : [])] }; + attributed: [...attributed, ...(nativeAttributed ? [native!] : [])], + nativeSourceBody: nativeAttributed ? native!.finalEvent!.body : null }; } /** Readiness waits for storage, not a passing explanation. Wrong wording must still fail grading. */ @@ -74,15 +76,17 @@ export function explainsConnectionUnavailable(text: string): boolean { } export function gradeConnectionGuidanceDecline(input: ConnectionGuidanceDeclineInput) { - const { validDecision, afterDecision, attributed } = declineReplyEvidence(input); + const { validDecision, afterDecision, attributed, nativeSourceBody } = declineReplyEvidence(input); return [ { id: "guidance-decline-decision", passed: validDecision && input.decisions.length === 1, detail: "Exactly one correctly typed, resolved decline belongs to the selected decision." }, { id: "guidance-decline-attributed-explanation", - passed: attributed.some(reply => typeof reply.body === "string" && explainsConnectionUnavailable(reply.body)), + passed: attributed.some(reply => typeof reply.body === "string" && explainsConnectionUnavailable(reply.body)) + && (!input.nativeFinalReply || explainsConnectionUnavailable(input.nativeFinalReply.body)), detail: "A saved explanation follows the decision and joins to the final successful task run; native task replies also require the reloaded final-message receipt." }, { id: "guidance-decline-no-use", passed: (input.caseId === "connection-decline" || input.calls === 0) && input.sameConnections && - afterDecision.every(reply => (typeof reply.body !== "string" || !reply.body.includes(input.marker))), + afterDecision.every(reply => (typeof reply.body !== "string" || !reply.body.includes(input.marker))) + && (nativeSourceBody === null || !nativeSourceBody.includes(input.marker)), detail: "No connection changes or unread marker in replies. Installed-service/provider declines also require an observed zero fixture-call count; Notion setup does not execute a service." }, ]; } diff --git a/tests/runner-e2e/connection-guidance.test.ts b/tests/runner-e2e/connection-guidance.test.ts index 3b1159636b8..adb70c0281c 100644 --- a/tests/runner-e2e/connection-guidance.test.ts +++ b/tests/runner-e2e/connection-guidance.test.ts @@ -52,7 +52,7 @@ describe("neutral decline evidence", () => { const native = () => ({ ...structuredClone(valid), caseId: "service-decline" as const, replies: [], decisions: [{ id: "decision", kind: "request_confirmation", status: "rejected", resolvedAt: "2026-10-06T22:00:00Z" }], runs: [{ ...valid.runs[0], runtimeMode: "native", startedAt: "2026-10-06T22:00:01Z" }], - nativeFinalReply: { runId: "run", agentId: "lead", body: valid.replies[0].body, source: "native-turn", observedAfterReload: true, + nativeFinalReply: { runId: "run", agentId: "lead", body: valid.replies[0].body, sourceBody: valid.replies[0].body, source: "native-turn", observedAfterReload: true, visible: true, settled: true, nativeFinal: true, anchor: "run:turn", finalEvent: { seq: 10, sourceEventId: "final", sourceInstanceId: "runner", sourceSeq: 10, protocolSchemaVersion: 1, body: valid.replies[0].body } }, }); @@ -62,6 +62,17 @@ describe("neutral decline evidence", () => { expect(passes(input)).toBe(true); expect(input.replies).toEqual([]); }); + it("checks both the visible explanation and preserved native Markdown when links become chips", () => { + const input = native(); + const body = "Contacts are unavailable. See [Connection outcome](/QA/issues/QA-123#document-outcome)."; + Object.assign(input.nativeFinalReply, { sourceBody: body, body: "Contacts are unavailable. See QA-123.", origin: "https://fixture.test", pageUrl: "https://fixture.test/QA/issues/QA-123", + content: [{ text: "Contacts are unavailable. See " }, { href: "/QA/issues/QA-123#document-outcome", label: "QA-123", decorated: true }, { text: "." }] }); + input.nativeFinalReply.finalEvent.body = body; + expect(passes(input)).toBe(true); + Object.assign(input.nativeFinalReply, { sourceBody: body.replace("Connection outcome", "PRIVATE_MARKER") }); + input.nativeFinalReply.finalEvent.body = body.replace("Connection outcome", "PRIVATE_MARKER"); + expect(passes(input)).toBe(false); + }); it("excludes an earlier successful native reply when a later successful run exists", () => { const input = native(); input.runs.push({ ...input.runs[0], id: "latest", startedAt: "2026-10-06T22:00:04Z", finishedAt: "2026-10-06T22:00:06Z" }); @@ -73,7 +84,7 @@ describe("neutral decline evidence", () => { }); it("rejects stale, unbound or untruthful native decline output and unread service data", () => { for (const change of [{ runId: "other" }, { agentId: "other" }, { source: "summary" }, { observedAfterReload: false }, - { settled: false }, { nativeFinal: false }, { visible: false }, { anchor: "other:turn" }, { finalEvent: null }]) { + { settled: false }, { nativeFinal: false }, { visible: false }, { anchor: "other:turn" }, { finalEvent: null }, { sourceBody: undefined }]) { const input = native(); Object.assign(input.nativeFinalReply, change); expect(passes(input)).toBe(false); } for (const change of [{ startedAt: "2026-10-06T21:59:00Z" }, { startedAt: "invalid" }, { runtimeMode: "legacy" }, @@ -81,7 +92,7 @@ describe("neutral decline evidence", () => { const input = native(); Object.assign(input.runs[0], change); expect(passes(input)).toBe(false); } for (const body of ["Done.", "Unavailable but PRIVATE_MARKER"]) { - const input = native(); input.nativeFinalReply.body = input.nativeFinalReply.finalEvent.body = body; + const input = native(); input.nativeFinalReply.body = input.nativeFinalReply.sourceBody = input.nativeFinalReply.finalEvent.body = body; expect(hasConnectionGuidanceDeclineReply(input)).toBe(true); expect(passes(input)).toBe(false); } for (const change of [{ calls: 1 }, { sameConnections: false }, { decisions: [] }]) expect(passes({ ...native(), ...change })).toBe(false); diff --git a/tests/runner-e2e/native-task-reply.spec.ts b/tests/runner-e2e/native-task-reply.spec.ts new file mode 100644 index 00000000000..14372a944a5 --- /dev/null +++ b/tests/runner-e2e/native-task-reply.spec.ts @@ -0,0 +1,81 @@ +import { expect, test } from "@playwright/test"; +import { execFileSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { readBoundTaskFinalReply } from "./native-task-reply.js"; + +const markdown = "I **couldn't retrieve** the pages. See [Connection outcome](/QA/issues/QA-123#document-outcome)."; +const rendered = "I couldn't retrieve the pages. See QA-123."; +const run = { id: "final-run", companyId: "company", agentId: "agent", status: "succeeded", runtimeMode: "native", + resultJson: { presentationDecision: { commentId: "selected" } } }; +const comment = { id: "selected", authorAgentId: "agent", createdByRunId: run.id, body: markdown }; + +test("the QA tsx loader produces a self-contained browser callback", async ({ page }) => { + const moduleUrl = new URL("./native-task-reply.ts", import.meta.url).href; + const loader = fileURLToPath(new URL("../../cli/node_modules/tsx/dist/loader.mjs", import.meta.url)); + const serialized = JSON.parse(execFileSync(process.execPath, ["--import", loader, "--input-type=module", "-e", + `import { observeTaskReplyElements } from ${JSON.stringify(moduleUrl)}; console.log(JSON.stringify(observeTaskReplyElements.toString()));`], { encoding: "utf8" })); + await page.setContent(`
${rendered}
`); + const observations: any = await page.evaluate(`(${serialized})(Array.from(document.querySelectorAll('[data-testid="reply"]')))`); + expect(observations[0].body).toBe("I couldn't retrieve the pages. See QA-123."); + expect(observations[0].content.some((part: any) => part.label === "QA-123" && part.decorated)).toBe(true); +}); + +for (const [source, html, passes] of [ + ["Depends on QA-123.", "Depends on QA-123.", true], + ["See https://example.com/", "See https://example.com/", true], + ["Depends on TREE-604.", "Depends on TREE-604.", true], + ["Depends on QA-123.", "Depends on QA-123.", false], + ["See https://example.com/", "See https://example.com/", false], + ["- [x] Finished\n- [ ] Next", "", true], + ["- [x] Finished\n- [ ] Next", "", false], +] as const) test(`reloaded auto-linked content: ${source} (${passes})`, async ({ page }) => { + const url = "https://fixture.test/QA/issues/QA-1"; + await page.route(url, route => route.fulfill({ contentType: "text/html", body: + `
${html}
` })); + await page.goto(url); + const result = readBoundTaskFinalReply({ page, runId: run.id, agentId: "agent", knownCompanyPrefixes: ["QA"], deadlineAt: Date.now() + (passes ? 3000 : 300), + load: async () => ({ run, comments: [{ ...comment, body: source }], events: [] }) }); + if (passes) expect((await result).sourceBody).toBe(source); + else await expect(result).rejects.toThrow(); +}); + +for (const [name, content, passes] of [ + ["decorated document link and bold text", rendered, true], + ["ordinary link with its source label", rendered.replace(" data-mention-kind='issue'", "").replace(">QA-123", ">Connection outcome"), true], + ["renamed ordinary link", rendered.replace(" data-mention-kind='issue'", ""), false], + ["wrong destination", rendered.replace("#document-outcome", "#document-other"), false], + ["missing link", rendered.replace(//, "QA-123"), false], + ["extra visible text", rendered + " Extra.", false], + ["hidden source text", rendered.replace("couldn't retrieve", ""), false], +] as const) test(`reloaded task reply: ${name}`, async ({ page }) => { + await page.route("https://fixture.test/QA/issues/QA-123", route => route.fulfill({ contentType: "text/html", body: + `
${content}
` })); + await page.goto("https://fixture.test/QA/issues/QA-123"); + const result = readBoundTaskFinalReply({ page, runId: run.id, agentId: "agent", knownCompanyPrefixes: ["QA"], deadlineAt: Date.now() + (passes ? 3000 : 300), + load: async () => ({ run, comments: [comment], events: [] }) }); + if (passes) { + const receipt = await result; + expect(receipt.source).toBe("posted-comment"); + expect(receipt.sourceBody).toBe(markdown); + expect(receipt.body).toBe(content.includes(">Connection outcome") ? "I couldn't retrieve the pages. See Connection outcome." : "I couldn't retrieve the pages. See QA-123."); + expect(receipt.observedAfterReload).toBe(true); + } else await expect(result).rejects.toThrow(); +}); + +for (const [sourceHref, renderedHref, passes] of [ + ["/issues/QA-123#document-outcome", "/QA/issues/QA-123#document-outcome", true], + ["#document-outcome", "/QA/issues/QA-123#document-outcome", true], + ["/issues/QA-123#document-outcome", "/OTHER/issues/QA-123#document-outcome", false], + ["#document-outcome", "/QA/issues/QA-999#document-outcome", false], + ["/projects/example", "/projects/example", true], + ["/projects/example", "/QA/projects/example", false], +] as const) test(`reloaded link ${sourceHref} to ${renderedHref}`, async ({ page }) => { + const url = "https://fixture.test/QA/issues/QA-123"; + await page.route(url, route => route.fulfill({ contentType: "text/html", body: + `
Read Outcome
` })); + await page.goto(url); + const result = readBoundTaskFinalReply({ page, runId: run.id, agentId: "agent", knownCompanyPrefixes: ["QA"], deadlineAt: Date.now() + (passes ? 3000 : 300), + load: async () => ({ run, comments: [{ ...comment, body: `Read [Outcome](${sourceHref})` }], events: [] }) }); + if (passes) expect((await result).pageUrl).toBe(url); + else await expect(result).rejects.toThrow(); +}); diff --git a/tests/runner-e2e/native-task-reply.test.ts b/tests/runner-e2e/native-task-reply.test.ts index 06243c6c562..5e7d7635bf0 100644 --- a/tests/runner-e2e/native-task-reply.test.ts +++ b/tests/runner-e2e/native-task-reply.test.ts @@ -73,3 +73,19 @@ it("accepts a proven native final before the selected comment is projected, and expect(taskReplyBelongsToRun(observed, pendingProjection, "owned-agent", [])).toBe(false); } }); + +it("binds complete Markdown content and the exact link destination despite a decorated issue label", () => { + const body = "I **couldn't retrieve** the pages. See [Connection outcome](/QA/issues/QA-123#document-outcome)."; + const comment = { id: "selected", authorAgentId: "owned-agent", createdByRunId: "final-run", body }; + const published = { ...run, resultJson: { presentationDecision: { commentId: "selected" } } }; + const bubble: TaskReplyObservation = { ...observed, anchor: "selected", origin: "https://fixture.test", pageUrl: "https://fixture.test/QA/issues/QA-123", + body: "I couldn't retrieve the pages. See QA-123.", + content: [{ text: "I couldn't retrieve the pages. See " }, { href: "https://fixture.test/QA/issues/QA-123#document-outcome", label: "QA-123", decorated: true }, { text: "." }] }; + expect(taskReplyBelongsToRun(bubble, published, "owned-agent", [comment])).toBe(true); + const copy = events(); copy[0]!.payload.prpEvent.payload.text = body; + expect(taskReplyBelongsToRun({ ...bubble, anchor: "final-run:turn" }, run, "owned-agent", [], copy)).toBe(true); + expect(taskReplyBelongsToRun({ ...bubble, body: "Different visible response" }, published, "owned-agent", [comment])).toBe(false); + for (const content of [undefined, [{ text: bubble.body }], [...bubble.content!, { text: " extra" }], + [{ text: "I couldn't retrieve the pages. See " }, { href: "/other-task", label: "QA-123" }, { text: "." }]]) + expect(taskReplyBelongsToRun({ ...bubble, content }, published, "owned-agent", [comment])).toBe(false); +}); diff --git a/tests/runner-e2e/native-task-reply.ts b/tests/runner-e2e/native-task-reply.ts index afe678b4732..59d11f1a178 100644 --- a/tests/runner-e2e/native-task-reply.ts +++ b/tests/runner-e2e/native-task-reply.ts @@ -1,9 +1,11 @@ import { expect, type Page } from "@playwright/test"; import { isValidNativePrpEnvelope } from "./native-event-envelope.js"; +import { taskReplyContentMatches, type ReplyContentPart } from "./task-reply-content.js"; type Row = Record; export interface TaskReplyObservation { body: string; anchor: string | null; settled: boolean; nativeFinal: boolean; visible: boolean; + content?: ReplyContentPart[]; origin?: string; pageUrl?: string; knownCompanyPrefixes?: string[]; } export type BoundTaskFinalReply = Awaited>; @@ -36,10 +38,10 @@ export function taskReplyBelongsToRun(observation: TaskReplyObservation, run: Ro const projected = typeof decision?.commentId === "string" ? comments.find(comment => comment.id === decision.commentId) : null; if (projected) { return observation.anchor === projected.id && projected.authorAgentId === agentId - && projected.createdByRunId === run.id && typeof projected.body === "string" && projected.body.trim() === observation.body; + && projected.createdByRunId === run.id && typeof projected.body === "string" && taskReplyContentMatches(observation, projected.body); } const finals = nativeFinalReplyEvents(events, run); - return finals.length === 1 && finals[0]!.body === observation.body && observation.settled && observation.nativeFinal + return finals.length === 1 && taskReplyContentMatches(observation, finals[0]!.body) && observation.settled && observation.nativeFinal && (observation.anchor === `${run.id}:turn` || observation.anchor?.startsWith(`${run.id}:turn:`) === true); } @@ -50,23 +52,51 @@ export function uniqueBoundTaskReply(all: TaskReplyObservation[], run: Row, agen ? matches[0]! : null; } +export function observeTaskReplyElements(elements: Array): TaskReplyObservation[] { + return elements.map(element => { + const content: Array<{ text: string } | { href: string; label: string; decorated: boolean } | { checked: boolean }> = []; + const nodes: Array = [element]; + while (nodes.length) { + const node = nodes.pop()!; + if (typeof node === "string") { content.push({ text: node }); continue; } + if (node.nodeType === Node.TEXT_NODE) { content.push({ text: node.textContent ?? "" }); continue; } + if (!(node instanceof HTMLElement) || node.getAttribute("aria-hidden") === "true" + || getComputedStyle(node).display === "none" || getComputedStyle(node).visibility === "hidden") continue; + if (node instanceof HTMLInputElement && node.type === "checkbox") { + if (node.getClientRects().length) content.push({ checked: node.checked }); + continue; + } + if (node instanceof HTMLAnchorElement) { + if (node.getClientRects().length && node.innerText.trim()) content.push({ href: node.href, label: node.innerText, + decorated: node.matches('[data-mention-kind="issue"], [data-sidebar-document-link]') }); + continue; + } + const block = ["P", "DIV", "BR", "LI", "PRE", "BLOCKQUOTE", "H1", "H2", "H3", "H4", "H5", "H6"].includes(node.tagName); + if (block) { content.push({ text: " " }); nodes.push(" "); } + for (let index = node.childNodes.length - 1; index >= 0; index--) nodes.push(node.childNodes[index]!); + } + return { + body: (element as HTMLElement).innerText.trim(), + content, origin: location.origin, pageUrl: location.href, + anchor: element.closest("[data-thread-anchor]")?.getAttribute("data-thread-anchor") ?? null, + settled: element.closest('[data-testid="task-chat-turn"]')?.getAttribute("data-settled") === "true", + nativeFinal: element.closest('[data-testid="task-chat-final-response"]') !== null, + visible: element.getClientRects().length > 0 && getComputedStyle(element).visibility !== "hidden" && getComputedStyle(element).display !== "none", + }; + }); +} + /** Observe the real task thread after a fresh load, rather than a finish summary * or a provider's claimed result. The caller separately validates task/run scope. */ export async function readBoundTaskFinalReply(input: { - page: Page; runId: string; agentId: string; expectedText?: string; deadlineAt: number; + page: Page; runId: string; agentId: string; expectedText?: string; deadlineAt: number; knownCompanyPrefixes: string[]; load(): Promise<{ run: Row; comments: Row[]; events: Row[] }>; }) { await input.page.reload(); const replies = input.page.getByTestId("task-chat-thread").getByTestId("task-chat-agent-bubble"); - const observe = () => replies.evaluateAll(elements => elements.map(element => ({ - body: (element as HTMLElement).innerText.trim(), - anchor: element.closest("[data-thread-anchor]")?.getAttribute("data-thread-anchor") ?? null, - settled: element.closest('[data-testid="task-chat-turn"]')?.getAttribute("data-settled") === "true", - nativeFinal: element.closest('[data-testid="task-chat-final-response"]') !== null, - visible: element.getClientRects().length > 0 && getComputedStyle(element).visibility !== "hidden" && getComputedStyle(element).display !== "none", - }))); + const observe = () => replies.evaluateAll(observeTaskReplyElements); const read = async () => { - const state = await input.load(), all = await observe(); + const state = await input.load(), all = (await observe()).map(row => ({ ...row, knownCompanyPrefixes: input.knownCompanyPrefixes })); const match = state.run.id === input.runId ? uniqueBoundTaskReply(all, state.run, input.agentId, state.comments, input.expectedText, state.events) : null; return { ...state, match }; }; @@ -75,9 +105,12 @@ export async function readBoundTaskFinalReply(input: { await expect.poll(async () => { return Boolean((await read()).match); }, { timeout, intervals: [1000, 2000, 5000] }).toBe(true); - const { match, run, events } = await read(); + const { match, run, events, comments } = await read(); if (!match) throw new Error("native_reply_changed"); const source = match.anchor === run.resultJson?.presentationDecision?.commentId ? "posted-comment" : "native-turn"; + const finalEvent = source === "native-turn" ? nativeFinalReplyEvents(events, run)[0] : null; + const sourceBody = source === "posted-comment" ? comments.find(comment => comment.id === match.anchor)?.body : finalEvent?.body; + if (typeof sourceBody !== "string" || !taskReplyContentMatches(match, sourceBody)) throw new Error("native_reply_source_changed"); return { runId: input.runId, agentId: input.agentId, observedAfterReload: true, - source, finalEvent: source === "native-turn" ? nativeFinalReplyEvents(events, run)[0] : null, ...match }; + source, sourceBody, finalEvent, ...match }; } diff --git a/tests/runner-e2e/playwright-support.config.ts b/tests/runner-e2e/playwright-support.config.ts index d384b7a7fc9..8d2bd94f902 100644 --- a/tests/runner-e2e/playwright-support.config.ts +++ b/tests/runner-e2e/playwright-support.config.ts @@ -3,7 +3,7 @@ import { defineConfig } from "@playwright/test"; /** Browser-only harness regressions: no Paperclip instance or provider credentials. */ export default defineConfig({ testDir: ".", - testMatch: ["screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts", "github-review-check.spec.ts", "antigravity-journey-browser.spec.ts"], + testMatch: ["native-task-reply.spec.ts", "screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts", "github-review-check.spec.ts", "antigravity-journey-browser.spec.ts"], workers: 1, retries: 0, timeout: 10_000, diff --git a/tests/runner-e2e/product-journeys/agent-flows.ts b/tests/runner-e2e/product-journeys/agent-flows.ts index d2daa20c5f9..57a7c772565 100644 --- a/tests/runner-e2e/product-journeys/agent-flows.ts +++ b/tests/runner-e2e/product-journeys/agent-flows.ts @@ -97,6 +97,9 @@ export async function runAgentJourney(id: AgentJourney, context: JourneyContext, const runs = state.taskRuns.filter(run => run.status === "succeeded"); if (!runs.length || runs.some(run => !Number.isFinite(Date.parse(run.finishedAt)))) throw new JourneyError("failed", "final_run_time_missing"); const run = [...runs].sort((a, b) => Date.parse(a.finishedAt) - Date.parse(b.finishedAt)).at(-1)!; + const companies = await context.api.get("/api/companies?scope=accessible"); + if (!Array.isArray(companies) || !companies.some(row => row.id === context.company.id) + || companies.some(row => typeof row.issuePrefix !== "string" || !row.issuePrefix)) throw new JourneyError("failed", "reply_company_context_missing"); await open(issue); const receipt = await readBoundTaskFinalReply({ page: context.page, runId: run.id, agentId: agent.id, load: async () => { @@ -104,7 +107,7 @@ export async function runAgentJourney(id: AgentJourney, context: JourneyContext, context.api.get(`/api/heartbeat-runs/${run.id}/events?afterSeq=${afterSeq}&limit=${limit}`))]); return { run: latest.taskRuns.find(row => row.id === run.id)!, comments: latest.comments, events }; }, - expectedText: marker, deadlineAt: options.deadlineAt }); + expectedText: marker, deadlineAt: options.deadlineAt, knownCompanyPrefixes: companies.map(row => row.issuePrefix) }); await retain(`${issue.id}-final-reply.json`, receipt); return receipt; }; diff --git a/tests/runner-e2e/task-reply-content.test.ts b/tests/runner-e2e/task-reply-content.test.ts new file mode 100644 index 00000000000..36ecbe89cfa --- /dev/null +++ b/tests/runner-e2e/task-reply-content.test.ts @@ -0,0 +1,41 @@ +import { expect, it } from "vitest"; +import { taskReplyContentMatches } from "./task-reply-content.js"; + +it("parses CommonMark escapes, entities, inline formatting and paragraph boundaries", () => { + expect(taskReplyContentMatches({ body: "Visible" }, "~~Visible~~")).toBe(true); + expect(taskReplyContentMatches({ body: "Couldn't retrieve a_b & c. No retry." }, + "**Couldn't retrieve** `a_b` & c.\n\nNo _retry_.")).toBe(true); + expect(taskReplyContentMatches({ body: "Couldn't retrieve a_b & c. No retry. Extra." }, + "**Couldn't retrieve** `a_b` & c.\n\nNo _retry_.")).toBe(false); +}); + +it("rejects unsupported Markdown rather than stripping unknown or invisible content", () => { + for (const source of ["Visible ", "Visible ![image](https://fixture.test/image.png)", "|Column|\n|---|\n|Visible|"]) + expect(taskReplyContentMatches({ body: "Visible" }, source)).toBe(false); +}); + +it("matches generated GFM checkboxes and rejects changed check states", () => { + const observation = { body: "Finished Next", content: [{ checked: true }, { text: " Finished " }, { checked: false }, { text: " Next" }] }; + expect(taskReplyContentMatches(observation, "- [x] Finished\n- [ ] Next")).toBe(true); + expect(taskReplyContentMatches(observation, "- [ ] Finished\n- [ ] Next")).toBe(false); +}); + +it("uses production GFM autolinks and issue linking with the viewer's known companies", () => { + const context = { pageUrl: "https://fixture.test/QA/issues/QA-1", knownCompanyPrefixes: ["QA"] }; + expect(taskReplyContentMatches({ ...context, body: "Depends on QA-123.", content: [{ text: "Depends on " }, + { href: "/QA/issues/QA-123", label: "QA-123", decorated: true }, { text: "." }] }, "Depends on QA-123.")).toBe(true); + expect(taskReplyContentMatches({ ...context, body: "See https://example.com/", content: [{ text: "See " }, + { href: "https://example.com/", label: "https://example.com/" }] }, "See https://example.com/")).toBe(true); + expect(taskReplyContentMatches({ ...context, body: "Depends on TREE-604." }, "Depends on TREE-604.")).toBe(true); +}); + +it("uses the production company prefix and current task URL without accepting a changed target", () => { + const observation = { body: "Read Outcome", pageUrl: "https://fixture.test/QA/issues/QA-123", + content: [{ text: "Read " }, { href: "/QA/issues/QA-123#document-outcome", label: "Outcome" }] }; + for (const href of ["/issues/QA-123#document-outcome", "#document-outcome"]) + expect(taskReplyContentMatches(observation, `Read [Outcome](${href})`)).toBe(true); + for (const href of ["/QA/issues/QA-999#document-outcome", "#document-other"]) + expect(taskReplyContentMatches(observation, `Read [Outcome](${href})`)).toBe(false); + expect(taskReplyContentMatches({ ...observation, content: [{ text: "Read " }, + { href: "/OTHER/issues/QA-123#document-outcome", label: "Outcome" }] }, "Read [Outcome](/issues/QA-123#document-outcome)")).toBe(false); +}); diff --git a/tests/runner-e2e/task-reply-content.ts b/tests/runner-e2e/task-reply-content.ts new file mode 100644 index 00000000000..d3d6ce9f5e0 --- /dev/null +++ b/tests/runner-e2e/task-reply-content.ts @@ -0,0 +1,57 @@ +import { markdownReplyContent } from "../../ui/src/lib/markdown-reply-content.js"; +import { applyCompanyPrefix, extractCompanyPrefixFromPath } from "../../ui/src/lib/company-routes.js"; +import { parseIssueReferenceFromHref } from "../../ui/src/lib/issue-reference.js"; + +export type ReplyContentPart = { text: string } | { href: string; label?: string; decorated?: boolean } | { checked: boolean }; + +function normalized(parts: ReplyContentPart[], pageUrl: string, source = false) { + const result: ReplyContentPart[] = []; + for (const part of parts) { + if ("checked" in part) { + if (typeof part.checked !== "boolean") return null; + result.push({ checked: part.checked }); + } else if ("href" in part) { + try { + const issueRef = source ? parseIssueReferenceFromHref(part.href) : null; + const href = issueRef ? applyCompanyPrefix(issueRef.href, extractCompanyPrefixFromPath(new URL(pageUrl).pathname)) : part.href; + result.push({ href: new URL(href, pageUrl).href, label: part.label?.replace(/\s+/g, " ").trim(), decorated: part.decorated }); + } catch { return null; } + } else { + const text = part.text.replace(/\s+/g, " "); + const previous = result.at(-1); + if (previous && "text" in previous) previous.text = (previous.text + text).replace(/\s+/g, " "); + else if (text) result.push({ text }); + } + } + const first = result[0], last = result.at(-1); + if (first && "text" in first) first.text = first.text.trimStart(); + if (last && "text" in last) last.text = last.text.trimEnd(); + return result.filter(part => !("text" in part) || part.text); +} + +/** Compare complete message content while allowing the UI to decorate a link + * with an issue/document chip. Its exact destination still has to match. + * Unsupported Markdown fails closed rather than stripping arbitrary syntax. */ +export function taskReplyContentMatches( + observation: { body: string; content?: ReplyContentPart[]; origin?: string; pageUrl?: string; knownCompanyPrefixes?: string[] }, + markdown: string, +) { + if (observation.content) { + // Bind the structured projection to the independently observed innerText. + // A destination alone cannot attest to arbitrary visible wording. + if (observation.content.some(part => "href" in part && (typeof part.label !== "string" || !part.label.trim()))) return false; + const displayed = observation.content.map(part => "text" in part ? part.text : "href" in part ? part.label! : "").join("").replace(/\s+/g, " ").trim(); + if (displayed !== observation.body.replace(/\s+/g, " ").trim()) return false; + } + const parts = markdownReplyContent(markdown, observation.knownCompanyPrefixes); + if (!parts) return false; + const pageUrl = observation.pageUrl ?? observation.origin ?? "https://reply.invalid"; + const expected = normalized(parts, pageUrl, true); + const actual = normalized(observation.content ?? [{ text: observation.body }], pageUrl); + return expected !== null && actual !== null && expected.length === actual.length && expected.every((part, index) => { + const displayed = actual[index]!; + return "text" in part ? "text" in displayed && part.text === displayed.text + : "href" in part ? "href" in displayed && part.href === displayed.href && (part.label === displayed.label || displayed.decorated === true) + : "checked" in displayed && part.checked === displayed.checked; + }); +} diff --git a/ui/src/lib/markdown-reply-content.ts b/ui/src/lib/markdown-reply-content.ts new file mode 100644 index 00000000000..67c70f1b338 --- /dev/null +++ b/ui/src/lib/markdown-reply-content.ts @@ -0,0 +1,56 @@ +import { Fragment, isValidElement, type ReactNode } from "react"; +import Markdown from "react-markdown"; +import remarkGfm from "remark-gfm"; +import { remarkLinkIssueReferences } from "./issue-reference.js"; + +export type MarkdownReplyPart = { text: string } | { href: string; label: string } | { checked: boolean }; +type AstNode = { type: string; children?: AstNode[] }; + +/** The reply observer consumes the UI's existing parser without adding a + * parallel Markdown dependency graph. Unsupported structures fail closed. */ +export function markdownReplyContent(markdown: string, knownPrefixes?: string[]): MarkdownReplyPart[] | null { + let rawHtml = false; + function rejectHtml() { + return (tree: AstNode) => { + const nodes = [tree]; + while (nodes.length) { + const node = nodes.pop()!; + if (node.type === "html") rawHtml = true; + nodes.push(...node.children ?? []); + } + }; + } + const rendered = Markdown({ children: markdown, + remarkPlugins: [remarkGfm, [remarkLinkIssueReferences, { knownPrefixes }], rejectHtml] }); + if (rawHtml) return null; + const parts: MarkdownReplyPart[] = []; + function visit(node: ReactNode, label?: string[]): boolean { + if (node == null || typeof node === "boolean") return true; + if (typeof node === "string" || typeof node === "number") { + if (label) label.push(String(node)); else parts.push({ text: String(node) }); + return true; + } + if (Array.isArray(node)) return node.every(child => visit(child, label)); + if (!isValidElement<{ children?: ReactNode; href?: string; type?: string; checked?: boolean; disabled?: boolean }>(node)) return false; + if (node.type === Fragment) return visit(node.props.children, label); + if (!label && node.type === "input" && node.props.type === "checkbox" && node.props.disabled === true) { + parts.push({ checked: node.props.checked === true }); + return true; + } + if (node.type === "a" && !label && node.props.href) { + const words: string[] = []; + if (!visit(node.props.children, words)) return false; + parts.push({ href: node.props.href, label: words.join("") }); + return true; + } + if (node.type === "br") { if (label) label.push(" "); else parts.push({ text: " " }); return true; } + if (["strong", "em", "del", "code"].includes(String(node.type))) return visit(node.props.children, label); + if (!label && ["p", "ul", "ol", "li", "blockquote", "h1", "h2", "h3", "h4", "h5", "h6"].includes(String(node.type))) { + if (!visit(node.props.children)) return false; + parts.push({ text: " " }); + return true; + } + return false; + } + return visit(rendered) ? parts : null; +}