diff --git a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts index c9808786052..f04ca959a30 100644 --- a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts +++ b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts @@ -3062,6 +3062,45 @@ describe("OpenCodeServerDriver", () => { await session.close({ reason: "test" }); }); + it("saves an answer once when the model restates it after the completion call", async () => { + await chmod(fixture, 0o755); + const root = await mkdtemp(join(tmpdir(), "paperclip-opencode-driver-")); + const workspace = await mkdtemp( + join(tmpdir(), "paperclip-opencode-workspace-"), + ); + roots.push(root, workspace); + const driver = new OpenCodeServerDriver({ + model: "openrouter/deepseek/deepseek-v4-flash-0731", + runtimeDirectory: root, + command: fixture, + environment: { + PATH: process.env.PATH, + OPENROUTER_API_KEY: "fixture-key", + }, + }); + const session = await driver.openSession({ + runId: "run-answer-then-restatement", + normalizedSessionId: "answer-then-restatement", + workingDirectory: workspace, + }); + await session.startTurn({ + message: { role: "user", text: "answer-before-finish-then-restatement" }, + }); + const events = await collectTurnEvents(session.events()); + const finalMessages = events.filter( + (event) => + event.eventType === "item.completed" && + event.payload.kind === "agentMessage" && + event.payload.channel === "final", + ); + expect(finalMessages).toHaveLength(1); + expect(finalMessages[0]?.payload.text).toBe( + "I can't change agent budgets from chat — that's board-only, and agent credentials get 403. Two agents also match \"Morgan,\" so choose the intended one and set $50/month on its budget tab: [Morgan budgets](/COM/agents/agent-morgan/budgets) or [Morgan Research budgets](/COM/agents/agent-morgan-research/budgets).", + ); + expect(finalMessages[0]?.itemId).toBe("part-reference"); + await session.close({ reason: "test" }); + }); + it.each(["session.idle", "session.status", "session.error", "aborted"])("settles accepted completion before a racing %s event", async terminalType => { await chmod(fixture, 0o755); const root = await mkdtemp(join(tmpdir(), "paperclip-opencode-completion-race-")); diff --git a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts index b8177f6ef8d..4ccf82c8df1 100644 --- a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts +++ b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts @@ -23,6 +23,7 @@ import { getCACertificates } from "node:tls"; import { pipeline } from "node:stream/promises"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { restatesEarlierReply } from "./restated-reply.js"; import { parseOpenCodeReasoningMode, type OpenCodeReasoningMode } from "./reasoning-mode.js"; import { attributeUnsupportedToolMedia } from "./tool-media-attribution.js"; import { openCodeProviderFailure } from "./provider-failure.js"; @@ -2301,12 +2302,17 @@ class OpenCodeHarnessSession implements HarnessSession { // before that call: the person saw both, and the later message often just // refers back ("Answered above"). Structure cannot tell an answer from // commentary, so keep every eligible part rather than drop the answer. - const preface = + const candidatePreface = this.#semanticResultProviderMessageId !== null && correlatedAfterResult.length === 0 && uncorrelatedAfterResult[0]?.part === selected ? beforeResult.map(({ part }) => part) : []; + const preface = + candidatePreface.length && + restatesEarlierReply(candidatePreface.map((part) => part.text.trim()).join("\n\n"), selected.text) + ? [] + : candidatePreface; const text = preface.length ? [...preface, selected].map((part) => part.text.trim()).filter(Boolean).join("\n\n") : selected.text; diff --git a/packages/paperclip-runner/src/drivers/opencode/restated-reply.test.ts b/packages/paperclip-runner/src/drivers/opencode/restated-reply.test.ts new file mode 100644 index 00000000000..116cda28980 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/opencode/restated-reply.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from "vitest"; +import { restatesEarlierReply } from "./restated-reply.js"; + +const recordedBefore = + "I can't set this from chat — changing an agent's monthly budget is a board-only action (agent credentials get 403). There are also two matching agents, so pick the intended one in the web app and set its monthly limit to $50 on the budget tab: [Morgan budgets](/COM/agents/a1/budgets) or [Morgan Research budgets](/COM/agents/a2/budgets). Tell me which Morgan you meant and I'll track the right one."; +const recordedAfter = + "I can't change agent budgets from chat — that's board-only, and agent credentials get 403. Two agents also match \"Morgan,\" so choose the intended one and set $50/month on its budget tab: [Morgan budgets](/COM/agents/a1/budgets) or [Morgan Research budgets](/COM/agents/a2/budgets)."; + +describe("restatesEarlierReply", () => { + it("treats an identical later message as a restatement", () => { + expect(restatesEarlierReply(recordedBefore, `${recordedBefore}\n`)).toBe(true); + }); + + it("treats the recorded reworded answer as a restatement", () => { + expect(restatesEarlierReply(recordedBefore, recordedAfter)).toBe(true); + }); + + it("keeps an answer whose later message only refers back to it", () => { + expect( + restatesEarlierReply( + "Two inbox items need you: [COM-2](/COM/issues/COM-2) is blocked and [COM-1](/COM/issues/COM-1) is waiting for review.", + "Answered above: two inbox items need your attention.", + ), + ).toBe(false); + }); + + it("keeps an earlier link the later message drops", () => { + expect( + restatesEarlierReply( + recordedBefore, + recordedAfter.replace(" or [Morgan Research budgets](/COM/agents/a2/budgets)", " or Morgan Research's budget tab"), + ), + ).toBe(false); + }); + + it("keeps an earlier code block the later message drops", () => { + expect( + restatesEarlierReply( + "Run this in your terminal:\n\n```shell\nnpx paperclipai auth login\n```\n\nThen confirm with whoami.", + "Run the sign-in command in your terminal, then confirm with whoami before any write.", + ), + ).toBe(false); + }); + + const reportBefore = "The full launch report is ready at https://example.com/reports/launch and covers the signup funnel, the pricing page and the onboarding emails for review."; + const reportAfter = "The launch report is ready and covers the signup funnel, the pricing page and the onboarding emails, so you can review it."; + + it("keeps an earlier bare URL the later message drops", () => { + expect(restatesEarlierReply(reportBefore, reportAfter)).toBe(false); + }); + + it("treats a later message that keeps the bare URL as a restatement", () => { + expect(restatesEarlierReply(reportBefore, `${reportAfter} Link: https://example.com/reports/launch.`)).toBe(true); + }); + + it("keeps an earlier autolink the later message drops", () => { + expect( + restatesEarlierReply(reportBefore.replace("https://example.com/reports/launch", ""), reportAfter), + ).toBe(false); + }); + + it("keeps an earlier reference-style link the later message drops", () => { + expect( + restatesEarlierReply( + "The full launch [report][launch] is ready and covers the signup funnel, the pricing page and the onboarding emails for review.\n\n[launch]: /COM/issues/COM-7#document-report", + reportAfter, + ), + ).toBe(false); + }); + + const codeAnswer = (code: string) => + `Run this script to check the export before you send it to the team:\n\n\`\`\`python\n${code}\n\`\`\`\n\nIt prints the row count and exits.`; + + it("keeps an earlier code block whose string contents differ only in spacing", () => { + expect(restatesEarlierReply(codeAnswer('print("a b")'), codeAnswer('print("a b")'))).toBe(false); + }); + + it("keeps an earlier code block whose indentation differs", () => { + expect( + restatesEarlierReply( + codeAnswer("for row in rows:\n if row:\n print(row)"), + codeAnswer("for row in rows:\n if row:\n print(row)"), + ), + ).toBe(false); + }); + + it("treats a later message with the same code block as a restatement", () => { + const code = "for row in rows:\n print(row)"; + expect(restatesEarlierReply(codeAnswer(code), codeAnswer(code).replace("Run this script", "Use this script"))).toBe(true); + }); + + it("keeps earlier prose that says something different", () => { + expect( + restatesEarlierReply( + "The deploy failed because the database migration timed out after 30 seconds on the orders table.", + "I opened a follow-up task for Morgan to split the migration and will report back once it lands.", + ), + ).toBe(false); + }); +}); diff --git a/packages/paperclip-runner/src/drivers/opencode/restated-reply.ts b/packages/paperclip-runner/src/drivers/opencode/restated-reply.ts new file mode 100644 index 00000000000..8f4225b1466 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/opencode/restated-reply.ts @@ -0,0 +1,43 @@ +export function restatesEarlierReply(earlier: string, later: string): boolean { + const before = earlier.trim(); + const after = later.trim(); + if (!before || !after) return false; + if (before === after) return true; + if (after.length * 2 < before.length) return false; + const laterTargets = linkTargets(after); + if (![...linkTargets(before)].every((target) => laterTargets.has(target))) return false; + const laterBlocks = new Set(fencedBlocks(after)); + if (!fencedBlocks(before).every((block) => laterBlocks.has(block))) return false; + const earlierWords = proseWords(before); + if (earlierWords.size === 0) return true; + const laterWords = proseWords(after); + let repeated = 0; + for (const word of earlierWords) if (laterWords.has(word)) repeated += 1; + return repeated * 2 >= earlierWords.size; +} + +const INLINE_LINK_TARGET = /\]\(\s*]+)>?(?:\s+"[^"]*")?\s*\)/g; +const REFERENCE_DEFINITION = /^[ \t]{0,3}\[[^\]\n]+\]:[ \t]*]+)>?/gm; +const AUTOLINK = /<([a-z][a-z\d+.-]*:[^\s<>]+)>/gi; +const BARE_URL = /\b(?:https?|ftp|mailto):[^\s<>()[\]`"']+/gi; +const FENCED_BLOCK = /```[^\n]*\n([\s\S]*?)```/g; + +function linkTargets(text: string): Set { + const targets = new Set(); + for (const pattern of [INLINE_LINK_TARGET, REFERENCE_DEFINITION, AUTOLINK]) { + for (const match of text.matchAll(pattern)) targets.add(match[1]!); + } + for (const match of text.replace(FENCED_BLOCK, " ").matchAll(BARE_URL)) { + targets.add(match[0].replace(/[.,;:!?]+$/, "")); + } + return targets; +} + +function fencedBlocks(text: string): string[] { + return [...text.matchAll(FENCED_BLOCK)].map((match) => match[1]!.replace(/\n$/, "")); +} + +function proseWords(text: string): Set { + const prose = text.replace(FENCED_BLOCK, " ").replace(INLINE_LINK_TARGET, "]"); + return new Set(prose.toLowerCase().match(/[\p{L}\p{N}]+/gu) ?? []); +} diff --git a/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs b/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs index f142d2880de..346e305899e 100644 --- a/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs +++ b/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs @@ -800,6 +800,15 @@ const server = createServer(async (request, response) => { const answerBeforeFinishThenReference = String( parsedPrompt.message ?? "", ).includes("answer-before-finish-then-reference"); + const answerBeforeFinishThenRestatement = String( + parsedPrompt.message ?? "", + ).includes("answer-before-finish-then-restatement"); + const answerBeforeFinishText = answerBeforeFinishThenRestatement + ? "I can't set this from chat — changing an agent's monthly budget is a board-only action (agent credentials get 403). There are also two matching agents, so pick the intended one in the web app and set its monthly limit to $50 on the budget tab: [Morgan budgets](/COM/agents/agent-morgan/budgets) or [Morgan Research budgets](/COM/agents/agent-morgan-research/budgets). Tell me which Morgan you meant and I'll track the right one." + : "Two inbox items need you: [COM-2](/COM/issues/COM-2) is blocked and [COM-1](/COM/issues/COM-1) is waiting for review."; + const answerAfterFinishText = answerBeforeFinishThenRestatement + ? "I can't change agent budgets from chat — that's board-only, and agent credentials get 403. Two agents also match \"Morgan,\" so choose the intended one and set $50/month on its budget tab: [Morgan budgets](/COM/agents/agent-morgan/budgets) or [Morgan Research budgets](/COM/agents/agent-morgan-research/budgets)." + : "Answered above: two inbox items need your attention."; const finalAfterToolCommentary = String( parsedPrompt.message ?? "", ).includes("final-after-tool-commentary"); @@ -1001,7 +1010,7 @@ const server = createServer(async (request, response) => { }, }, }); - } else if (answerBeforeFinishThenReference) { + } else if (answerBeforeFinishThenReference || answerBeforeFinishThenRestatement) { emit({ type: "message.part.updated", id: "event-answer-before-finish", @@ -1011,7 +1020,7 @@ const server = createServer(async (request, response) => { id: "part-answer-before-finish", messageID: "message-assistant", type: "text", - text: "Two inbox items need you: [COM-2](/COM/issues/COM-2) is blocked and [COM-1](/COM/issues/COM-1) is waiting for review.", + text: answerBeforeFinishText, time: { start: 1, end: 2 }, }, }, @@ -1052,7 +1061,7 @@ const server = createServer(async (request, response) => { id: "part-reference", messageID: "message-reference", type: "text", - text: "Answered above: two inbox items need your attention.", + text: answerAfterFinishText, time: { start: 3, end: 4 }, }, },