mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(opencode): bind the attached run's contract after a restored launch (#20001)
## 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 "<expected>".`, 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
201163793a
commit
946c098bf3
6 files changed
+136
-6
No files matched your search
@@ -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<typeof openCodeProxyCompletionContract> = undefined;
|
||||
const announcedTurnIds = new Set<string>();
|
||||
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<void> {
|
||||
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)
|
||||
|
||||
@@ -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 <unistd.h>\n#include <stdlib.h>\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<string, unknown>; 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<void>(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);
|
||||
@@ -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<string, unknown>) {
|
||||
const completionContract = record(params.completionContract);
|
||||
const revision = text(completionContract.revision).trim();
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user