mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
fix(runner): retain admitted OpenCode pack on ordinary continuation (#19558)
## Thinking Path > - Paperclip manages AI agents and preserves their work across runs. > - Native OpenCode turns can restore a saved provider session after the prior daemon stops. > - The Rust provider binds that saved state to exact qualified launch paths and artifact bytes. > - Ordinary continuation selected the current provider pack, although live reattachment retained the admitted pack. > - A new proxy or a diagnostics setting could therefore replace the launch profile behind saved state. > - This pull request retains the recorded pack for ordinary OpenCode continuation with a verified durable binding. > - The benefit is compatible continuation without weakening artifact integrity or granting replay authority. ## Linked Issues or Issue Description Refs: #17512, #18237. These added saved authority and compatible pack reuse. This change covers ordinary OpenCode continuation. **What happened?** An ordinary remote OpenCode continuation could select a newer pack while restoring its existing provider session. The Rust recovery guard then rejected the changed launch profile. The same error can occur after a controller release changes the bundled proxy. **Expected behavior** A continuation with a valid saved binding and pack receipt uses the same admitted artifacts. If those artifacts are unavailable, it holds the saved state instead of selecting another pack or creating a provider session. **Steps to reproduce** 1. Admit a remote OpenCode session with a qualified protected provider pack. 2. Persist its pack receipt and suspended durable Runner state. 3. Change the controller bundle to a valid pack with a different proxy digest. 4. Start an ordinary continuation with diagnostics enabled. The new executor regression fails before the repair because selection rejects the available original pack in favor of the new bundle. It passes after the repair. ## What Changed - Retain the saved provider pack for ordinary OpenCode continuation with a verified durable session binding. - Keep provider-pack retention independent of surviving-daemon identity: a new daemon records its current binary, and an authorized continuity break restores normal compatible pack selection. - Test retained pack selection after a proxy upgrade and fail closed when only a newer pack is available. - Test the real Rust executor with changed, valid proxy file bytes. It rejects drift without changing saved state and accepts the original artifacts again. - Document the selection rule and its limits. ## Verification - Initial prevention focused Vitest: 220 passed across native session execution, pack authority, pack verification and pack preparation. The executor regression was observed failing before the production change. - Rust native provider tests: all five OpenCode tests passed. These include the existing drift guard, actual changed proxy bytes, original artifact restoration and restart-lost turn behavior. - Advisory review follow-up: reproduced stale daemon identity in the actual executor and incompatible pack selection after a governed continuity archive. Both fail before their corresponding repairs and pass after them. Added an executor regression for a missing authority receipt; it remains at the current release integrity gate without creating a new receipt. - Updated head `639c080856` integrates master `17901d74ba` by normal merge. Affected Vitest: 202 passed across four files, including all new review regressions; all seven server typecheck layers passed. Exact-head CI and fresh Greptile/Paperclip review are pending. The prior head had 57 successful checks plus successful Snyk and Greptile 5/5; those results do not qualify this revision. - Environment: macOS arm64, Node 26.4.0, locked pnpm 9.15.4 dependencies, controlled remote command boundary and real Rust provider executor. No paid provider calls or hosted retries. - This is a bounded prevention change. It does not deliver historical provider artifacts to a replacement sandbox or replacement controller. Missing saved receipts or unavailable old artifacts remain a separate cold-recovery gap. No full hosted recovery is claimed. ## Risks - A continuation now requires its exact recorded pack when it has a durable binding. A newer installed pack cannot substitute for missing historical bytes. - Fresh launches, other providers and explicit continuity replacement keep their existing paths. Current ownership, company, model, permission and receipt binding checks remain in effect. - The repair does not relax the Rust launch-profile guard, replay prior tool effects, or authorize a new provider conversation. Existing published review receipts remain history. ## Model Used OpenAI Codex, GPT-6 family. The host does not expose the exact model ID or context window. Capabilities used: reasoning, code editing, shell execution, tests and authenticated read-only browser diagnostics. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
744056ecc8
commit
b55d8a9201
6 files changed
+162
-9
No files matched your search
@@ -116,6 +116,7 @@ import { acknowledgedNativeStopExecutionHasStopped, isAcknowledgedNativeStop } f
|
||||
import { admittedNativeProviderPack, nativeProviderPackAdmissionPending, rememberAdmittedNativeProviderPack } from "./native-provider-pack-admission.js";
|
||||
import { canonicalProviderPackJson } from "./remote-provider-pack-verification.js";
|
||||
import * as providerPackAdmission from "./native-provider-pack-admission.js";
|
||||
import * as nativeExecutionDriver from "./native-runner-execution-driver.js";
|
||||
|
||||
vi.mock("node:fs", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("node:fs")>();
|
||||
@@ -1878,7 +1879,7 @@ describe("remote provider pack manifest", () => {
|
||||
return JSON.stringify(value);
|
||||
};
|
||||
|
||||
it.each(["opencode", "opencode-dirty", "opencode-diagnostics", "opencode-diagnostics-stale", "cursor", "claude-x64", "claude-arm64"] as const)("uses the bundled Linux image authority for remote %s without controller overrides", async provider => {
|
||||
it.each(["opencode", "opencode-continuation", "opencode-continuation-missing", "opencode-continuation-no-receipt", "opencode-continuation-replacement", "opencode-dirty", "opencode-diagnostics", "opencode-diagnostics-stale", "cursor", "claude-x64", "claude-arm64"] as const)("uses the bundled Linux image authority for remote %s without controller overrides", async provider => {
|
||||
const architecture = provider === "claude-arm64" ? "arm64" : "x64";
|
||||
const claude = provider.startsWith("claude-");
|
||||
const opencode = provider.startsWith("opencode");
|
||||
@@ -1957,6 +1958,7 @@ describe("remote provider pack manifest", () => {
|
||||
const transport = state.createTransport.mock.calls.at(-1)![0] as RunnerTransportOptions & {
|
||||
controlPlaneRegistration: (authority: unknown) => Promise<unknown>;
|
||||
environment?: NodeJS.ProcessEnv;
|
||||
runnerArtifact?: { version: string; digest: string };
|
||||
};
|
||||
expect(transport.runnerBinary).toBe(state.bundledRunnerBinary);
|
||||
if (opencode) {
|
||||
@@ -1977,7 +1979,7 @@ describe("remote provider pack manifest", () => {
|
||||
expect(transport).toMatchObject({ providerPackAuthorityDigest: installed.digest,
|
||||
providerNodeCommand: "/opt/paperclip-runner/provider-pack/node_modules/node/bin/node",
|
||||
providerNodeCommandSha256: installed.payload.artifacts.nodeCommand.sha256 });
|
||||
if (provider === "opencode" || provider === "opencode-dirty") {
|
||||
if (provider === "opencode" || provider.startsWith("opencode-continuation") || provider === "opencode-dirty") {
|
||||
const identity = { ...remoteExecution.binding, sessionId: remoteExecution.session.normalizedSessionId! };
|
||||
const session = { identity: () => identity } as never;
|
||||
const backend = state.createBackend.mock.results.at(-1)!.value;
|
||||
@@ -2002,6 +2004,75 @@ describe("remote provider pack manifest", () => {
|
||||
expect(execute.mock.calls.filter(([call]) => call.args?.[0] === "--build-metadata")).toHaveLength(1);
|
||||
expect(execute.mock.calls.some(([call]) => call.args?.[0] === "-e" || call.args?.[0] === "--version")).toBe(false);
|
||||
expect(syncIn.mock.calls.every(([operations]) => operations.every(operation => operation.files.every(file => file.kind === "file")))).toBe(true);
|
||||
if (provider.startsWith("opencode-continuation")) {
|
||||
const identity = transport.prpIdentity!;
|
||||
await mkdir(join(transport.stateDirectory!, "control-plane"), { recursive: true });
|
||||
await writeFile(join(transport.stateDirectory!, "control-plane", "control-plane-state.json"), JSON.stringify(durableControlPlaneState(identity)));
|
||||
await mkdir(join(transport.stateDirectory!, "runner"), { recursive: true });
|
||||
await writeFile(join(transport.stateDirectory!, "runner", "runner-state.json"), JSON.stringify(durableRunnerState(identity, "suspended")));
|
||||
const next = structuredClone(manifest);
|
||||
next.payload.runnerSourceRevision = "b".repeat(40);
|
||||
next.payload.artifacts.opencodeProxy.sha256 = "sha256:" + "f".repeat(64);
|
||||
next.payload.bridgeDigest = "sha256:" + createHash("sha256")
|
||||
.update(next.payload.artifacts.opencodeProxy.sha256 + "\n" + digest + "\n" + next.payload.distDigest).digest("hex");
|
||||
next.digest = "sha256:" + createHash("sha256").update(canonical(next.payload)).digest("hex");
|
||||
await writeFile(state.bundledPackManifestPath!, JSON.stringify(next));
|
||||
const currentDaemon = Buffer.concat([daemon, Buffer.from("newly qualified daemon bytes")]);
|
||||
await writeFile(state.bundledRunnerBinary!, currentDaemon);
|
||||
// A normal next turn restores this provider session; it is not a live
|
||||
// daemon reattachment. Enabling diagnostics must not replace the
|
||||
// already admitted proxy behind that retained provider state.
|
||||
const receiptPath = join(transport.stateDirectory!, "provider-pack-authority.json");
|
||||
const receiptBefore = await readFile(receiptPath, "utf8");
|
||||
if (provider === "opencode-continuation-no-receipt") await rm(receiptPath);
|
||||
const prepareRemote = vi.spyOn(nativeExecutionDriver, "prepareNativeRemoteRunner");
|
||||
const backendsBefore = state.createBackend.mock.calls.length;
|
||||
const selectedExecute = provider === "opencode-continuation-missing"
|
||||
? async (command: { command: string; args?: string[] }) => command.args?.[0] === "--build-metadata"
|
||||
? { ...await execute(command), stdout: JSON.stringify({
|
||||
schema: "paperclip-runner/runnerd-build-metadata/v1", binaryName: "paperclip-runnerd",
|
||||
packageName: "@paperclipai/paperclip-runner", binaryContractVersion: 2,
|
||||
durableSessionCapabilities: ["unlimited_runtime", "connection_lease_renewal"], prpTransportModes: ["listen_ws"],
|
||||
providerPacks: [{ root: "/opt/paperclip-runner/provider-pack", manifest: next, immutable: true }],
|
||||
}) } : execute(command)
|
||||
: execute;
|
||||
const continuation = createRunnerdBackend({ db: leaseDb(remoteExecution), execution: remoteExecution,
|
||||
runnerInstanceId: identity.runnerInstanceId, runnerIngressAuthorized: true,
|
||||
runnerEnvironment: configuredEnvironmentProjection(provider === "opencode-continuation-replacement" ? {} : { PAPERCLIP_OPENCODE_PERFORMANCE_DIAGNOSTICS: "1" }),
|
||||
runnerExecutionTarget: { kind: "remote", transport: "sandbox", remoteCwd: "/workspace",
|
||||
environmentId: "environment", leaseId: "lease", providerKey: "daytona",
|
||||
effectiveCapabilities: { runnerWebSocketIngress: true }, runner: { execute: selectedExecute, syncIn } } as never });
|
||||
if (provider === "opencode-continuation-missing" || provider === "opencode-continuation-no-receipt") {
|
||||
await expect(continuation).rejects.toThrow("image provider pack did not match the bundled release identity");
|
||||
expect(state.createBackend).toHaveBeenCalledTimes(backendsBefore);
|
||||
if (provider === "opencode-continuation-missing") expect(await readFile(receiptPath, "utf8")).toBe(receiptBefore);
|
||||
else expect(existsSync(receiptPath)).toBe(false);
|
||||
return;
|
||||
}
|
||||
const continued = await continuation;
|
||||
state.createBackend.mock.calls.at(-1)![1].codexTransportFactory!();
|
||||
expect(state.createTransport.mock.calls.at(-1)![0]).toMatchObject({
|
||||
providerPackAuthorityDigest: installed.digest,
|
||||
opencodeProxySha256: installed.payload.artifacts.opencodeProxy.sha256,
|
||||
opencodeProxyPath: "/opt/paperclip-runner/provider-pack/dist/cli/opencode-app-server-proxy.cjs",
|
||||
runnerArtifact: { version: transport.runnerArtifact!.version, digest: "sha256:" + createHash("sha256").update(currentDaemon).digest("hex") },
|
||||
});
|
||||
if (provider === "opencode-continuation-replacement") {
|
||||
const preparation = prepareRemote.mock.calls.at(-1)![0];
|
||||
const backend = state.createBackend.mock.results.at(-1)!.value;
|
||||
Object.assign(backend, { openSession: async () => {
|
||||
await nativeExecutionDriver.prepareNativeRemoteRunner(preparation);
|
||||
return { identity: () => ({ ...remoteExecution.binding, sessionId: remoteExecution.session.normalizedSessionId! }) };
|
||||
} });
|
||||
const priorIdentity = { ...remoteExecution.binding, sessionId: remoteExecution.session.normalizedSessionId! };
|
||||
await continued.openReplacementSession!({ identity: priorIdentity }, {
|
||||
backendKind: "runner", identity: priorIdentity, sessionId: priorIdentity.sessionId,
|
||||
providerSessionId: "prior-provider-session", activeTurnId: null, terminal: null,
|
||||
});
|
||||
state.createBackend.mock.calls.at(-1)![1].codexTransportFactory!();
|
||||
expect(state.createTransport.mock.calls.at(-1)![0]).toMatchObject({ providerPackAuthorityDigest: installed.digest });
|
||||
}
|
||||
}
|
||||
if (provider === "claude-x64" || provider === "opencode") {
|
||||
const admitted = opencode ? installed : manifest;
|
||||
const identity = transport.prpIdentity!;
|
||||
|
||||
@@ -13906,8 +13906,12 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
})();
|
||||
return currentProviderPackManifest;
|
||||
};
|
||||
// Ordinary per-turn OpenCode continuation restores the same durable provider
|
||||
// state even when no daemon survived. Its launch profile binds the admitted
|
||||
// artifact bytes and paths just as a live reattachment does.
|
||||
const reattachProviderPack = input.restartRecovery?.kind === "reattach_remote_runner" || input.remoteWarmRecovery !== undefined;
|
||||
if (requiresRemoteProviderPack && !reattachProviderPack) resolveCurrentProviderPackManifest();
|
||||
let retainProviderPack = reattachProviderPack || input.execution.provider.kind === "opencode" && durableBinding !== null;
|
||||
if (requiresRemoteProviderPack && !retainProviderPack) resolveCurrentProviderPackManifest();
|
||||
// The transport hashes runnerBinary on the controller before an external
|
||||
// launcher starts runnerd. Keep that artifact identity in the controller's
|
||||
// filesystem; the remote launcher separately owns the sandbox command path.
|
||||
@@ -13930,6 +13934,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
runnerInstanceId: effectiveRunnerInstanceId, environmentLeaseId: effectiveEnvironmentLeaseId,
|
||||
remoteCwd: remoteTarget!.remoteCwd, providerDigest: nativeSha256(input.execution.provider) },
|
||||
reattach,
|
||||
retainProviderPack: reattach || retainProviderPack,
|
||||
runnerArtifact: () => readRunnerdArtifactBinding(controllerRunnerBinary),
|
||||
current: resolveCurrentProviderPackManifest,
|
||||
}) : null;
|
||||
@@ -14064,11 +14069,11 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
runner: remoteCommandRunner, cwd: remoteTarget.remoteCwd, packRoot: installation.root,
|
||||
manifest: expected, installation,
|
||||
provider: input.execution.provider as { kind: "opencode" } | { kind: "acpx"; agent: string },
|
||||
requireExactIdentity: reattachProviderPack || explicitProviderPackSelection,
|
||||
requireCurrentOpenCodeProxy: !reattachProviderPack
|
||||
requireExactIdentity: retainProviderPack || explicitProviderPackSelection,
|
||||
requireCurrentOpenCodeProxy: !retainProviderPack
|
||||
&& runnerEnvironment?.PAPERCLIP_OPENCODE_PERFORMANCE_DIAGNOSTICS === "1",
|
||||
}));
|
||||
if (!reattachProviderPack) providerPackAuthority!.select(manifest);
|
||||
if (!retainProviderPack) providerPackAuthority!.select(manifest);
|
||||
expectedProviderPackManifest = manifest;
|
||||
activeRemoteProviderPackRoot = installation.root;
|
||||
providerPackVerified = true;
|
||||
@@ -15045,6 +15050,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
}
|
||||
}
|
||||
remotePrepared = false;
|
||||
retainProviderPack = false;
|
||||
providerPackAuthority = selectProviderPackAuthority(false);
|
||||
expectedProviderPackManifest = providerPackAuthority?.manifest ?? null;
|
||||
providerPackVerified = false;
|
||||
|
||||
@@ -93,6 +93,16 @@ it("retains the authenticated daemon identity across controller daemon replaceme
|
||||
expect(recovered.runnerArtifact).toEqual(runnerArtifact());
|
||||
expect(changedRunner).not.toHaveBeenCalled();
|
||||
});
|
||||
it("retains the admitted provider pack but binds a newly launched daemon to its current bytes", () => {
|
||||
const { input } = fixture();
|
||||
resolveRemoteProviderPackAuthority(input).verified();
|
||||
const currentRunner = { version: "new", digest: "sha256:" + "d".repeat(64) };
|
||||
const continued = resolveRemoteProviderPackAuthority({ ...input, reattach: false, retainProviderPack: true,
|
||||
current: () => manifest("b"), runnerArtifact: () => currentRunner } as Parameters<typeof resolveRemoteProviderPackAuthority>[0]);
|
||||
expect(continued.manifest).toEqual(manifest("a"));
|
||||
expect(continued.runnerArtifact).toEqual(currentRunner);
|
||||
expect(() => continued.select(manifest("b"))).toThrow("cannot replace");
|
||||
});
|
||||
it.each([{}, { version: "0.0.0", digest: "not-a-sha256" }])("rejects an invalid saved daemon identity: %j", runnerArtifact => {
|
||||
const { input, path } = fixture();
|
||||
resolveRemoteProviderPackAuthority(input).verified();
|
||||
|
||||
@@ -31,6 +31,7 @@ export function resolveRemoteProviderPackAuthority<T extends ProviderPackIdentit
|
||||
stateDirectory: string;
|
||||
binding: ProviderPackBinding;
|
||||
reattach: boolean;
|
||||
retainProviderPack?: boolean;
|
||||
current: () => T;
|
||||
runnerArtifact: () => RunnerArtifactIdentity;
|
||||
}): { manifest: T; runnerArtifact: RunnerArtifactIdentity; select: (manifest: T) => void; verified: () => void } {
|
||||
@@ -41,7 +42,7 @@ export function resolveRemoteProviderPackAuthority<T extends ProviderPackIdentit
|
||||
const path = join(input.stateDirectory, filename);
|
||||
let manifest: T | undefined;
|
||||
let runnerArtifact: RunnerArtifactIdentity | undefined;
|
||||
if (input.reattach) {
|
||||
if (input.reattach || input.retainProviderPack) {
|
||||
let fd: number | undefined;
|
||||
try {
|
||||
fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW);
|
||||
@@ -55,7 +56,7 @@ export function resolveRemoteProviderPackAuthority<T extends ProviderPackIdentit
|
||||
if (typeof saved.runnerArtifact?.version !== "string" || !saved.runnerArtifact.version
|
||||
|| !/^sha256:[a-f0-9]{64}$/.test(saved.runnerArtifact.digest)) throw new Error("invalid runner artifact identity");
|
||||
manifest = saved.manifest as T;
|
||||
runnerArtifact = saved.runnerArtifact;
|
||||
if (input.reattach) runnerArtifact = saved.runnerArtifact;
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
|
||||
throw new Error("runner_remote_provider_artifact_incompatible: saved provider pack authority is invalid or does not match this execution", { cause: error });
|
||||
@@ -78,7 +79,7 @@ export function resolveRemoteProviderPackAuthority<T extends ProviderPackIdentit
|
||||
return {
|
||||
get manifest() { return selected; },
|
||||
select: (manifest: T) => {
|
||||
if (input.reattach) throw new Error("runner_remote_provider_artifact_incompatible: cannot replace an attached pack authority");
|
||||
if (input.reattach || input.retainProviderPack) throw new Error("runner_remote_provider_artifact_incompatible: cannot replace an attached pack authority");
|
||||
assertManifest(manifest);
|
||||
const next = structuredClone(manifest);
|
||||
receipt = serialize(next);
|
||||
|
||||
Reference in new issue
Block a user