mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
Preserve ordinary link labels and production route semantics
Restrict renamed labels to existing product chips. Reuse the company routing helper and current task URL, with browser regressions for prefixes, fragments and wrong task/company targets. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
97048b0153
commit
7aa94ff745
7 files changed
+56
-17
No files matched your search
@@ -118,6 +118,8 @@ thread anchor and observed text; classic protocol comment matchers stay strict.
|
||||
Markdown replies compare complete text and exact link destinations after parsing
|
||||
CommonMark. 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 constructs
|
||||
such as images, raw HTML and GFM-only formatting fail closed. Exact marker checks
|
||||
remain exact, and decline checks inspect both visible explanations and source
|
||||
|
||||
@@ -66,7 +66,7 @@ describe("neutral decline evidence", () => {
|
||||
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",
|
||||
content: [{ text: "Contacts are unavailable. See " }, { href: "/QA/issues/QA-123#document-outcome", label: "QA-123" }, { text: "." }] });
|
||||
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") });
|
||||
|
||||
@@ -2,13 +2,15 @@ import { expect, test } from "@playwright/test";
|
||||
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 <strong>couldn't retrieve</strong> the pages. See <a href='/QA/issues/QA-123#document-outcome'><svg aria-hidden='true'></svg>QA-123</a>.";
|
||||
const rendered = "I <strong>couldn't retrieve</strong> the pages. See <a data-mention-kind='issue' href='/QA/issues/QA-123#document-outcome'><svg aria-hidden='true'></svg>QA-123</a>.";
|
||||
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 };
|
||||
|
||||
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</a>", ">Connection outcome</a>"), true],
|
||||
["renamed ordinary link", rendered.replace(" data-mention-kind='issue'", ""), false],
|
||||
["wrong destination", rendered.replace("#document-outcome", "#document-other"), false],
|
||||
["missing link", rendered.replace(/<a.*?<\/a>/, "QA-123"), false],
|
||||
["extra visible text", rendered + " Extra.", false],
|
||||
@@ -23,7 +25,23 @@ for (const [name, content, passes] of [
|
||||
const receipt = await result;
|
||||
expect(receipt.source).toBe("posted-comment");
|
||||
expect(receipt.sourceBody).toBe(markdown);
|
||||
expect(receipt.body).toBe("I couldn't retrieve the pages. See QA-123.");
|
||||
expect(receipt.body).toBe(content.includes(">Connection outcome</a>") ? "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],
|
||||
] 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:
|
||||
`<div data-testid='task-chat-thread'><div data-thread-anchor='selected'><div data-testid='task-chat-agent-bubble'>Read <a href='${renderedHref}'>Outcome</a></div></div></div>` }));
|
||||
await page.goto(url);
|
||||
const result = readBoundTaskFinalReply({ page, runId: run.id, agentId: "agent", 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();
|
||||
});
|
||||
@@ -80,7 +80,7 @@ it("binds complete Markdown content and the exact link destination despite a dec
|
||||
const published = { ...run, resultJson: { presentationDecision: { commentId: "selected" } } };
|
||||
const bubble: TaskReplyObservation = { ...observed, anchor: "selected", origin: "https://fixture.test",
|
||||
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" }, { text: "." }] };
|
||||
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);
|
||||
|
||||
@@ -5,7 +5,7 @@ import { taskReplyContentMatches, type ReplyContentPart } from "./task-reply-con
|
||||
type Row = Record<string, any>;
|
||||
export interface TaskReplyObservation {
|
||||
body: string; anchor: string | null; settled: boolean; nativeFinal: boolean; visible: boolean;
|
||||
content?: ReplyContentPart[]; origin?: string;
|
||||
content?: ReplyContentPart[]; origin?: string; pageUrl?: string;
|
||||
}
|
||||
export type BoundTaskFinalReply = Omit<Awaited<ReturnType<typeof readBoundTaskFinalReply>>, "sourceBody"> & { sourceBody?: string };
|
||||
|
||||
@@ -54,13 +54,14 @@ export function uniqueBoundTaskReply(all: TaskReplyObservation[], run: Row, agen
|
||||
|
||||
export function observeTaskReplyElements(elements: Array<HTMLElement | SVGElement>): TaskReplyObservation[] {
|
||||
return elements.map(element => {
|
||||
const content: Array<{ text: string } | { href: string; label: string }> = [];
|
||||
const content: Array<{ text: string } | { href: string; label: string; decorated: boolean }> = [];
|
||||
const walk = (node: Node) => {
|
||||
if (node.nodeType === Node.TEXT_NODE) { content.push({ text: node.textContent ?? "" }); return; }
|
||||
if (!(node instanceof HTMLElement) || node.getAttribute("aria-hidden") === "true"
|
||||
|| getComputedStyle(node).display === "none" || getComputedStyle(node).visibility === "hidden") return;
|
||||
if (node instanceof HTMLAnchorElement) {
|
||||
if (node.getClientRects().length && node.innerText.trim()) content.push({ href: node.href, label: node.innerText });
|
||||
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]') });
|
||||
return;
|
||||
}
|
||||
const block = ["P", "DIV", "BR", "LI", "PRE", "BLOCKQUOTE", "H1", "H2", "H3", "H4", "H5", "H6"].includes(node.tagName);
|
||||
@@ -71,7 +72,7 @@ export function observeTaskReplyElements(elements: Array<HTMLElement | SVGElemen
|
||||
walk(element);
|
||||
return {
|
||||
body: (element as HTMLElement).innerText.trim(),
|
||||
content, origin: location.origin,
|
||||
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,
|
||||
|
||||
@@ -12,3 +12,12 @@ it("rejects unsupported Markdown rather than stripping unknown or invisible cont
|
||||
for (const source of ["Visible <!-- hidden -->", "Visible ", "~~Visible~~"])
|
||||
expect(taskReplyContentMatches({ body: "Visible" }, source)).toBe(false);
|
||||
});
|
||||
|
||||
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 ["/OTHER/issues/QA-123#document-outcome", "/QA/issues/QA-999#document-outcome", "#document-other"])
|
||||
expect(taskReplyContentMatches(observation, `Read [Outcome](${href})`)).toBe(false);
|
||||
});
|
||||
@@ -1,13 +1,17 @@
|
||||
import { fromMarkdown } from "mdast-util-from-markdown";
|
||||
import { applyCompanyPrefix, extractCompanyPrefixFromPath } from "../../ui/src/lib/company-routes.js";
|
||||
|
||||
export type ReplyContentPart = { text: string } | { href: string; label?: string };
|
||||
export type ReplyContentPart = { text: string } | { href: string; label?: string; decorated?: boolean };
|
||||
type MarkdownNode = { type: string; value?: string; url?: string; children?: MarkdownNode[] };
|
||||
|
||||
function normalized(parts: ReplyContentPart[], origin: string) {
|
||||
function normalized(parts: ReplyContentPart[], pageUrl: string, source = false) {
|
||||
const result: ReplyContentPart[] = [];
|
||||
for (const part of parts) {
|
||||
if ("href" in part) {
|
||||
try { result.push({ href: new URL(part.href, origin).href }); } catch { return null; }
|
||||
try {
|
||||
const href = source ? applyCompanyPrefix(part.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);
|
||||
@@ -25,7 +29,7 @@ function normalized(parts: ReplyContentPart[], origin: string) {
|
||||
* 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 },
|
||||
observation: { body: string; content?: ReplyContentPart[]; origin?: string; pageUrl?: string },
|
||||
markdown: string,
|
||||
) {
|
||||
if (observation.content) {
|
||||
@@ -38,18 +42,23 @@ export function taskReplyContentMatches(
|
||||
const parts: ReplyContentPart[] = [];
|
||||
const inlineLabel = (node: MarkdownNode): boolean => ["text", "inlineCode", "strong", "emphasis"].includes(node.type)
|
||||
&& (node.children ?? []).every(inlineLabel);
|
||||
const labelText = (node: MarkdownNode): string => node.value ?? (node.children ?? []).map(labelText).join("");
|
||||
function visit(node: MarkdownNode): boolean {
|
||||
if (node.type === "text" || node.type === "inlineCode") { parts.push({ text: node.value ?? "" }); return true; }
|
||||
if (node.type === "break") { parts.push({ text: " " }); return true; }
|
||||
if (node.type === "link" && node.url && (node.children ?? []).every(inlineLabel)) { parts.push({ href: node.url }); return true; }
|
||||
if (node.type === "link" && node.url && (node.children ?? []).every(inlineLabel)) { parts.push({ href: node.url, label: labelText(node) }); return true; }
|
||||
if (!["root", "paragraph", "heading", "blockquote", "list", "listItem", "strong", "emphasis"].includes(node.type)) return false;
|
||||
for (const child of node.children ?? []) if (!visit(child)) return false;
|
||||
if (!["strong", "emphasis"].includes(node.type)) parts.push({ text: " " });
|
||||
return true;
|
||||
}
|
||||
if (!visit(fromMarkdown(markdown) as MarkdownNode)) return false;
|
||||
const origin = observation.origin ?? "https://reply.invalid";
|
||||
const expected = normalized(parts, origin);
|
||||
const actual = normalized(observation.content ?? [{ text: observation.body }], origin);
|
||||
return expected !== null && actual !== null && JSON.stringify(expected) === JSON.stringify(actual);
|
||||
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 displayed && part.href === displayed.href && (part.label === displayed.label || displayed.decorated === true);
|
||||
});
|
||||
}
|
||||
Reference in new issue
Block a user