From 42c64d4ec4b9b0898556fcdf6ba304c4285a7819 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:07:32 -0600 Subject: [PATCH] fix(runner): preserve deliberate OpenCode approval waits (#18610) ## Thinking Path > - Paperclip coordinates agent work and human approval. > - A saved Plan must wait for the user before work continues. > - The native runner interrupts the provider after it saves that wait. > - OpenCode can report the interruption with an idle event and no final answer. > - Empty-answer recovery treated that intentional pause as an incomplete answer. > - This change stops answer recovery for an interrupted turn. > - The runner still requires a real provider terminal before it settles the wait. ## Linked Issues or Issue Description **What happened?** OpenCode could send a corrective model request after Paperclip interrupted a turn for a saved approval request. The native runner then failed with `governed_wait_provider_terminal_unproven` because it did not receive the required terminal event within the existing drain window. **Expected behavior** An interrupted turn must stop requesting answers. A provider idle or cancelled event must remain available to the native wait settlement. An HTTP abort acknowledgement alone must not count as a terminal. **Steps to reproduce** 1. Start an OpenCode turn that has no final answer. 2. Interrupt that turn after saving a durable approval request. 3. Have the provider return `session.idle` or an idle `session.status`. 4. Observe that the old driver sends a second prompt instead of emitting the terminal event. The controlled regression fails before the fix for both idle event forms: it observes two prompts instead of one. The affected driver blob is identical in the deployed source and the current base. Related: #18364 added the empty-answer recovery path. ## What Changed - Record interruption intent for the active OpenCode turn before asynchronous settlement. - Cancel its answer-recovery timer and prevent corrective requests for that turn. - Preserve provider terminal checks and avoid a delayed abort of a later turn. - Cover both idle event forms, an existing correction, durable question handoff, in-flight tools, missing terminal evidence, and a later authorized turn. - Document the interruption contract. ## Verification - The new regression failed before the fix for the expected extra prompt. A second red test confirmed the sibling durable-question handoff also sent that unwanted prompt. - Interrupted idle events now produce explicit cancellation terminals. Tests reject a failed terminal and require normal completion on the later authorized turn. - All 305 targeted tests passed across the OpenCode driver, normal empty-answer recovery, interruption, backend publication, and native session runtime suites. - Runner TypeScript check passed: `tsc -p packages/paperclip-runner/tsconfig.json --noEmit`. - `git diff --check` passed. - Greptile reviewed the current commit at 5/5 with no findings. Both earlier Paperclip review findings are fixed and their threads are resolved. - The additional Paperclip advisory reviewer could not assess the final commit because its earlier workspace save did not finish. Its check is neutral; this is not a passing assessment. No reviewer configuration or workspace safety gate was changed. - Repository CI passed on run 41470761437, attempt 2. Attempt 1 retained a 15-second timeout in an unchanged voice-report test; only the failed jobs were retried once, on the same commit. The identical test also passed in an isolated baseline check against disposable PostgreSQL with its original timeout. - Live QA acceptance is pending. No eval prompt, model, allowance, timeout, or assertion changed. ## Risks The change affects OpenCode interruption and answer recovery. A silent provider must remain unproven; the negative control checks that case. Ordinary answer recovery remains enabled on authorized turns. No schema, permission, or provider configuration changes are required. ## Model Used OpenAI GPT-6 through Codex, with reasoning, code execution, and browser tools. The runtime does not expose a more specific model identifier 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 either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [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 --- packages/paperclip-runner/README.md | 6 + .../opencode/interrupt-empty-response.test.ts | 124 ++++++++++++++++++ .../opencode/opencode-server-driver.ts | 33 +++-- 3 files changed, 150 insertions(+), 13 deletions(-) create mode 100644 packages/paperclip-runner/src/drivers/opencode/interrupt-empty-response.test.ts diff --git a/packages/paperclip-runner/README.md b/packages/paperclip-runner/README.md index 02271ebc583..a58ef9fb3cb 100644 --- a/packages/paperclip-runner/README.md +++ b/packages/paperclip-runner/README.md @@ -391,6 +391,12 @@ runner supplies the pinned executable, selected model, and assigned MCP tools. Provider inference and reasoning still contribute to time to first text. These startup settings do not disable model reasoning. +An interrupted OpenCode turn does not request a replacement final answer. +This also applies when an unanswered native question moves to a durable wait. +The driver reports an interrupted idle as a cancelled terminal so the wait can +settle. An abort acknowledgement alone does not prove that the provider stopped. +A later authorized turn can use normal answer recovery. + On macOS, the verified OpenCode executable keeps its private launch pathname until the provider process exits. Removing that pathname immediately after spawn can kill or stall the signed binary before its health check succeeds. diff --git a/packages/paperclip-runner/src/drivers/opencode/interrupt-empty-response.test.ts b/packages/paperclip-runner/src/drivers/opencode/interrupt-empty-response.test.ts new file mode 100644 index 00000000000..3462e9ea308 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/opencode/interrupt-empty-response.test.ts @@ -0,0 +1,124 @@ +import { chmod, copyFile, mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { expect, it, vi } from "vitest"; +import type { PrpEvent } from "../../protocol/replay-contract.js"; +import { OpenCodeServerDriver } from "./opencode-server-driver.js"; + +it.each(["idle", "status", "silent", "correcting-idle", "correcting-silent", "busy-idle", "handoff-idle", "handoff-status", "handoff-silent"] as const)("does not restart an interrupted empty OpenCode response: %s", async terminal => { + const root = await mkdtemp(join(tmpdir(), "opencode-interrupt-empty-")); + const command = join(root, "fake-opencode.mjs"); + await copyFile(resolve("test/fixtures/fake-opencode-server.mjs"), command); + await chmod(command, 0o755); + let stream: ReadableStreamDefaultController | undefined; + let streamOpened!: () => void; + const opened = new Promise(resolve => { streamOpened = resolve; }); + const prompts: unknown[] = []; + const observed: PrpEvent[] = []; + const correcting = terminal.startsWith("correcting-"); + const silent = terminal.endsWith("silent"); + const busy = terminal === "busy-idle"; + const handoff = terminal.startsWith("handoff-"); + let ownedSessionId = ""; + const emit = (value: unknown) => stream!.enqueue(new TextEncoder().encode(`data: ${JSON.stringify(value)}\n\n`)); + let resumed = false; + const driver = new OpenCodeServerDriver({ + model: "openrouter/controlled-model", + runtimeDirectory: join(root, "runtime"), command, + environment: { PATH: process.env.PATH, OPENROUTER_API_KEY: "controlled-fixture-key" }, + fetch: async (input, init) => { + const url = new URL(String(input)); + if (url.pathname === "/event") return new Response(new ReadableStream({ + start(controller) { stream = controller; streamOpened(); }, + }), { headers: { "Content-Type": "text/event-stream" } }); + const sessionID = decodeURIComponent(url.pathname.split("/")[2] ?? ""); + if (url.pathname.endsWith("/prompt_async")) { + ownedSessionId = sessionID; + prompts.push(JSON.parse(String(init?.body))); + if (resumed) queueMicrotask(() => { + emit({ type: "message.updated", properties: { info: { id: "resumed-answer", sessionID, role: "assistant", finish: "stop", time: { completed: 2 } } } }); + emit({ type: "message.part.updated", properties: { part: { id: "resumed-part", messageID: "resumed-answer", sessionID, type: "text", text: "The authorized next turn completed.", time: { start: 1, end: 2 } } } }); + emit({ type: "session.idle", properties: { sessionID } }); + }); + else if (correcting && prompts.length === 1) queueMicrotask(() => { + emit({ type: "session.idle", properties: { sessionID } }); + }); + else if (busy && prompts.length === 1) queueMicrotask(() => { + emit({ type: "message.updated", properties: { info: { id: "tool-message", sessionID, role: "assistant" } } }); + emit({ type: "message.part.updated", properties: { part: { id: "owned-tool", messageID: "tool-message", sessionID, + type: "tool", tool: "bash", callID: "owned-call", state: { status: "running", input: { command: "controlled work" } } } } }); + }); + else if (handoff && prompts.length === 1) queueMicrotask(() => { + emit({ type: "question.asked", properties: { id: "owned-question", sessionID, questions: [ + { question: "Which color should I use?", header: "Color", options: [{ label: "Blue", description: "Use blue." }] }, + ] } }); + }); + return new Response(null, { status: 204 }); + } + if (url.pathname.endsWith("/abort")) { + if (!silent) queueMicrotask(() => { + emit(!terminal.endsWith("status") + ? { type: "session.idle", properties: { sessionID } } + : { type: "session.status", properties: { sessionID, status: { type: "idle" } } }); + }); + return Response.json(true); + } + if (url.pathname === "/question/owned-question/reject") return Response.json(true); + return fetch(input, init); + }, + }); + const session = await driver.openSession({ runId: "owned-run", normalizedSessionId: "owned-session", workingDirectory: root }); + let collecting: Promise | undefined; + try { + await opened; + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + collecting = (async () => { for await (const event of session.events()) observed.push(event); })(); + const first = await session.startTurn({ message: { role: "user", text: "Save a plan and wait for approval." } }); + await vi.advanceTimersByTimeAsync(0); + const allowedPrompts = correcting ? 2 : 1; + expect(prompts).toHaveLength(allowedPrompts); + if (handoff) { + const pending = session.pendingRuntimeRequests?.(); + expect(pending).toHaveLength(1); + const result = session.handoffRuntimeRequest?.({ requestId: "owned-question", turnId: first.turnId, + reason: "durable_handoff", signal: new AbortController().signal }); + expect(result?.result).toBe("handed_off"); + await result?.cleanup; + expect(observed.filter(event => event.eventType === "runtime_request.expired")).toHaveLength(1); + } else await session.interrupt?.({ turnId: first.turnId, reason: "Durable approval is pending." }); + await vi.advanceTimersByTimeAsync(0); + // HTTP acknowledgement alone is not a provider terminal. An observed idle + // is a terminal, but must never authorize a new corrective model request. + expect(prompts).toHaveLength(allowedPrompts); + const terminalEvents = () => observed.filter(event => ["turn.completed", "turn.cancelled", "turn.failed"].includes(event.eventType)); + expect(terminalEvents()).toHaveLength(silent || busy ? 0 : 1); + if (!silent && !busy) expect(terminalEvents()[0]?.eventType).toBe("turn.cancelled"); + await vi.advanceTimersByTimeAsync(20_001); + expect(prompts).toHaveLength(allowedPrompts); + expect(terminalEvents()).toHaveLength(silent || busy ? 0 : 1); + if (busy) { + emit({ type: "message.part.updated", properties: { part: { id: "owned-tool", messageID: "tool-message", sessionID: ownedSessionId, + type: "tool", tool: "bash", callID: "owned-call", state: { status: "completed", input: { command: "controlled work" }, output: "Work stopped.", time: { start: 1, end: 2 } } } } }); + emit({ type: "session.idle", properties: { sessionID: ownedSessionId } }); + await vi.advanceTimersByTimeAsync(0); + expect(terminalEvents()).toHaveLength(1); + expect(terminalEvents()[0]?.eventType).toBe("turn.cancelled"); + expect(prompts).toHaveLength(allowedPrompts); + } + if (!silent) { + resumed = true; + const next = await session.startTurn({ message: { role: "user", text: "The plan is approved. Continue." } }); + await vi.advanceTimersByTimeAsync(0); + expect(prompts).toHaveLength(allowedPrompts + 1); + expect(terminalEvents().filter(event => event.turnId === next.turnId)).toHaveLength(1); + expect(terminalEvents().find(event => event.turnId === next.turnId)?.eventType).toBe("turn.completed"); + expect(observed.some(event => event.eventType === "item.completed" && event.payload.kind === "agentMessage" + && event.payload.channel === "final" && event.payload.text === "The authorized next turn completed.")).toBe(true); + } + } finally { + vi.useRealTimers(); + await session.close({ reason: "controlled test complete" }); + await collecting; + await rm(root, { recursive: true, force: true }); + } +}, 15_000); 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 e21e3af9ff9..01504933c1e 100644 --- a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts +++ b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts @@ -544,6 +544,7 @@ class OpenCodeHarnessSession implements HarnessSession { #turnReasoningMode: OpenCodeReasoningMode = "default"; #emptyResponseRecovery: { turnId: string; priorMessages: Set; responseStarted: boolean; responseFinished: boolean } | null = null; #emptyResponseAckTimer: ReturnType | undefined; + #interruptedTurnId: string | null = null; #pendingToolCalls = 0; readonly #runningNativeToolParts = new Set(); #abort = new AbortController(); @@ -695,6 +696,7 @@ class OpenCodeHarnessSession implements HarnessSession { this.#turnStartSourceSeq = this.#sourceSequence; this.#turnReasoningMode = reasoningMode; this.#emptyResponseRecovery = null; + this.#interruptedTurnId = null; this.#runningNativeToolParts.clear(); this.#emit("turn.submitted", { envelopeSchema: this.#taskEnvelope.schema, @@ -765,7 +767,7 @@ class OpenCodeHarnessSession implements HarnessSession { } #failEmptyResponse(turnId: string, message: string): void { - if (this.#closed || this.#activeTurnId !== turnId || this.#result !== null || this.#pendingToolCalls > 0 || this.#runningNativeToolParts.size > 0) return; + if (this.#closed || this.#activeTurnId !== turnId || this.#interruptedTurnId === turnId || this.#result !== null || this.#pendingToolCalls > 0 || this.#runningNativeToolParts.size > 0) return; this.#emit("turn.failed", { status: "failed", error: { code: "opencode_empty_response", message, retryable: false } }, { turnId }); this.#terminalTurns.set(turnId, canonicalJson({ status: "failed" })); this.#activeTurnId = null; @@ -774,6 +776,9 @@ class OpenCodeHarnessSession implements HarnessSession { #recoverEmptyResponse(turnId: string): boolean { if (this.#result !== null || this.#pendingRuntimeRequests.size > 0) return false; if (this.#pendingToolCalls > 0 || this.#runningNativeToolParts.size > 0) return true; + // An intentional pause needs the provider's idle fact, not another model + // request. Keep in-flight tool work gated by the checks above. + if (this.#interruptedTurnId === turnId) return false; const recovery = this.#emptyResponseRecovery; const boundary = Math.max(this.#turnStartSourceSeq, this.#lastNonTerminalToolSourceSeq); if (this.#completedTextParts.some(part => part.observedSourceSeq > boundary)) return false; @@ -800,13 +805,18 @@ class OpenCodeHarnessSession implements HarnessSession { } async interrupt(input: { turnId?: string; reason?: string }): Promise { - await this.#completionSettlement; + const turnId = this.#activeTurnId; if ( input.turnId && - this.#activeTurnId && - input.turnId !== this.#activeTurnId + turnId && + input.turnId !== turnId ) throw new Error("stale OpenCode turn"); + if (!turnId) return; + this.#interruptedTurnId = turnId; + this.#clearEmptyResponseAckTimer(); + await this.#completionSettlement; + if (this.#activeTurnId !== turnId) return; await api( this.#fetch, this.#runtime, @@ -982,12 +992,7 @@ class OpenCodeHarnessSession implements HarnessSession { `/question/${encodeURIComponent(input.requestId)}/reject?${workspace}`, { method: "POST" }, ), - api( - this.#fetch, - this.#runtime, - `/session/${encodeURIComponent(this.#providerSessionId)}/abort`, - { method: "POST" }, - ), + this.interrupt({ turnId: input.turnId, reason: "Durable user input handoff." }), ]).then(() => undefined); return { result: "handed_off", cleanup }; } @@ -1864,12 +1869,14 @@ class OpenCodeHarnessSession implements HarnessSession { turnId ) { if (this.#recoverEmptyResponse(turnId)) return; + const interrupted = this.#interruptedTurnId === turnId; + const status = interrupted ? "cancelled" : "completed"; const fingerprint = canonicalJson({ - status: "completed", + status, semanticResult: this.#resultFingerprint, }); this.#terminalTurns.set(turnId, fingerprint); - this.#emitSettledFinalAgentMessage(turnId); + if (!interrupted) this.#emitSettledFinalAgentMessage(turnId); const workspace = this.#workspaceChangesByTurn.get(turnId); if (workspace !== undefined) this.#emit( @@ -1884,7 +1891,7 @@ class OpenCodeHarnessSession implements HarnessSession { // Emit while this turn is still `#activeTurnId`; the gate in `#emit` // drops any frame whose turnId is not the active turn, so nulling it // first would make `#emit` drop this very event. - this.#emit("turn.completed", { status: "completed" }, { turnId }); + this.#emit(interrupted ? "turn.cancelled" : "turn.completed", { status }, { turnId }); this.#activeTurnId = null; return; }