From 946c098bf3341e67728cba687a36ef07f3a0e9fa Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 03:35:52 -0600 Subject: [PATCH] fix(opencode): bind the attached run's contract after a restored launch (#20001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native OpenCode agents run through runnerd and a small OpenCode proxy. The proxy checks each `paperclip_finish` claim against the run's completion contract. > - Each follow-up run has its own contract revision. #19874 lets runnerd keep the OpenCode process across a contract change, and sends the new contract on `turn/start`. > - When runnerd starts again for a follow-up, it restores the saved session first. The restored proxy launches with the saved run's contract, but its environment already names the new run. > - The proxy applied the `turn/start` contract only when the run id changed. Here the run id did not change, so the proxy kept the saved contract and refused every correct finish claim. > - This pull request makes the proxy rebind whenever the turn's contract differs from the one its session holds. The stale-revision error also names the expected revision. > - The benefit: a queued-comment follow-up finishes on its first try, and an agent that sends a stale revision can repair it. ## Linked Issues or Issue Description Refs #19874 (warm runs carry their own completion contract). **What happened** A user sent a follow-up comment while an OpenCode agent was working. After the first run finished, Paperclip started a continuation run for the queued comment. The agent updated the plan and called `paperclip_finish` with the current revision `2`. The tool refused it twice with "Semantic result completion contract revision does not match". The error did not name the expected revision. The agent searched for it for 3 minutes, and cleanup cancelled the run. Root cause: runnerd restores a saved `session_open` session before it applies `run.attach` (`provider_backend.rs`: "execute() restores the durable provider before dispatching run.attach"). The restore launches the OpenCode proxy with the saved contract (revision `1`). Runnerd's environment already names the new run, so the proxy's `PAPERCLIP_RUN_ID` is the new run. `attach_run_in_place` keeps this provider for a changed contract (#19874) and sends `paperclipCompletionContract` revision `2` on `turn/start`. The proxy compared only the run id. The run id matched, so the proxy skipped `attachRun` and kept revision `1`. **Expected behavior** The continuation's first `paperclip_finish` with the current revision is accepted. If a claim has a stale revision, the error names the expected revision. **Steps to reproduce** 1. Use a native OpenCode agent with warm follow-ups (the default). 2. Start a task. While the agent works, post a follow-up comment, so that it queues. 3. Let the first run finish. Paperclip starts a continuation run with completion revision `2` and restores the saved OpenCode session. 4. The agent calls `paperclip_finish` with `contractRevision: "2"`. 5. Before this fix, the tool refuses it with "Semantic result completion contract revision does not match". The chat eval `steering.discard-queued-message` (runner-opencode) shows this in run events: `session.resumed` carries revision `2`, and both `paperclip_finish` results fail with the mismatch. ## What Changed - `opencode-app-server-proxy.ts`: record the contract that the session opened with. On `turn/start`, call `attachRun` when the run id changes or when the supplied `paperclipCompletionContract` differs from the session's contract. - `opencode-proxy-task-envelope.ts`: add `sameOpenCodeProxyCompletionContract`. - `opencode-server-driver.ts`: the stale-revision error now ends with `Use contractRevision "".`, like the criteria error. - Tests: a real-proxy regression for the restored launch, a driver test for the new error text, and a `repair-revision` mode in the fake OpenCode fixture. ## Verification - `pnpm exec vitest run src/cli/opencode-proxy-input.test.ts src/drivers/opencode/opencode-server-driver.test.ts src/cli/opencode-app-server-proxy.test.ts src/cli/opencode-proxy-thread-read.test.ts` in `packages/paperclip-runner`: 88 passed. - Red/green: without the source changes, both new tests fail. `binds the current contract when a restored launch already names the attached run` never sees the finish call reach the controller. `names the expected revision in a repairable stale-revision tool error` does not find the expected revision in the error. - `npx tsc --noEmit -p packages/paperclip-runner`: no errors in the changed files. (My worktree is missing a link for `@paperclipai/paperclip-eval-kernel`, which causes unrelated errors in `src/eval/workflow-harness.ts`.) - End-to-end: the chat eval `steering.discard-queued-message` (runner-opencode) with this fix cherry-picked. The run link goes in a comment below. ## Risks - Previous-release story (`rules/upgrade-safe-run-state.mdc`): this change does not change a persisted shape. The regression test uses the saved-state shape: launch parameters from the saved run, environment and `turn/start` from the new run. A runnerd that does not send `paperclipCompletionContract` behaves as before, so the proxy keeps its launch contract. The proxy and runnerd ship in the same runner artifact. - The proxy can now call `attachRun` for the same run id when the contract changes. `attachRun` resets the per-run result state. This occurs only at `turn/start`, when no turn is active, and only when the contract is different. A result for the earlier contract is not valid for the new contract. - The error text changed. No code parses it. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 --- .../src/cli/opencode-app-server-proxy.ts | 23 ++++-- .../src/cli/opencode-proxy-input.test.ts | 79 +++++++++++++++++++ .../src/cli/opencode-proxy-task-envelope.ts | 11 +++ .../opencode/opencode-server-driver.test.ts | 19 +++++ .../opencode/opencode-server-driver.ts | 4 +- .../test/fixtures/fake-opencode-server.mjs | 6 ++ 6 files changed, 136 insertions(+), 6 deletions(-) diff --git a/packages/paperclip-runner/src/cli/opencode-app-server-proxy.ts b/packages/paperclip-runner/src/cli/opencode-app-server-proxy.ts index 90e98ca0c2b..3782d09e129 100644 --- a/packages/paperclip-runner/src/cli/opencode-app-server-proxy.ts +++ b/packages/paperclip-runner/src/cli/opencode-app-server-proxy.ts @@ -11,7 +11,11 @@ import type { } from "../contracts/harness-driver.js"; import { OpenCodeServerDriver } from "../drivers/opencode/opencode-server-driver.js"; import { parseNativeRuntimeContext } from "../contracts/runtime-context.js"; -import { openCodeProxyCompletionContract, openCodeProxyTaskEnvelope } from "./opencode-proxy-task-envelope.js"; +import { + openCodeProxyCompletionContract, + openCodeProxyTaskEnvelope, + sameOpenCodeProxyCompletionContract, +} from "./opencode-proxy-task-envelope.js"; import { openCodeProxyItemNotification, openCodeProxyToolNotification, @@ -50,6 +54,11 @@ let cwd = ""; let activeModel = ""; let activeTurnId: string | null = null; let activeRunId = process.env.PAPERCLIP_RUN_ID ?? "runnerd-run"; +// The contract the open session checks finish claims against. The launch +// environment and the launch parameters can belong to different runs: runnerd +// restores a saved session with its saved contract while its environment +// already names the run being attached. +let activeCompletionContract: ReturnType = undefined; const announcedTurnIds = new Set(); const launchBinding = trustedOpenCodeLaunchBinding(process.argv.slice(2)); const usageLedger = new OpenCodeProxyUsageLedger(); @@ -166,6 +175,7 @@ async function open( workingDirectory: cwd, }); } + activeCompletionContract = openCodeProxyCompletionContract(params.completionContract); eventPump = pumpEvents(session); void eventPump.catch((error) => failProxy(error)); return threadResponse( @@ -354,13 +364,16 @@ async function handle(message: RpcMessage): Promise { const runId = params.paperclipRunId; if (typeof runId !== "string" || !/^[A-Za-z0-9][A-Za-z0-9._:-]{0,127}$/.test(runId)) throw new Error("Invalid Paperclip run identity"); - if (runId !== activeRunId) { + // Runnerd sends the attached run's contract with its run identity; + // the launch parameters may still name an earlier run's contract, + // even when the launch environment already names this run. + const contract = openCodeProxyCompletionContract(params.paperclipCompletionContract); + if (runId !== activeRunId + || (contract !== undefined && !sameOpenCodeProxyCompletionContract(contract, activeCompletionContract))) { if (!session.attachRun) throw new Error("OpenCode run attachment unavailable"); - // Runnerd sends the attached run's contract with its run identity; - // the launch parameters still name the first run's contract. - const contract = openCodeProxyCompletionContract(params.paperclipCompletionContract); await session.attachRun({ runId, ...(contract ? { completionContract: contract } : {}) }); activeRunId = runId; + if (contract !== undefined) activeCompletionContract = contract; } } const inputItems = Array.isArray(params.input) diff --git a/packages/paperclip-runner/src/cli/opencode-proxy-input.test.ts b/packages/paperclip-runner/src/cli/opencode-proxy-input.test.ts index e29ff22ba03..da64ce04f17 100644 --- a/packages/paperclip-runner/src/cli/opencode-proxy-input.test.ts +++ b/packages/paperclip-runner/src/cli/opencode-proxy-input.test.ts @@ -183,3 +183,82 @@ it.each([true, false])("settles a real proxy interrupt with controller feedback await rm(root, { recursive: true, force: true }); } }, 30_000); + +// Runnerd restores a saved session before it applies `run.attach`: the proxy +// launches with the saved run's contract while its environment already names +// the attached run. The attached run's contract still arrives on turn/start. +it("binds the current contract when a restored launch already names the attached run", async () => { + const { execFileSync, spawn } = await import("node:child_process"); + const { chmod, mkdir, mkdtemp, rm } = await import("node:fs/promises"); + const { closeSync, openSync } = await import("node:fs"); + const { randomUUID } = await import("node:crypto"); + const { tmpdir } = await import("node:os"); + const { join, resolve } = await import("node:path"); + const { createInterface } = await import("node:readline"); + const root = await mkdtemp(join(tmpdir(), "opencode-proxy-restored-contract-")); + const launchRoot = join(root, `.paperclip-verified-executable-${randomUUID().replaceAll("-", "")}`); + await mkdir(launchRoot, { mode: 0o700 }); + await chmod(launchRoot, 0o700); + const executable = join(launchRoot, "launch"); + const fixture = resolve("test/fixtures/fake-opencode-server.mjs"); + execFileSync("cc", ["-x", "c", "-o", executable, "-"], { input: `#include \n#include \nint main(int argc, char **argv) { char **args = calloc(argc + 2, sizeof(char *)); args[0] = ${JSON.stringify(process.execPath)}; args[1] = ${JSON.stringify(fixture)}; for (int i = 1; i < argc; i++) args[i + 1] = argv[i]; execv(args[0], args); return 127; }` }); + if (process.platform === "darwin") { + execFileSync("codesign", ["--force", "--sign", "-", executable]); + execFileSync("codesign", ["--verify", "--strict", executable]); + } + await chmod(executable, 0o500); + const proxy = join(root, "proxy.cjs"); + await (await import("node:fs/promises")).writeFile(proxy, execFileSync(process.execPath, ["--input-type=module", "-e", `import { bundleVerifiedProviderEntrypoints } from "./scripts/build-verified-provider-entrypoints.mjs"; const entries = await bundleVerifiedProviderEntrypoints({ write: false }); process.stdout.write(entries.find(({ entrypoint }) => entrypoint.name === "opencode-app-server-proxy").verifiedResult.outputFiles[0].contents);`], { maxBuffer: 16 * 1024 * 1024 })); + const fd = process.platform === "linux" ? openSync(executable, "r") : undefined; + const child = spawn(process.execPath, [proxy, "--paperclip-trusted-opencode-executable", fd === undefined ? executable : "/proc/self/fd/3"], { + env: { ...process.env, PAPERCLIP_OPENCODE_RUNTIME_DIR: join(root, "runtime"), OPENROUTER_API_KEY: "fixture-key", + PAPERCLIP_RUN_ID: "attached-run-2" }, + stdio: fd === undefined ? ["pipe", "pipe", "pipe"] : ["pipe", "pipe", "pipe", fd], + detached: process.platform !== "win32", + }); + if (fd !== undefined) closeSync(fd); + const messages: Array<{ id?: string | number; method?: string; params?: Record; result?: unknown; error?: unknown }> = []; + const input = createInterface({ input: child.stdout! }); + input.on("line", line => messages.push(JSON.parse(line))); + child.stderr!.resume(); + let exited = false; + const exit = new Promise(resolve => child.once("exit", () => { exited = true; resolve(); })); + const send = (message: unknown) => child.stdin!.write(`${JSON.stringify(message)}\n`); + const response = async (id: number) => { + await expect.poll(() => messages.find(message => message.id === id), { timeout: 5_000 }).toBeDefined(); + const value = messages.find(message => message.id === id)!; + expect(value.error).toBeUndefined(); + return value.result; + }; + try { + send({ id: 1, method: "initialize", params: {} }); + await response(1); + // Launch parameters carry the saved run's contract. + send({ id: 2, method: "thread/start", params: { cwd: root, model: "openrouter/deepseek/deepseek-v4-flash-0731", completionContract: { revision: "1", criterionIds: ["objective"] }, conversationMode: "prepared" } }); + await response(2); + send({ id: 3, method: "turn/start", params: { paperclipRunId: "attached-run-2", + paperclipCompletionContract: { revision: "2", criterionIds: ["pending_comment_1", "pending_comment_2"] }, + input: [{ type: "text", text: JSON.stringify({ schema: "paperclip.native-model-envelope.v3", task: { prompt: "completion-feedback" }, + completionContract: { revision: "2", criteria: [{ id: "pending_comment_1", requirement: "Use a readable font." }, { id: "pending_comment_2", requirement: "Add a contact section." }] } }) }] } }); + await response(3); + // The finish claim reaches the controller on its first try. + await expect.poll(() => messages.find(message => message.method === "item/tool/call"), { timeout: 5_000 }).toBeDefined(); + const call = messages.find(message => message.method === "item/tool/call")!; + expect(call.params).toMatchObject({ tool: "paperclip_finish", arguments: { completionClaim: { contractRevision: "2" } } }); + send({ id: call.id, result: { success: true, contentItems: [{ type: "inputText", text: "Accepted." }] } }); + await expect.poll(() => messages.filter(message => message.method === "paperclip/runResult").length, { timeout: 5_000 }).toBe(1); + expect(JSON.stringify(messages.find(message => message.method === "paperclip/runResult"))).toContain('"contractRevision":"2"'); + child.stdin!.end(); + await expect.poll(() => exited, { timeout: 5_000 }).toBe(true); + await exit; + } finally { + child.kill("SIGTERM"); + if (!exited) { + await Promise.race([exit, new Promise(resolve => setTimeout(resolve, 1_000))]); + if (!exited && child.pid) process.kill(process.platform === "win32" ? child.pid : -child.pid, "SIGKILL"); + await exit; + } + input.close(); + await rm(root, { recursive: true, force: true }); + } +}, 30_000); diff --git a/packages/paperclip-runner/src/cli/opencode-proxy-task-envelope.ts b/packages/paperclip-runner/src/cli/opencode-proxy-task-envelope.ts index d19cbacb52b..a871a1cb0b7 100644 --- a/packages/paperclip-runner/src/cli/opencode-proxy-task-envelope.ts +++ b/packages/paperclip-runner/src/cli/opencode-proxy-task-envelope.ts @@ -22,6 +22,17 @@ export function openCodeProxyCompletionContract(value: unknown): { revision: str : undefined; } +/** Whether two per-turn contracts name the same revision and criterion ids, in order. */ +export function sameOpenCodeProxyCompletionContract( + left: { revision: string; criterionIds: readonly string[] } | undefined, + right: { revision: string; criterionIds: readonly string[] } | undefined, +): boolean { + if (left === undefined || right === undefined) return left === right; + return left.revision === right.revision + && left.criterionIds.length === right.criterionIds.length + && left.criterionIds.every((id, index) => id === right.criterionIds[index]); +} + export function openCodeProxyTaskEnvelope(params: Record) { const completionContract = record(params.completionContract); const revision = text(completionContract.revision).trim(); 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 9afbe5c5a51..a1e31515dcf 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 @@ -2549,6 +2549,25 @@ describe("OpenCodeServerDriver", () => { await session.close({ reason: "test" }); }); + it("names the expected revision in a repairable stale-revision tool error", async () => { + await chmod(fixture, 0o755); + const root = await mkdtemp(join(tmpdir(), "paperclip-opencode-revision-")); + 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" }, taskEnvelope: createCodexTaskEnvelope({ objective: "Apply the accepted decision", contractRevision: "approval-v2", criteria: [{ id: "human_response", requirement: "Apply the approved response" }] }) }); + const session = await driver.openSession({ runId: "revision-repair", normalizedSessionId: "revision-repair", workingDirectory: workspace }); + await session.startTurn({ message: { role: "user", text: "repair-revision" } }); + const events = await collectTurnEvents(session.events()); + const results = events.filter((event) => event.eventType === "run.result.proposed"); + expect(results).toHaveLength(1); + expect(results[0].payload).toMatchObject({ completionClaim: { contractRevision: "approval-v2" } }); + const files = await readdir(root, { recursive: true }); + const evidence = files.find((name) => name.endsWith("fake-revision-repair.json")); + expect(evidence).toBeDefined(); + expect(JSON.parse(await readFile(join(root, evidence!), "utf8"))).toMatchObject({ result: { isError: true, content: [{ text: expect.stringContaining('Use contractRevision "approval-v2"') }] } }); + await session.close({ reason: "test" }); + }); + it.each(["monitor", "response_wake"] as const)("resolves an omitted %s through the real OpenCode MCP boundary only from current server state", async kind => { for (const eligible of [true, false]) { await chmod(fixture, 0o755); 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 72f2b5801f1..8d536ccaa78 100644 --- a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts +++ b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts @@ -1232,8 +1232,10 @@ class OpenCodeHarnessSession implements HarnessSession { validation.result.completionClaim.contractRevision !== this.#taskEnvelope.completionContract.revision ) { + // Name the expected revision so the provider can repair its claim + // in this turn, as the criteria error below does. throw new Error( - "Semantic result completion contract revision does not match", + `Semantic result completion contract revision does not match. Use contractRevision ${JSON.stringify(this.#taskEnvelope.completionContract.revision)}.`, ); } const expectedIds = this.#taskEnvelope.completionContract.criteria.map((criterion) => criterion.id); diff --git a/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs b/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs index f344de74f2d..83a81d42209 100644 --- a/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs +++ b/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs @@ -275,6 +275,12 @@ async function callTerminalTool(promptBody, signal) { const rejected = await mcpRequest("tools/call", { name: "paperclip_finish", arguments: bad }); await writeFile(join(process.env.XDG_DATA_HOME, "fake-criteria-repair.json"), JSON.stringify(rejected)); } + if (String(prompt.message ?? "").includes("repair-revision")) { + const stale = structuredClone(result); + stale.completionClaim.contractRevision = "stale-revision"; + const rejected = await mcpRequest("tools/call", { name: "paperclip_finish", arguments: stale }); + await writeFile(join(process.env.XDG_DATA_HOME, "fake-revision-repair.json"), JSON.stringify(rejected)); + } const call = () => mcpRequest("tools/call", { name: blocked ? "paperclip_block" : "paperclip_finish", arguments: result,