mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 16:35:48 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent adapters own local or remote provider processes. > - Run cleanup must know when the final provider process has stopped. > - A remote timeout or a lost transport does not prove that the provider stopped. > - Retried provider invocations also make an earlier stop signal stale. > - This pull request adds a verified final-invocation stop callback before workspace restoration. > - The dependent instruction revision change uses that boundary to preserve private instruction edits safely. ## Linked Issues or Issue Description **What happened?** Adapter completion did not expose a reliable point between provider shutdown and workspace restoration. Cleanup could lose provider-written files, or treat a remote timeout as proof that a process stopped. **Expected behavior** Cleanup runs once after the final provider invocation has a verified stop receipt and before workspace restoration. An incomplete remote command keeps collection pending. **Steps to reproduce** 1. Run a remote provider that returns a timeout without a numeric exit code. 2. Let the adapter return or retry the provider. 3. Attempt to collect provider-written files during cleanup. The adapter has no verified final-process boundary to use. This is the prerequisite for the stacked canonical instruction revision pull request. It has no database or UI dependency. Related #13291 verifies remote termination for later recovery; this change exposes the earlier adapter-owned stop boundary before workspace restoration. The scopes do not duplicate each other. ## What Changed - Add a stop callback to adapter execution context and a final-invocation fence. - Confirm local child closure and complete remote exit receipts. Reject remote timeouts, missing exits, transport failures, and SSH exit 255 as stop proof. - Invoke collection once before workspace restoration in eight CLI adapters and at the confirmed ACP stop boundary. - Preserve the stop observation when a later log flush fails. - Run bridge and workspace cleanup in `finally` even when collection rejects. ACP records a safe error without exposing a raw filesystem path. Grok keeps collection errors separate from workspace restore failures and preserves completed provider results when both cleanup steps fail. - Correct the existing Cursor test shell fixture so bounded remote file reads run against real fixture files. ## Verification - Three stop-boundary regressions failed before the callback implementation and passed after it. - Independent prerequisite branch: 377 tests passed across 24 adapter, process-target, and ACP suites. Two added collector-rejection tests failed before the cleanup fix and passed after it. - A third regression reproduced Grok misclassifying a collection failure as failed workspace restoration. Two additional cases covered completed and failed provider turns when collection and restore both fail. The Grok and restore-classifier suites passed 48 tests. - All nine affected package typechecks and affected package builds passed; Grok checks passed again after its classification fix. - Integrated instruction branch: native local, legacy local, and native Daytona each passed three browser tasks with exact persisted bytes, fresh-task readback, history/restore, and explicit conflict resolution. Unchanged warm Daytona passed three turns. Legacy Codex passed all five checks again after the exception-safe cleanup fix. - Alternate staging passed the same three-task native Daytona flow: exact stopped-run save, independent downloaded readback, browser history/restore, and explicit resolution of a real concurrent edit. The deployed source was `14c3d810c9e05625121b3d27767aea9317b03125`, which covers the initial adapter callback. Later Grok cleanup failures are qualified by the adapter tests above. - The final combined native Daytona flow passed again on deployed `2bedd0f23bf4698b1f8b818f6796900647030427`: three fresh tasks proved ordinary instruction edits, independent readback, History/Restore, concurrent board conflict, and explicit candidate resolution. This native staging flow does not claim to exercise the Grok adapter. ## Risks - An unverified remote stop intentionally does not trigger collection. A later controller with verified stop evidence must recover it or report the copy unavailable. - The callback is optional. Callers that do not register it retain their existing behavior. - The callback runs before workspace restoration and can delay cleanup if its caller does not bound its own work. The dependent instruction collector uses bounded reads and retries. A rejected callback still permits bridge and workspace cleanup; it cannot claim an instruction save. ## Model Used OpenAI Codex, GPT-6, with tool use and code execution. The runtime does not expose the exact deployment variant or context-window size. Multiple Codex agents implemented and verified the change. ## 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 <noreply@paperclip.ing>
317 lines
12 KiB
TypeScript
317 lines
12 KiB
TypeScript
import { lstat, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
|
|
import type { SandboxManagedRuntimeAsset } from "@paperclipai/adapter-utils/sandbox-managed-runtime";
|
|
|
|
// Captured Codex `home` asset descriptor + the sandbox `auth.json` fixture the
|
|
// mocked runtime hands back during teardown. Mutated per-test so a single
|
|
// harness drives every round-trip case through the REAL `execute()` wiring.
|
|
const captured: { assets: SandboxManagedRuntimeAsset[] } = { assets: [] };
|
|
const sandboxAuthFixture: { bytes: Buffer } = { bytes: Buffer.from("{}") };
|
|
const REMOTE_RUNTIME_ROOT = "/remote/workspace/.paperclip-runtime/codex";
|
|
|
|
const {
|
|
runChildProcess,
|
|
ensureCommandResolvable,
|
|
resolveCommandForLogs,
|
|
prepareAdapterExecutionTargetRuntime,
|
|
startAdapterExecutionTargetPaperclipBridge,
|
|
} = vi.hoisted(() => ({
|
|
runChildProcess: vi.fn(async () => ({
|
|
exitCode: 0,
|
|
signal: null,
|
|
timedOut: false,
|
|
stdout: "",
|
|
stderr: "",
|
|
pid: 321,
|
|
startedAt: new Date().toISOString(),
|
|
})),
|
|
ensureCommandResolvable: vi.fn(async () => undefined),
|
|
resolveCommandForLogs: vi.fn(async () => "/usr/bin/codex"),
|
|
prepareAdapterExecutionTargetRuntime: vi.fn(),
|
|
startAdapterExecutionTargetPaperclipBridge: vi.fn(async () => null),
|
|
}));
|
|
|
|
vi.mock("@paperclipai/adapter-utils/server-utils", async () => {
|
|
const actual = await vi.importActual<typeof import("@paperclipai/adapter-utils/server-utils")>(
|
|
"@paperclipai/adapter-utils/server-utils",
|
|
);
|
|
return {
|
|
...actual,
|
|
ensureCommandResolvable,
|
|
resolveCommandForLogs,
|
|
runChildProcess,
|
|
};
|
|
});
|
|
|
|
vi.mock("@paperclipai/adapter-utils/execution-target", async () => {
|
|
const actual = await vi.importActual<typeof import("@paperclipai/adapter-utils/execution-target")>(
|
|
"@paperclipai/adapter-utils/execution-target",
|
|
);
|
|
return {
|
|
...actual,
|
|
prepareAdapterExecutionTargetRuntime,
|
|
startAdapterExecutionTargetPaperclipBridge,
|
|
};
|
|
});
|
|
|
|
import { execute } from "./execute.js";
|
|
|
|
// Mirror the sandbox core's restore closure: capture the assets `execute()`
|
|
// declares, then during teardown invoke each asset's `restore` with an injected
|
|
// `readFile` (returns the sandbox fixture) and the remote asset dir. This drives
|
|
// the exact `restore` contribution the Codex adapter wires in production without
|
|
// needing a live sandbox.
|
|
prepareAdapterExecutionTargetRuntime.mockImplementation(async (input: { assets?: SandboxManagedRuntimeAsset[] }) => {
|
|
captured.assets = input.assets ?? [];
|
|
return {
|
|
target: { kind: "remote", transport: "ssh" },
|
|
workspaceRemoteDir: "/remote/workspace",
|
|
runtimeRootDir: REMOTE_RUNTIME_ROOT,
|
|
assetDirs: { home: `${REMOTE_RUNTIME_ROOT}/home` },
|
|
restoreWorkspace: async () => {
|
|
for (const asset of captured.assets) {
|
|
if (!asset.restore) continue;
|
|
await asset.restore({
|
|
assetDir: `${REMOTE_RUNTIME_ROOT}/home`,
|
|
readFile: async () => sandboxAuthFixture.bytes,
|
|
});
|
|
}
|
|
},
|
|
};
|
|
});
|
|
|
|
describe("codex execute — outbound auth copy-back restore contribution", () => {
|
|
const cleanupDirs: string[] = [];
|
|
let savedCodexHomeEnv: string | undefined;
|
|
|
|
afterEach(async () => {
|
|
vi.clearAllMocks();
|
|
if (savedCodexHomeEnv === undefined) {
|
|
delete process.env.CODEX_HOME;
|
|
} else {
|
|
process.env.CODEX_HOME = savedCodexHomeEnv;
|
|
}
|
|
while (cleanupDirs.length > 0) {
|
|
const dir = cleanupDirs.pop();
|
|
if (!dir) continue;
|
|
await rm(dir, { recursive: true, force: true }).catch(() => undefined);
|
|
}
|
|
});
|
|
|
|
function subscriptionAuth(input: { accountId: string; lastRefresh?: string; marker: string }): string {
|
|
return JSON.stringify(
|
|
{
|
|
tokens: {
|
|
id_token: `id-token-${input.marker}`,
|
|
access_token: `access-token-${input.marker}`,
|
|
refresh_token: `refresh-token-${input.marker}`,
|
|
account_id: input.accountId,
|
|
},
|
|
...(input.lastRefresh ? { last_refresh: input.lastRefresh } : {}),
|
|
},
|
|
null,
|
|
2,
|
|
);
|
|
}
|
|
|
|
async function runTeardown(input: { sandboxAuth: string; hostAuth: string; onProviderStopped?: () => Promise<void> }) {
|
|
const rootDir = await mkdtemp(
|
|
path.join(os.tmpdir(), "paperclip-codex-copyback-e2e-"),
|
|
);
|
|
cleanupDirs.push(rootDir);
|
|
const workspaceDir = path.join(rootDir, "workspace");
|
|
// The shared host home is what `resolveSharedCodexHomeDir` returns
|
|
// (process.env.CODEX_HOME) — the copy-back target. Point it at a tmp dir so
|
|
// the round-trip never touches the real host credential.
|
|
const sharedHostHome = path.join(rootDir, "shared-codex-home");
|
|
await mkdir(workspaceDir, { recursive: true });
|
|
await mkdir(sharedHostHome, { recursive: true });
|
|
const hostAuthPath = path.join(sharedHostHome, "auth.json");
|
|
await writeFile(hostAuthPath, input.hostAuth, { mode: 0o600 });
|
|
|
|
savedCodexHomeEnv = process.env.CODEX_HOME;
|
|
process.env.CODEX_HOME = sharedHostHome;
|
|
sandboxAuthFixture.bytes = Buffer.from(input.sandboxAuth, "utf8");
|
|
|
|
const executionResult = await execute({
|
|
runId: "run-copyback-e2e",
|
|
onProviderStopped: input.onProviderStopped,
|
|
agent: {
|
|
id: "agent-1",
|
|
companyId: "company-1",
|
|
name: "CodexCoder",
|
|
adapterType: "codex_local",
|
|
adapterConfig: {},
|
|
},
|
|
runtime: { sessionId: null, sessionParams: null, sessionDisplayId: null, taskKey: null },
|
|
config: {
|
|
command: "codex",
|
|
engine: "cli",
|
|
// External CODEX_HOME (outside the managed company tree) so no managed
|
|
// seeding rewrites auth.json before teardown; equals the shared host home.
|
|
env: { CODEX_HOME: sharedHostHome },
|
|
},
|
|
context: {
|
|
paperclipWorkspace: {
|
|
cwd: workspaceDir,
|
|
source: "project_primary",
|
|
},
|
|
},
|
|
executionTransport: {
|
|
remoteExecution: {
|
|
host: "127.0.0.1",
|
|
port: 2222,
|
|
username: "fixture",
|
|
remoteWorkspacePath: "/remote/workspace",
|
|
remoteCwd: "/remote/workspace",
|
|
privateKey: "PRIVATE KEY",
|
|
knownHosts: "[127.0.0.1]:2222 ssh-ed25519 AAAA",
|
|
strictHostKeyChecking: true,
|
|
},
|
|
},
|
|
onLog: async () => {},
|
|
});
|
|
|
|
return {
|
|
finalHostAuth: await readFile(hostAuthPath, "utf8"),
|
|
finalHostMode: (await lstat(hostAuthPath)).mode & 0o777,
|
|
executionResult,
|
|
};
|
|
}
|
|
|
|
it("collects stopped-provider instruction edits before a throwing remote restore", async () => {
|
|
const order: string[] = [];
|
|
prepareAdapterExecutionTargetRuntime.mockImplementationOnce(async () => ({
|
|
target: { kind: "remote", transport: "ssh" }, workspaceRemoteDir: "/remote/workspace",
|
|
runtimeRootDir: REMOTE_RUNTIME_ROOT, assetDirs: { home: `${REMOTE_RUNTIME_ROOT}/home` },
|
|
restoreWorkspace: async () => { order.push("restore"); throw new Error("restore failed"); },
|
|
}));
|
|
await expect(runTeardown({ sandboxAuth: "{}", hostAuth: "{}", onProviderStopped: async () => { order.push("collect"); } })).rejects.toThrow("restore failed");
|
|
expect(order).toEqual(["collect", "restore"]);
|
|
});
|
|
|
|
it("collects after a failed provider exit before restoring its workspace", async () => {
|
|
runChildProcess.mockResolvedValueOnce({ exitCode: 1, signal: null, timedOut: false, stdout: "", stderr: "provider failed", pid: 321, startedAt: new Date().toISOString() });
|
|
const collected = vi.fn(async () => {});
|
|
await runTeardown({ sandboxAuth: "{}", hostAuth: "{}", onProviderStopped: collected });
|
|
expect(collected).toHaveBeenCalledOnce();
|
|
});
|
|
|
|
it("stops the bridge and restores the workspace when instruction collection rejects", async () => {
|
|
const order: string[] = [];
|
|
startAdapterExecutionTargetPaperclipBridge.mockResolvedValueOnce({
|
|
env: {}, stop: async () => { order.push("bridge-stop"); },
|
|
} as never);
|
|
prepareAdapterExecutionTargetRuntime.mockImplementationOnce(async () => ({
|
|
target: { kind: "remote", transport: "ssh" }, workspaceRemoteDir: "/remote/workspace",
|
|
runtimeRootDir: REMOTE_RUNTIME_ROOT, assetDirs: { home: `${REMOTE_RUNTIME_ROOT}/home` },
|
|
restoreWorkspace: async () => { order.push("restore"); },
|
|
}));
|
|
await expect(runTeardown({ sandboxAuth: "{}", hostAuth: "{}", onProviderStopped: async () => {
|
|
order.push("collect");
|
|
throw new Error("instruction collection failed");
|
|
} })).rejects.toThrow("instruction collection failed");
|
|
expect(order).toEqual(["collect", "bridge-stop", "restore"]);
|
|
});
|
|
|
|
it("declares a Codex `home` asset carrying both inbound provision and outbound restore contributions", async () => {
|
|
await runTeardown({
|
|
sandboxAuth: subscriptionAuth({ accountId: "acct", lastRefresh: "2026-07-09T01:00:00Z", marker: "s" }),
|
|
hostAuth: subscriptionAuth({ accountId: "acct", lastRefresh: "2026-07-09T02:00:00Z", marker: "h" }),
|
|
});
|
|
|
|
const homeAsset = captured.assets.find((asset) => asset.key === "home");
|
|
expect(homeAsset).toBeDefined();
|
|
expect(homeAsset?.provision).toBeTruthy();
|
|
expect(typeof homeAsset?.restore).toBe("function");
|
|
});
|
|
|
|
it("round-trips a strictly-newer same-identity sandbox auth.json to the shared host at 0600 on teardown", async () => {
|
|
const sandboxAuth = subscriptionAuth({
|
|
accountId: "acct-same",
|
|
lastRefresh: "2026-07-09T02:00:00Z",
|
|
marker: "sandbox-newer",
|
|
});
|
|
const hostAuth = subscriptionAuth({
|
|
accountId: "acct-same",
|
|
lastRefresh: "2026-07-09T01:00:00Z",
|
|
marker: "host-older",
|
|
});
|
|
|
|
const result = await runTeardown({ sandboxAuth, hostAuth });
|
|
|
|
expect(result.finalHostAuth).toBe(sandboxAuth);
|
|
expect(result.finalHostMode).toBe(0o600);
|
|
});
|
|
|
|
it("keeps the host auth.json when the sandbox copy is a tie or older on teardown", async () => {
|
|
const cases = [
|
|
{
|
|
name: "tie",
|
|
sandboxAuth: subscriptionAuth({ accountId: "acct-same", lastRefresh: "2026-07-09T02:00:00Z", marker: "s-tie" }),
|
|
hostAuth: subscriptionAuth({ accountId: "acct-same", lastRefresh: "2026-07-09T02:00:00Z", marker: "h-tie" }),
|
|
},
|
|
{
|
|
name: "older",
|
|
sandboxAuth: subscriptionAuth({ accountId: "acct-same", lastRefresh: "2026-07-09T01:00:00Z", marker: "s-old" }),
|
|
hostAuth: subscriptionAuth({ accountId: "acct-same", lastRefresh: "2026-07-09T02:00:00Z", marker: "h-new" }),
|
|
},
|
|
];
|
|
|
|
for (const entry of cases) {
|
|
const result = await runTeardown({ sandboxAuth: entry.sandboxAuth, hostAuth: entry.hostAuth });
|
|
expect(result.finalHostAuth, entry.name).toBe(entry.hostAuth);
|
|
expect(result.finalHostMode, entry.name).toBe(0o600);
|
|
}
|
|
});
|
|
|
|
it("surfaces workspace restore failure after successful provider execution", async () => {
|
|
prepareAdapterExecutionTargetRuntime.mockResolvedValueOnce({
|
|
target: { kind: "remote", transport: "ssh" },
|
|
workspaceRemoteDir: "/remote/workspace",
|
|
runtimeRootDir: REMOTE_RUNTIME_ROOT,
|
|
assetDirs: { home: `${REMOTE_RUNTIME_ROOT}/home` },
|
|
restoreWorkspace: async () => {
|
|
throw new Error("workspace copy-back failed");
|
|
},
|
|
});
|
|
|
|
await expect(
|
|
runTeardown({
|
|
sandboxAuth: subscriptionAuth({ accountId: "acct", marker: "sandbox" }),
|
|
hostAuth: subscriptionAuth({ accountId: "acct", marker: "host" }),
|
|
}),
|
|
).rejects.toThrow("workspace copy-back failed");
|
|
});
|
|
|
|
it("preserves a provider failure when workspace restore also fails", async () => {
|
|
runChildProcess.mockResolvedValueOnce({
|
|
exitCode: 1,
|
|
signal: null,
|
|
timedOut: false,
|
|
stdout: "",
|
|
stderr: "provider failed first",
|
|
pid: 321,
|
|
startedAt: new Date().toISOString(),
|
|
});
|
|
prepareAdapterExecutionTargetRuntime.mockResolvedValueOnce({
|
|
target: { kind: "remote", transport: "ssh" },
|
|
workspaceRemoteDir: "/remote/workspace",
|
|
runtimeRootDir: REMOTE_RUNTIME_ROOT,
|
|
assetDirs: { home: `${REMOTE_RUNTIME_ROOT}/home` },
|
|
restoreWorkspace: async () => {
|
|
throw new Error("workspace copy-back failed second");
|
|
},
|
|
});
|
|
|
|
const result = await runTeardown({
|
|
sandboxAuth: subscriptionAuth({ accountId: "acct", marker: "sandbox" }),
|
|
hostAuth: subscriptionAuth({ accountId: "acct", marker: "host" }),
|
|
});
|
|
expect(result.executionResult.errorMessage).toBe("provider failed first");
|
|
});
|
|
});
|